feat: allow for not sorting during PATJetUpdater call #32297

andrzejnovak · 2020-11-26T13:44:27Z

Allow for running updateJetCollection without resorting the jets. This is to be used when i.e. updating subjet btag discriminators, but they are already ordered/matched to FatJets.

Does not modify the default behaviour.

@alefisico

cmsbuild · 2020-11-26T13:52:50Z

-code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-32297/20113

This PR adds an extra 36KB to repository

Code check has found code style and quality issues which could be resolved by applying following patch(s)

code-format:
https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-32297/20113/code-format.patch
e.g. curl https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-32297/20113/code-format.patch | patch -p1
You can also run scram build code-format to apply code format directly

cmsbuild · 2020-11-26T14:10:33Z

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-32297/20116

This PR adds an extra 36KB to repository

cmsbuild · 2020-11-26T14:11:00Z

A new Pull Request was created by @andrzejnovak (Andrzej Novak) for master.

It involves the following packages:

PhysicsTools/PatAlgos

@perrotta, @jpata, @cmsbuild, @santocch, @slava77 can you please review it and eventually sign? Thanks.
@rappoccio, @gouskos, @JyothsnaKomaragiri, @ahinzmann, @smoortga, @schoef, @mmarionncern, @jdamgov, @jdolen, @nhanvtran, @gkasieczka, @clelange, @emilbols, @hatakeyamak, @ferencek, @gpetruc, @andrzejnovak, @mariadalfonso, @seemasharmafnal this is something you requested to watch as well.
@silviodonato, @dpiparo, @qliphy you are the release manager for this.

cms-bot commands are listed here

perrotta · 2020-11-27T07:34:38Z

please test

cmsbuild · 2020-11-27T07:35:02Z

The tests are being triggered in jenkins.

CMSSW_11_3_X_2020-11-26-2300/slc7_amd64_gcc900: https://cmssdt.cern.ch/jenkins/job/ib-run-pr-tests/11093/console Started: 2020/11/27 10:47

cmsbuild · 2020-11-27T09:54:47Z

-1

Tested at: d5ab218

CMSSW: CMSSW_11_3_X_2020-11-26-2300
SCRAM_ARCH: slc7_amd64_gcc900
You can see the results of the tests here:
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-45c264/11093/summary.html

I found follow errors while testing this PR

Failed tests: HeaderConsistency ClangBuild

Clang:

I found compilation error while trying to compile with clang. Command used:

USER_CUDA_FLAGS='--expt-relaxed-constexpr' USER_CXXFLAGS='-Wno-register -fsyntax-only' scram build -k -j 32 COMPILER='llvm compile'

>> Compiling edm plugin /data/cmsbld/jenkins/workspace/ib-run-pr-tests/CMSSW_11_3_X_2020-11-26-2300/src/PhysicsTools/PatAlgos/plugins/RecoMETExtractor.cc
>> Compiling edm plugin /data/cmsbld/jenkins/workspace/ib-run-pr-tests/CMSSW_11_3_X_2020-11-26-2300/src/PhysicsTools/PatAlgos/plugins/StringResolutionProviderESProducer.cc
>> Compiling edm plugin /data/cmsbld/jenkins/workspace/ib-run-pr-tests/CMSSW_11_3_X_2020-11-26-2300/src/PhysicsTools/PatAlgos/plugins/TauJetCorrFactorsProducer.cc
>> Compiling edm plugin /data/cmsbld/jenkins/workspace/ib-run-pr-tests/CMSSW_11_3_X_2020-11-26-2300/src/PhysicsTools/PatAlgos/plugins/TrackAndVertexUnpacker.cc
>> Compiling edm plugin /data/cmsbld/jenkins/workspace/ib-run-pr-tests/CMSSW_11_3_X_2020-11-26-2300/src/PhysicsTools/PatAlgos/plugins/VertexAssociationProducer.cc
/data/cmsbld/jenkins/workspace/ib-run-pr-tests/CMSSW_11_3_X_2020-11-26-2300/src/PhysicsTools/PatAlgos/plugins/PATTriggerProducer.cc:45:21: error: call to implicitly-deleted default constructor of 'pat::PATTriggerProducer::ModuleLabelToPathAndFlags'
PATTriggerProducer::PATTriggerProducer(const ParameterSet& iConfig)
                    ^
