Adding new Energy Correlation Functions from fastjet-contrib 1.026 #19601
Conversation
|
A new Pull Request was created by @rappoccio for master. It involves the following packages: PhysicsTools/PatAlgos @perrotta, @cmsbuild, @slava77, @monttj, @davidlange6 can you please review it and eventually sign? Thanks. cms-bot commands are listed here |
|
@cmsbuild please test |
|
The tests are being triggered in jenkins. |
|
Comparison job queued. |
|
please test |
|
The tests are being triggered in jenkins. |
|
+1 The following merge commits were also included on top of IB + this PR after doing git cms-merge-topic: |
|
Comparison job queued. |
|
Comparison is ready Comparison Summary:
|
| produces<edm::ValueMap<float> >(ecfN_str.str().c_str()); | ||
| routine_.push_back(std::auto_ptr<fastjet::contrib::EnergyCorrelator> ( new fastjet::contrib::EnergyCorrelator( *n, beta_, fastjet::contrib::EnergyCorrelator::pt_R ) )); | ||
| } | ||
| if ( iConfig.exists("alpha") ) { |
There was a problem hiding this comment.
"alpha" always exist, since you are now using fillDescription()
If you want to treat separately old configs which did not have alpha explicitely set, you must look for a different logic
There was a problem hiding this comment.
Ahh, I didn't realize that. Thanks. Updated now.
| src = cms.InputTag("ak8CHSJets"), | ||
| ecftype = cms.string("C"), | ||
| Njets = cms.vuint32(1, 2, 3), | ||
| beta = cms.double(2.0), |
There was a problem hiding this comment.
"alpha" is still defined by fillDescriptions(), and it will be set as "1" whatever the value of beta.
If you want a different value you must explicitely define it here
The same in the other configs below
|
The code-checks are being triggered in jenkins. |
|
+code-checks |
|
Fixed the indents now. |
|
please test |
|
The tests are being triggered in jenkins. |
|
+1 The following merge commits were also included on top of IB + this PR after doing git cms-merge-topic: |
|
Comparison job queued. |
|
Comparison is ready Comparison Summary:
|
|
Trying to summarize what changed with the latest commits, since last review (and reco signature) the following updates were applied:
Did I miss anything? The timing measured with the new modules does not seem significantly improved (but I should probably test it in a machine emptier that now cmsdev02 to write here a more quantitative statement) |
|
I am seeing a factor of ~2 in workflow 10024. Somehow with workflow 10224 on 10 events I saw an even larger improvement, which didn't necessarily make a lot of sense to me. |
|
So are we okay with this? There is one more request from SMP to add, and I don't want the features to collide ;) |
|
After some debug I verified that the cut on the third jet pt can save some cpu but not that much, in accordance with the observations. In fact, the selector cut acts on ak8 jets (which are by the way rather energetic in the 10224 wf output), but not on the subjects, which give the most of the combinations. In any case the observation is that the cut works, although not being as effective as hoped in reducing the cpu time. I can therefor confirm my previous signature, being fine with the latest adjustments introduced since then, |
|
Thanks @perrotta, I will put this into the final request from SMP. |
|
+1
|
|
merge |
The new energy correlation functions are the next generation of the n-subjettiness tau variables. These have similar performance, but are much better behaved as a function of jet pt and mass. However, we prefer to use the GROOMED observables, so this comes with two necessary code changes:
applySubstructure in MiniAOD: Need to move the AK8 PF jets with puppi and soft drop to the top of the file. We then run ECFs on the AK8 Puppi soft-dropped collection, and store as user floats. Then, we need to merge the soft-dropped version with the ungroomed version for final storage.
JetSubstructurePacker: To accomplish the merging of soft-dropped ECFs with the ungroomed version, we also need to "forward" the userFloats from the pat::Jets that are soft-dropped into the ungroomed collection.
The rest are the updates of the ECFs themselves. Once this is integrated, we will prepare a backport to 9.2.x to take advantage of upcoming MINIAOD reproductions.