Simplify jet constituent access in MiniAOD #22914
Conversation
|
The code-checks are being triggered in jenkins. |
|
+code-checks Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-22914/4301 |
|
A new Pull Request was created by @ahinzmann for master. It involves the following packages: DataFormats/PatCandidates @perrotta, @monttj, @cmsbuild, @slava77, @gpetruc, @arizzi can you please review it and eventually sign? Thanks. cms-bot commands are listed here |
|
while this makes sense I wonder if we have some troubles anywhere on user code relying on current behavior. For example something like this: |
DataFormats/PatCandidates/src/Jet.cc
Outdated
| const std::vector<reco::CandidatePtr> & jdaus = daughterPtrVector(); | ||
| for (const reco::CandidatePtr & dau : jdaus) { | ||
| if (dau->isJet()) { | ||
| const std::vector<reco::CandidatePtr> & sjdaus = edm::Ptr<pat::Jet>(dau)->daughterPtrVector(); |
There was a problem hiding this comment.
I would prefer
const pat::Jet *subjet = dynamic_cast<const pat::Jet *>(&*dau);
if (subjet) { ...
or perhaps with reco::Jet instead of pat::Jet?
there's no guarantee that isJet() will return true only a pat::Jet object (and there may be overheads in doing the cast using edm::Ptr, which is unnecessary since you're not writing it to a file)
|
@arizzi This should only affect code which is trying to find subjets via daughter() rather than subjets(). Since multiple subjet collections are stored, access via daughter() is anyways unsafe. That's why it may be acceptable to change behavior for this particular case. |
|
The code-checks are being triggered in jenkins. |
|
+code-checks Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-22914/4304 |
|
@cmsbuild please test |
|
The tests are being triggered in jenkins. |
|
please test |
|
The tests are being triggered in jenkins. |
|
Comparison job queued. |
|
Comparison is ready Comparison Summary:
|
|
+1
|
|
@ahinzmann |
|
@fabiocos |
|
@slava77 I understand from the review thread that the result is now backward compatible with the previous behaviour, and for 10_2_X this new way of accessing constituents should be ok. |
|
+1 |
|
@fabiocos : an explict "merge" is needed |
|
merge as usual I often forget it... |
The slimmedJetsAK8 in MiniAOD are produced with the JetSubstructurePacker.cc. To save space the jet daughters are used to store both subjets and jet constituents. In order to compute e.g. jet ID or substructure observables, the jet constituents are needed, which can currently be obtained by collecting the constituents of all subjet daughters of the jet plus the non-subjet daughters of the jet.
To make this much simpler for the user, propose to adapt the pat jet interface to perform this operation automatically.
The user can the consistently obtain subjets via the member function subjets() and constituent particles via the member function daughter().
This does not change any content of RECO or MiniAOD, it only changes the interface to jets with subjets.
@arizzi @gpetruc @rappoccio Please have a look.
Suggest to backport this to all analysis releases if agreed.