/data/cmsbld/jenkins/workspace/ib-run-pr-tests/CMSSW_11_3_X_2020-11-26-2300/src/PhysicsTools/PatAlgos/plugins/PATTriggerProducer.h:145:39: note: default constructor of 'ModuleLabelToPathAndFlags' is implicitly deleted because field 'empty_' of const-qualified type 'const std::vector' would not be initialized
      const std::vector empty_;
                                      ^

cmsbuild · 2020-11-27T09:54:49Z

Comparison not run due to Build errors/Fireworks only changes/No short matrix requested (RelVals and Igprof tests were also skipped)

perrotta · 2020-11-30T14:23:00Z

+1

It ended up that this will only involve some (private? future?) nanoAOD production, while the change is completely transparent to reco: let assign xpog for evaluating it, then
Jenkins tetst pass and show no differences wrt the baseline, as expected

perrotta · 2020-11-30T14:23:08Z

assign xpog

cmsbuild · 2020-11-30T14:23:20Z

New categories assigned: xpog

@fgolf,@mariadalfonso,@gouskos you have been requested to review this Pull request/Issue and eventually sign? Thanks

santocch · 2020-11-30T22:55:59Z

+1

silviodonato · 2020-12-07T17:43:00Z

kind reminder @cms-sw/xpog-l2

andrzejnovak · 2020-12-08T23:04:17Z

I suppose this is not a critical PR to have in asap, but at the same time the change is trivial.

mariadalfonso · 2020-12-09T15:11:09Z

I suppose this is not a critical PR to have in asap, but at the same time the change is trivial.

@andrzejnovak

I'm trying to figure out the case when you need to switch on/off this sorting.

in the default nano in 10_6 PATJetUpdater is called 6 times.
Any of this case follow your need or not of sorting ?

