Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jettoolbox mini aod 706 #4929

Merged
merged 11 commits into from Sep 12, 2014

Conversation

alefisico
Copy link
Contributor

Adding jet substructure variables into the miniAOD recipe. @arizzi @gpetruc

@cmsbuild
Copy link
Contributor

A new Pull Request was created by @alefisico (Alejandro Gomez Espinosa) for CMSSW_7_0_X.

Jettoolbox mini aod 706

It involves the following packages:

PhysicsTools/PatAlgos
RecoJets/JetAlgorithms
RecoJets/JetProducers

@nclopezo, @monttj, @cmsbuild, @StoyanStoynev, @slava77, @vadler, @Degano can you please review it and eventually sign? Thanks.
@nhanvtran, @yslai, @TaiSakuma, @mmarionncern, @schoef this is something you requested to watch as well.
You can sign-off by replying to this message having '+1' in the first line of your reply.
You can reject by replying to this message having '-1' in the first line of your reply.
@nclopezo, @ktf you are the release manager for this.
You can merge this pull request by typing 'merge' in the first line of your comment.

@cmsbuild
Copy link
Contributor

-1
Tested at: 725fc30
I found an error when building:

>> Compiling edm plugin /build/cmsbuild/jenkins-workarea/workspace/ib-integration-CMSSW_7_0_X-slc6_amd64_gcc481/CMSSW_7_0_X_2014-08-12-0200/src/RecoJets/JetProducers/plugins/FixedGridRhoProducerFastjet.cc 
>> Compiling edm plugin /build/cmsbuild/jenkins-workarea/workspace/ib-integration-CMSSW_7_0_X-slc6_amd64_gcc481/CMSSW_7_0_X_2014-08-12-0200/src/RecoJets/JetProducers/plugins/InputGenJetsParticleSelector.cc 
>> Compiling edm plugin /build/cmsbuild/jenkins-workarea/workspace/ib-integration-CMSSW_7_0_X-slc6_amd64_gcc481/CMSSW_7_0_X_2014-08-12-0200/src/RecoJets/JetProducers/plugins/JetIDProducer.cc 
>> Compiling edm plugin /build/cmsbuild/jenkins-workarea/workspace/ib-integration-CMSSW_7_0_X-slc6_amd64_gcc481/CMSSW_7_0_X_2014-08-12-0200/src/RecoJets/JetProducers/plugins/NjettinessAdder.cc 
>> Compiling edm plugin /build/cmsbuild/jenkins-workarea/workspace/ib-integration-CMSSW_7_0_X-slc6_amd64_gcc481/CMSSW_7_0_X_2014-08-12-0200/src/RecoJets/JetProducers/plugins/PileupJPTJetIdProducer.cc 
/build/cmsbuild/jenkins-workarea/workspace/ib-integration-CMSSW_7_0_X-slc6_amd64_gcc481/CMSSW_7_0_X_2014-08-12-0200/src/RecoJets/JetProducers/plugins/NjettinessAdder.cc:2:41: fatal error: fastjet/contrib/Njettiness.hh: No such file or directory
 #include "fastjet/contrib/Njettiness.hh"
                                         ^
compilation terminated.
/build/cmsbuild/jenkins-workarea/workspace/ib-integration-CMSSW_7_0_X-slc6_amd64_gcc481/CMSSW_7_0_X_2014-08-12-0200/src/RecoJets/JetProducers/plugins/NjettinessAdder.cc:2:41: fatal error: fastjet/contrib/Njettiness.hh: No such file or directory
 #include "fastjet/contrib/Njettiness.hh"


you can see the results of the tests here:
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-4929/246/summary.html

@alefisico
Copy link
Contributor Author

Hi

yes, because the fastjet-contrib package must be include in the cmssw environment. This part I don't know how to do it, since the files that I have to add are outside the repository.

I had to include these two files:
fastjet-contrib.xml and fastjet-contrib-archive.xml into CMSSW_7_0_X/config/toolbox/slc6_amd64_gcc481/tools/selected/

fastjet-contrib.xml contains:

<tool name="fastjet-contrib" version="1.009">
    <info url="http://fastjet.hepforge.org/contrib/"/>
    <lib name="fastjetcontribfragile"/>
    <client>
      <environment name="FASTJET_CONTRIB_BASE" default="/cvmfs/cms.cern.ch/slc6_amd64_gcc481/external/fastjet-contrib/1.009"/>
      <environment name="LIBDIR" default="$FASTJET_CONTRIB_BASE/lib"/>
      <environment name="INCLUDE" default="$FASTJET_CONTRIB_BASE/include"/>
    </client>
 </tool>

