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

Fix fatJetTable genJetAK8Idx matching #33839

Merged
merged 1 commit into from Jun 6, 2021

Conversation

laurenhay
Copy link
Contributor

PR description:

Fixes issue where the GenJetAK8IDx matching did not match the corresponding GenJetAK8Table id's because of a difference in pT cut between the collections being referenced https://hypernews.cern.ch/HyperNews/CMS/get/JetMET/2103.htm

PR validation:

Validated on file in question in 10_6_19_patch2. Could not find any files with the same issue present in the 12 series.
https://cmsweb.cern.ch/das/request?instance=prod/global&input=dataset+file%3D%2Fstore%2Fmc%2FRunIISummer19UL17NanoAODv2%2FTTToHadronic_TuneCP5_13TeV-powheg-pythia8%2FNANOAODSIM%2F106X_mc2017_realistic_v8-v1%2F280000%2F1FA6BD39-B319-C143-92BC-13BC01AF4B89.root

if this PR is a backport please specify the original PR and why you need to backport that PR:

NA

@cmsbuild
Copy link
Contributor

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-33839/22858

@cmsbuild
Copy link
Contributor

A new Pull Request was created by @laurenhay for master.

It involves the following packages:

PhysicsTools/NanoAOD

@cmsbuild, @mariadalfonso, @gouskos, @fgolf can you please review it and eventually sign? Thanks.
@gpetruc, @swertz 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

@mariadalfonso
Copy link
Contributor

please test

@cmsbuild
Copy link
Contributor

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-d0b602/15312/summary.html
COMMIT: a039dc0
CMSSW: CMSSW_12_0_X_2021-05-25-2300/slc7_amd64_gcc900
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week0/cms-sw/cmssw/33839/15312/install.sh to create a dev area with all the needed externals and cmssw changes.

Comparison Summary

Summary:

  • No significant changes to the logs found
  • Reco comparison results: 1 differences found in the comparisons
  • DQMHistoTests: Total files compared: 37
  • DQMHistoTests: Total histograms compared: 2650486
  • DQMHistoTests: Total failures: 1
  • DQMHistoTests: Total nulls: 0
  • DQMHistoTests: Total successes: 2650463
  • DQMHistoTests: Total skipped: 22
  • DQMHistoTests: Total Missing objects: 0
  • DQMHistoSizes: Histogram memory added: 0.0 KiB( 36 files compared)
  • Checked 155 log files, 37 edm output root files, 37 DQM output files
  • TriggerResults: no differences found

@mariadalfonso
Copy link
Contributor

@laurenhay can you check the samples listed here and report here the "Scan" before and after the fix ?
You should be able to read in 11_2 the mini of 10_6
https://hypernews.cern.ch/HyperNews/CMS/get/JetMET/2103.html

@laurenhay
Copy link
Contributor Author

@laurenhay can you check the samples listed here and report here the "Scan" before and after the fix ?
You should be able to read in 11_2 the mini of 10_6
https://hypernews.cern.ch/HyperNews/CMS/get/JetMET/2103.html

We scanned the file in 10_6_19_patch2 before changes and got the same output Matej saw:

[lahay@cmsdev20 src]$ root -l root://cms-xrd-global.cern.ch//store/mc/RunIISummer19UL17NanoAODv2/TTToHadronic_TuneCP5_13TeV-powheg-pythia8/NANOAODSIM/106X_mc2017_realistic_v8-v1/280000/1FA6BD39-B319-C143-92BC-13BC01AF4B89.root
root [0] 
Attaching file root://cms-xrd-global.cern.ch//store/mc/RunIISummer19UL17NanoAODv2/TTToHadronic_TuneCP5_13TeV-powheg-pythia8/NANOAODSIM/106X_mc2017_realistic_v8-v1/280000/1FA6BD39-B319-C143-92BC-13BC01AF4B89.root as _file0...
(TFile *) 0x55a2cf0
root [1] Events->Scan("FatJet_genJetAK8Idx[0]:nGenJetAK8","FatJet_genJetAK8Idx[0]>nGenJetAK8 ")
************************************
*    Row   * FatJet_ge * nGenJetAK *
************************************
*      149 *         2 *         0 *
*      561 *         3 *         2 *
*     2134 *         2 *         1 *
*     2939 *         5 *         3 *
*     9582 *         3 *         0 *
*    11441 *         3 *         2 *
*    16619 *         3 *         2 *
*    17807 *         2 *         0 *
... 

After the change, I reproduced a subset of the events using this cmsDriver command:

cmsDriver.py --python_filename TOP-RunIISummer19UL17NanoAODv2-00091_1_cfg.py --eventcontent NANOAODSIM --customise Configuration/DataProcessing/Utils.addMonitoring --datatier NANOAODSIM --fileout file:TOP-RunIISummer19UL17NanoAODv2-00091.root --conditions 106X_mc2017_realistic_v8 --step NANO --filein dbs:/TTToHadronic_TuneCP5_13TeV-powheg-pythia8/RunIISummer19UL17MiniAOD-106X_mc2017_realistic_v6-v4/MINIAODSIM --era Run2_2017,run2_nanoAOD_106Xv1 --no_exec --mc -n 100

And the resulting output from scanning the file was:

[lahay@cmsdev20 src]$ root -l TOP-RunIISummer19UL17NanoAODv2-00091.root 
*** DISPLAY not set, setting it to lxplus734.cern.ch:0.0
root [0] 
Attaching file TOP-RunIISummer19UL17NanoAODv2-00091.root as _file0...
(TFile *) 0x344c970
root [1] Events->Scan("FatJet_genJetAK8Idx[0]:nGenJetAK8","FatJet_genJetAK8Idx[0]>nGenJetAK8 ")
************************************
*    Row   * FatJet_ge * nGenJetAK *
************************************
************************************
==> 0 selected entries
(long long) 0

I tried to reproduce the file and changes in 11_2 (and 12_0) using the auto:run2_mc conditions flag, but I also likely need an eras modifier, but do not see one to use for anything past 10_6.
This is the error when I try to run in an CMSSW version past 10_6:

Traceback (most recent call last):
  File "/cvmfs/cms.cern.ch/slc7_amd64_gcc900/cms/cmssw/CMSSW_11_2_1/bin/slc7_amd64_gcc900/cmsDriver.py", line 56, in <module>
    run()
  File "/cvmfs/cms.cern.ch/slc7_amd64_gcc900/cms/cmssw/CMSSW_11_2_1/bin/slc7_amd64_gcc900/cmsDriver.py", line 28, in run
    configBuilder.prepare()
  File "/cvmfs/cms.cern.ch/slc7_amd64_gcc900/cms/cmssw-patch/CMSSW_11_2_1_patch2/python/Configuration/Applications/ConfigBuilder.py", line 2273, in prepare
    self.pythonCfgCode += self.addCustomise()
  File "/cvmfs/cms.cern.ch/slc7_amd64_gcc900/cms/cmssw-patch/CMSSW_11_2_1_patch2/python/Configuration/Applications/ConfigBuilder.py", line 885, in addCustomise
    self.process=getattr(package,fcn)(self.process)
  File "/build/lahay/CMSSW_11_2_1_patch2/python/PhysicsTools/NanoAOD/nano_cff.py", line 377, in nanoAOD_customizeMC
    process = nanoAOD_recalibrateMETs(process,isData=False)
  File "/build/lahay/CMSSW_11_2_1_patch2/python/PhysicsTools/NanoAOD/nano_cff.py", line 236, in nanoAOD_recalibrateMETs
    getJetMCFlavour=False
  File "/cvmfs/cms.cern.ch/slc7_amd64_gcc900/cms/cmssw-patch/CMSSW_11_2_1_patch2/python/PhysicsTools/PatAlgos/tools/jetTools.py", line 1057, in __call__
    self.apply(process)
  File "/cvmfs/cms.cern.ch/slc7_amd64_gcc900/cms/cmssw-patch/CMSSW_11_2_1_patch2/python/PhysicsTools/PatAlgos/tools/ConfigToolBase.py", line 71, in apply
    self.toolCode(process)
  File "/cvmfs/cms.cern.ch/slc7_amd64_gcc900/cms/cmssw-patch/CMSSW_11_2_1_patch2/python/PhysicsTools/PatAlgos/tools/jetTools.py", line 1172, in toolCode
    _newPatJets.genJetMatch.setModuleLabel('patJetGenJetMatch'+_labelName+postfix)
AttributeError: 'EDProducer' object has no attribute 'genJetMatch'

(@rappoccio )

@mariadalfonso
Copy link
Contributor

from local tests we have error of this type
https://gitlab.cern.ch/cms-nanoAOD/nanoAOD-integration/-/pipelines/2671791/builds

MSG
1848----- Begin Fatal Exception 01-Jun-2021 12:00:47 CEST-----------------------
1849An exception of category 'Configuration' occurred while
1850   [0] Processing  Event run: 1 lumi: 58 event: 5759 stream: 0
1851   [1] Running path 'nanoAOD_step'
1852  [2] Calling method for module SimpleCandidateFlatTableProducer/'fatJetMCTable'
1853Exception Message:
1854m

@rappoccio
Copy link
Contributor

Thanks Maria. @laurenhay is relocating to CERN yesterday and today so we will get it up ASAP.

@@ -668,7 +668,7 @@
nBHadrons = Var("jetFlavourInfo().getbHadrons().size()", "uint8", doc="number of b-hadrons"),
nCHadrons = Var("jetFlavourInfo().getcHadrons().size()", "uint8", doc="number of c-hadrons"),
hadronFlavour = Var("hadronFlavour()", int, doc="flavour from hadron ghost clustering"),
genJetAK8Idx = Var("?genJetFwdRef().backRef().isNonnull()?genJetFwdRef().backRef().key():-1", int, doc="index of matched gen AK8 jet"),
genJetAK8Idx = Var("?genJetFwdRef().backRef().pt() > 100. && genJetFwdRef().backRef().isNonnull()?genJetFwdRef().backRef().key():-1", int, doc="index of matched gen AK8 jet"),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fix is trivial here, just need to switch order:

genJetFwdRef().backRef().isNonnull() && genJetFwdRef().backRef().pt() > 100. 

@cmsbuild
Copy link
Contributor

cmsbuild commented Jun 2, 2021

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-33839/23042

@mariadalfonso
Copy link
Contributor

please test

@cmsbuild
Copy link
Contributor

cmsbuild commented Jun 4, 2021

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-d0b602/15623/summary.html
COMMIT: d27e7e3
CMSSW: CMSSW_12_0_X_2021-06-03-1100/slc7_amd64_gcc900
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week1/cms-sw/cmssw/33839/15623/install.sh to create a dev area with all the needed externals and cmssw changes.

Comparison Summary

Summary:

  • No significant changes to the logs found
  • Reco comparison results: 5 differences found in the comparisons
  • DQMHistoTests: Total files compared: 37
  • DQMHistoTests: Total histograms compared: 2650486
  • DQMHistoTests: Total failures: 1
  • DQMHistoTests: Total nulls: 0
  • DQMHistoTests: Total successes: 2650463
  • DQMHistoTests: Total skipped: 22
  • DQMHistoTests: Total Missing objects: 0
  • DQMHistoSizes: Histogram memory added: 0.0 KiB( 36 files compared)
  • Checked 155 log files, 37 edm output root files, 37 DQM output files
  • TriggerResults: no differences found

@mariadalfonso
Copy link
Contributor

large stat runs now successfully

NANO changes:

  • particleNetMD and rawDeepTau2017v2p1VSmu are known non reproducibility issues related to the ML
  • the nanoaodFlatTable_fatJetMCTable__DQM_obj_ints__genJetAK8Idx minimally change and is inline with the scope of this PR.

There are miniAOD changes: this should not happen !
this is related to the slimmedAK8, so might be that the gen-matching sequence in MINI and NANO talk and the gen ref doesn't match anymore
@slava77 @perrotta @jpata do you have any insight ?

@rappoccio
Copy link
Contributor

this is related to the slimmedAK8, so might be that the gen-matching sequence in MINI and NANO talk and the gen ref doesn't match anymore

Wow, this is really surprising to me. Can you point me to the changes?

@mariadalfonso
Copy link
Contributor

this is related to the slimmedAK8, so might be that the gen-matching sequence in MINI and NANO talk and the gen ref doesn't match anymore

Wow, this is really surprising to me. Can you point me to the changes?

https://cmssdt.cern.ch/SDT/jenkins-artifacts/baseLineComparisons/CMSSW_12_0_X_2021-06-03-1100+d0b602/43200/validateJR/all_mini_OldVSNew_TTbarwf25p0/

@rappoccio
Copy link
Contributor

Bizarre. The changes are in the pair discriminator vector, which was clearly not touched at all. Is there another change that was somehow picked up by this test?

@mariadalfonso
Copy link
Contributor

please test

(let's check if the miniAOD changes are reproducible)

@cmsbuild
Copy link
Contributor

cmsbuild commented Jun 5, 2021

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-d0b602/15670/summary.html
COMMIT: d27e7e3
CMSSW: CMSSW_12_0_X_2021-06-04-2300/slc7_amd64_gcc900
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week1/cms-sw/cmssw/33839/15670/install.sh to create a dev area with all the needed externals and cmssw changes.

Comparison Summary

Summary:

  • No significant changes to the logs found
  • Reco comparison results: 4 differences found in the comparisons
  • DQMHistoTests: Total files compared: 37
  • DQMHistoTests: Total histograms compared: 2650486
  • DQMHistoTests: Total failures: 6
  • DQMHistoTests: Total nulls: 1
  • DQMHistoTests: Total successes: 2650457
  • DQMHistoTests: Total skipped: 22
  • DQMHistoTests: Total Missing objects: 0
  • DQMHistoSizes: Histogram memory added: 0.004 KiB( 36 files compared)
  • DQMHistoSizes: changed ( 312.0 ): 0.004 KiB MessageLogger/Warnings
  • Checked 155 log files, 37 edm output root files, 37 DQM output files
  • TriggerResults: no differences found

@mariadalfonso
Copy link
Contributor

mariadalfonso commented Jun 5, 2021

the miniAOD changes of previous tests are not reproducible
#33839 (comment)
now only changes expected for the nanoAOD genJetAK8Idx

@mariadalfonso
Copy link
Contributor

+xpog

@cmsbuild
Copy link
Contributor

cmsbuild commented Jun 5, 2021

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)

@mariadalfonso
Copy link
Contributor

@laurenhay please prepare the backport to 10_6_X

@qliphy
Copy link
Contributor

qliphy commented Jun 6, 2021

+1

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

5 participants