process.patJetsReapplyJEC = cms.EDProducer("PATJetUpdater",
process.updatedJets = cms.EDProducer("PATJetUpdater",
process.updatedJetsAK8 = cms.EDProducer("PATJetUpdater",
process.updatedPatJets = cms.EDProducer("PATJetUpdater",
process.updatedPatJetsAK8WithDeepInfo = cms.EDProducer("PATJetUpdater",
process.updatedPatJetsTransientCorrectedAK8WithDeepInfo = cms.EDProducer("PATJetUpdater",

andrzejnovak · 2020-12-09T15:54:56Z

@mariadalfonso No, at the moment this is not needed for any central workflow, but if we wanted to re-calculate btags also for subjets like it currently does for fatjets, it would get used (pt sorting the subjets would mess up the subject link to FatJet_subjetIdx)

alefisico · 2020-12-09T17:12:33Z

I suppose this is not a critical PR to have in asap, but at the same time the change is trivial.

@andrzejnovak

I'm trying to figure out the case when you need to switch on/off this sorting.

in the default nano in 10_6 PATJetUpdater is called 6 times.
Any of this case follow your need or not of sorting ?

process.patJetsReapplyJEC = cms.EDProducer("PATJetUpdater",
process.updatedJets = cms.EDProducer("PATJetUpdater",
process.updatedJetsAK8 = cms.EDProducer("PATJetUpdater",
process.updatedPatJets = cms.EDProducer("PATJetUpdater",
process.updatedPatJetsAK8WithDeepInfo = cms.EDProducer("PATJetUpdater",
process.updatedPatJetsTransientCorrectedAK8WithDeepInfo = cms.EDProducer("PATJetUpdater",

If I can add something here, in the case when we were trying to run the AK8 jet collection with the new PUPPI tune from miniAOD, we would need to include this small fix as part of that PR to correctly handle the link between the subjet collection and the jet collection.

mariadalfonso · 2020-12-09T18:30:55Z

I suppose this is not a critical PR to have in asap, but at the same time the change is trivial.

@andrzejnovak
I'm trying to figure out the case when you need to switch on/off this sorting.
in the default nano in 10_6 PATJetUpdater is called 6 times.
Any of this case follow your need or not of sorting ?
process.patJetsReapplyJEC = cms.EDProducer("PATJetUpdater",
process.updatedJets = cms.EDProducer("PATJetUpdater",
process.updatedJetsAK8 = cms.EDProducer("PATJetUpdater",
process.updatedPatJets = cms.EDProducer("PATJetUpdater",
process.updatedPatJetsAK8WithDeepInfo = cms.EDProducer("PATJetUpdater",
process.updatedPatJetsTransientCorrectedAK8WithDeepInfo = cms.EDProducer("PATJetUpdater",

If I can add something here, in the case when we were trying to run the AK8 jet collection with the new PUPPI tune from miniAOD, we would need to include this small fix as part of that PR to correctly handle the link between the subjet collection and the jet collection.

Hi @alefisico
is it possible to have the PR related to the addition of the AK8 jet collection with the new PUPPI tune?
at least we have a way to validate this reordering before merging

Maria

andrzejnovak · 2020-12-09T21:42:25Z

@mariadalfonso I am not sure what you mean. The only way you can detect something wrong has happened in this case is if you try to access the subjet collection and expect a specific ordering. i.e when the subjet index is explicitly used somewhere else like the linking to FatJets. I am not sure, but I think that's currently beyond the scope of the nano dqm.

In the current CMSSW, calling updateJetCollection on the subjet collection, when creating Nano will unset the order given in

[1]

and therefore the values in [2] will point to different jets than intended.

The effect is visible if one calculates the dR(FatJet, Subjet) for the subjets linked via the index in [2] In orange you see the result when the subjet collection is pT sorted after the update.

IIUC [1] is called in MINI production and not called again in Nano so the stored subjet collection is not pT ordered in neither MINI nor Nano. However, if it was needed to update the subjet collection in nano, like it is for regular jets [3] this would sort the subjet collection, scrambling the values in [2].

[1] https://github.com/cms-sw/cmssw/blob/master/PhysicsTools/PatAlgos/python/slimming/applySubstructure_cff.py#L156-L194
[2] https://github.com/cms-sw/cmssw/blob/master/PhysicsTools/NanoAOD/python/jets_cff.py#L474-L478
[3] https://github.com/cms-sw/cmssw/blob/master/PhysicsTools/NanoAOD/python/nano_cff.py#L161

silviodonato · 2020-12-14T16:18:49Z

kind reminder @cms-sw/xpog-l2

mariadalfonso · 2020-12-15T17:30:43Z

+xpog

update the PATJetUpdater in view of future use of subject, see offline validation here
#32297 (comment)

cmsbuild · 2020-12-15T17:31:08Z

This pull request is fully signed and it will be integrated in one of the next master IBs (tests are also fine). This pull request will now be reviewed by the release team before it's merged. @silviodonato, @dpiparo, @qliphy (and backports should be raised in the release meeting by the corresponding L2)

silviodonato · 2020-12-15T18:29:04Z

+1

cmsbuild added this to the CMSSW_11_3_X milestone Nov 26, 2020

cmsbuild added analysis-pending code-checks-pending comparison-pending orp-pending pending-signatures reconstruction-pending tests-pending labels Nov 26, 2020

andrzejnovak mentioned this pull request Nov 26, 2020

[10_6_X] feat: allow for not sorting during PATJetUpdater call #32298

Merged

cmsbuild added code-checks-rejected and removed code-checks-pending labels Nov 26, 2020

andrzejnovak force-pushed the nosortUpdate branch from 6663bc0 to 74b0314 Compare November 26, 2020 14:00

cmsbuild added code-checks-pending and removed code-checks-rejected labels Nov 26, 2020

feat: allow for not sorting during PATJetUpdater call

d5ab218

andrzejnovak force-pushed the nosortUpdate branch from 74b0314 to d5ab218 Compare November 26, 2020 14:04

cmsbuild added code-checks-approved and removed code-checks-pending labels Nov 26, 2020

cmsbuild added tests-started and removed tests-pending labels Nov 27, 2020

cmsbuild added comparison-notrun and removed comparison-pending tests-started labels Nov 27, 2020

cmsbuild added reconstruction-approved xpog-pending and removed reconstruction-pending labels Nov 30, 2020

cmsbuild added analysis-approved and removed analysis-pending labels Nov 30, 2020

cmsbuild added fully-signed xpog-approved and removed pending-signatures xpog-pending labels Dec 15, 2020

mariadalfonso mentioned this pull request Dec 15, 2020

features for V9 nanoAOD cms-nanoAOD/cmssw#555

Closed

55 tasks

cmsbuild added orp-approved and removed orp-pending labels Dec 15, 2020

cmsbuild merged commit f06a7de into cms-sw:master Dec 15, 2020

This was referenced Dec 16, 2020

Add and use preprocessors macros for loop unrolling #32499

Merged

[Do not merge] For testing bot commit statuses #32496

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: allow for not sorting during PATJetUpdater call #32297

feat: allow for not sorting during PATJetUpdater call #32297

andrzejnovak commented Nov 26, 2020

cmsbuild commented Nov 26, 2020

cmsbuild commented Nov 26, 2020

cmsbuild commented Nov 26, 2020

perrotta commented Nov 27, 2020

cmsbuild commented Nov 27, 2020 •

edited

cmsbuild commented Nov 27, 2020

cmsbuild commented Nov 27, 2020

perrotta commented Nov 30, 2020

perrotta commented Nov 30, 2020

cmsbuild commented Nov 30, 2020

santocch commented Nov 30, 2020

silviodonato commented Dec 7, 2020

andrzejnovak commented Dec 8, 2020

mariadalfonso commented Dec 9, 2020

andrzejnovak commented Dec 9, 2020

alefisico commented Dec 9, 2020

mariadalfonso commented Dec 9, 2020

andrzejnovak commented Dec 9, 2020

silviodonato commented Dec 14, 2020

mariadalfonso commented Dec 15, 2020

cmsbuild commented Dec 15, 2020

silviodonato commented Dec 15, 2020

feat: allow for not sorting during PATJetUpdater call #32297

feat: allow for not sorting during PATJetUpdater call #32297

Conversation

andrzejnovak commented Nov 26, 2020

cmsbuild commented Nov 26, 2020

cmsbuild commented Nov 26, 2020

cmsbuild commented Nov 26, 2020

perrotta commented Nov 27, 2020

cmsbuild commented Nov 27, 2020 • edited

cmsbuild commented Nov 27, 2020

cmsbuild commented Nov 27, 2020

perrotta commented Nov 30, 2020

perrotta commented Nov 30, 2020

cmsbuild commented Nov 30, 2020

santocch commented Nov 30, 2020

silviodonato commented Dec 7, 2020

andrzejnovak commented Dec 8, 2020

mariadalfonso commented Dec 9, 2020

andrzejnovak commented Dec 9, 2020

alefisico commented Dec 9, 2020

mariadalfonso commented Dec 9, 2020

andrzejnovak commented Dec 9, 2020

silviodonato commented Dec 14, 2020

mariadalfonso commented Dec 15, 2020

cmsbuild commented Dec 15, 2020

silviodonato commented Dec 15, 2020

cmsbuild commented Nov 27, 2020 •

edited