while fastjet-contrib-archive.xml contains:

  <tool name="fastjet-contrib-archive" version="1.009">
    <info url="http://fastjet.hepforge.org/contrib/"/>
    <lib name="EnergyCorrelator"/>
    <lib name="GenericSubtractor"/>
    <lib name="JetCleanser"/>
    <lib name="JetFFMoments"/>
    <lib name="Nsubjettiness"/>
    <lib name="ScJet"/>
    <lib name="SubjetCounting"/>
    <lib name="VariableR"/>
    <client>
      <environment name="FASTJET_CONTRIB_BASE" default="/cvmfs/cms.cern.ch/slc6_amd64_gcc481/external/fastjet-contrib/1.009"/>
      <environment name="LIBDIR" default="$FASTJET_CONTRIB_BASE/lib"/>
      <environment name="INCLUDE" default="$FASTJET_CONTRIB_BASE/include"/>
    </client>
  </tool>

Then you will have to set them up:
scram setup fastjet-contrib
scram setup fastjet-contrib-archive

A quick description is here: https://twiki.cern.ch/twiki/bin/viewauth/CMS/JetToolbox#Produce_miniAOD_including_the_to

Please let me know if I have to do something else.
cheers,

@ghost
Copy link

ghost commented Aug 13, 2014

@alefisico we cannot merge a pull request that uses external software which is not the one we provide alongside with cmssw.
That being said I see 2 options here:

  1. If this PR is really needed in 7_0_X series then you should ask to backport fastjet-contrib from 7_2_X externals repository to 7_0_X therefore making it available in 7_0 environment

  2. Make this PR for 7_2_X where fastjet-contrib is already in place and can be used directly.

Cheers

@gpetruc
Copy link
Contributor

gpetruc commented Aug 13, 2014

Hi Alessandro,

In this case it's option (1) what we need: backport of the fastjet-contrib
to 7.0.X
If the external doesn't exist in 70X then it should be easy, it can't break
anything existing.

Giovanni

Il 13-ago-2014 15:43 "Alessandro Degano" notifications@github.com ha
scritto:

@alefisico https://github.com/alefisico we cannot merge a pull request
that uses external software which is not the one we provide alongside with
cmssw.
That being said I see 2 options here:

  1. If this PR is really needed in 7_0_X series then you should ask to
    backport fastjet-contrib from 7_2_X externals repository to 7_0_X therefore
    making it available in 7_0 environment

  2. Make this PR for 7_2_X where fastjet-contrib is already in place and
    can be used directly.

Cheers


Reply to this email directly or view it on GitHub
#4929 (comment).

@ktf
Copy link
Contributor

ktf commented Aug 18, 2014

@nclopezo can you try doing the back port in 70X? It should be trivial. Pass by if you have any troubles.

@davidlange6
Copy link
Contributor

@slava77 , @StoyanStoynev - could you re-review this one please? Trying to close out the 70x build we need

@StoyanStoynev
Copy link
Contributor

@slava77 I can look at it but let me know if you have anything specific for it in mind (as you were reviewing it).

@StoyanStoynev
Copy link
Contributor

Tested df6404b on top of CMSSW_7_0_X_2014-09-12-0200.
The update from the previous tests are about the random number services and as far as I can tell these are changed according to the agreed discussion above. There is no effect on standard RECO (as before; short matrix run with the FWLite based script for comparisons). I also checked the event content of miniAOD in wf 25202 (ttbar/25 ns/PU) - the branch patJets_slimmedJetsAK8__PAT changes from 5074.74 to 5248.22 Bytes/Event (Uncompressed); from the updates to the miniAOD code I'd expect an effect on AK8 jets. No other issues.
@slava77 if you have nothing to add I'd sign off.

@slava77
Copy link
Contributor

slava77 commented Sep 12, 2014

Nothing from me for 70X.
We need to have the same random number setting method in 71X and 72X for this code.

@jstupak @alefisico please follow up, if it's not compliant to the review we had here already

@StoyanStoynev
Copy link
Contributor

+1
For df6404b .
Based on the review above.

@cmsbuild
Copy link
Contributor

This pull request is fully signed and it will be integrated in one of the next CMSSW_7_0_X IBs unless changes (tests are also fine). @nclopezo, @ktf can you please take care of it?

@jstupak
Copy link
Contributor

jstupak commented Sep 12, 2014

@slava77 Does this mean this won't be automatically forward ported?

@arizzi
Copy link
Contributor

arizzi commented Sep 12, 2014

I understood from @ktf that auto forward porting is now off... so I guess you need to forward port by hand

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet