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

Nano: drop frozen V10 config, make triggerObjectTable more easily customizable #40321

Merged

Conversation

swertz
Copy link
Contributor

@swertz swertz commented Dec 15, 2022

PR description:

PR validation:

Ran nano matrix workflows.

@swertz
Copy link
Contributor Author

swertz commented Dec 15, 2022

enable nano

@cmsbuild
Copy link
Contributor

-code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-40321/33402

  • This PR adds an extra 32KB to repository

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

@cmsbuild
Copy link
Contributor

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-40321/33404

  • This PR adds an extra 32KB to repository

@cmsbuild
Copy link
Contributor

A new Pull Request was created by @swertz (Sébastien Wertz) for master.

It involves the following packages:

  • Configuration/Eras (operations)
  • Configuration/PyReleaseValidation (pdmv, upgrade)
  • PhysicsTools/NanoAOD (xpog)

@perrotta, @rappoccio, @swertz, @vlimant, @bbilin, @cmsbuild, @AdrianoDee, @srimanob, @kskovpen, @sunilUIET, @fabiocos, @davidlange6 can you please review it and eventually sign? Thanks.
@makortel, @kpedro88, @fabiocos, @Martin-Grunewald, @missirol, @trtomei, @gpetruc, @beaucero, @slomeo this is something you requested to watch as well.
@perrotta, @dpiparo, @rappoccio you are the release manager for this.

cms-bot commands are listed here

@swertz
Copy link
Contributor Author

swertz commented Dec 15, 2022

please test

@cmsbuild
Copy link
Contributor

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-041c1a/29628/summary.html
COMMIT: d42d61a
CMSSW: CMSSW_13_0_X_2022-12-14-2300/el8_amd64_gcc11
Additional Tests: NANO
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week1/cms-sw/cmssw/40321/29628/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
  • ROOTFileChecks: Some differences in event products or their sizes found
  • Reco comparison results: 44 differences found in the comparisons
  • DQMHistoTests: Total files compared: 49
  • DQMHistoTests: Total histograms compared: 3557521
  • DQMHistoTests: Total failures: 1229
  • DQMHistoTests: Total nulls: 0
  • DQMHistoTests: Total successes: 3556270
  • DQMHistoTests: Total skipped: 22
  • DQMHistoTests: Total Missing objects: 0
  • DQMHistoSizes: Histogram memory added: 0.0 KiB( 48 files compared)
  • Checked 211 log files, 162 edm output root files, 49 DQM output files
  • TriggerResults: no differences found

NANO Comparison Summary

Summary:

  • No significant changes to the logs found
  • ROOTFileChecks: Some differences in event products or their sizes found
  • Reco comparison results: 110 differences found in the comparisons
  • DQMHistoTests: Total files compared: 11
  • DQMHistoTests: Total histograms compared: 10755
  • DQMHistoTests: Total failures: 152
  • DQMHistoTests: Total nulls: 0
  • DQMHistoTests: Total successes: 10603
  • DQMHistoTests: Total skipped: 0
  • DQMHistoTests: Total Missing objects: 0
  • DQMHistoSizes: Histogram memory added: 0.0 KiB( 10 files compared)
  • Checked 23 log files, 10 edm output root files, 11 DQM output files

Nano size comparison Summary:

Sample kb/ev ref kb/ev diff kb/ev ev/s/thd ref ev/s/thd diff rate mem/thd ref mem/thd
2500.31 2.230 2.200 0.030 ( +1.4% ) 9.51 9.35 +1.7% 1.557 1.515
2500.311 2.321 2.322 -0.001 ( -0.0% ) 9.26 8.99 +3.0% 1.924 1.877
2500.312 2.273 2.276 -0.003 ( -0.1% ) 9.05 9.13 -0.8% 1.913 1.870
2500.33 1.094 1.089 0.005 ( +0.4% ) 21.87 21.14 +3.4% 1.685 1.685
2500.331 1.388 1.388 0.000 ( +0.0% ) 15.97 15.59 +2.4% 1.832 1.832
2500.332 1.314 1.319 -0.005 ( -0.4% ) 17.58 17.52 +0.3% 1.789 1.794
2500.401 2.152 2.155 -0.003 ( -0.2% ) 10.35 10.07 +2.7% 1.236 1.207
2500.501 1.716 1.721 -0.004 ( -0.3% ) 16.60 15.76 +5.4% 1.146 1.142
2500.511 1.126 1.129 -0.003 ( -0.3% ) 30.39 26.94 +12.8% 1.392 1.442
2500.601 2.064 2.068 -0.005 ( -0.2% ) 12.50 12.57 -0.5% 1.208 1.188

@swertz
Copy link
Contributor Author

swertz commented Dec 15, 2022

Looking at the jet trigger filters for Run2

  • 2016:
    image

  • 2017:
    image

  • 2018:
    image

It seems that the "Run3" trigger filters introduced in #39818 (therefore appearing in blue) are actually more relevant than the "Run2" filters previously present in UL nanoV9 (appearing in red), for 2017 and 2018 menus. The previous "Run2" filters only seem to make sense for 2016.

I think for completeness we should make sure all the necessary Run2 filters are present, so I'll make further changes to add some of the filters from #39818 also for 2017 and 2018 eras.

@johnalison does that make sense to you?

@srimanob
Copy link
Contributor

srimanob commented Jan 3, 2023

+Upgrade

From the upgrade related-code, only clean up in Configuration/PyReleaseValidation.

@swertz
Copy link
Contributor Author

swertz commented Jan 10, 2023

Hi @cms-sw/operations-l2 @cms-sw/pdmv-l2, do you have any comments on this?

@kskovpen
Copy link
Contributor

+pdmv

@perrotta
Copy link
Contributor

please test
(This seems to touch also non-nanoAOD workflows: let have the three weeks old test refreshed first, and then we should try to understand it)

@cmsbuild
Copy link
Contributor

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-041c1a/29875/summary.html
COMMIT: 4b4d26a
CMSSW: CMSSW_13_0_X_2023-01-09-2300/el8_amd64_gcc11
Additional Tests: NANO
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week1/cms-sw/cmssw/40321/29875/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
  • ROOTFileChecks: Some differences in event products or their sizes found
  • Reco comparison results: 16 differences found in the comparisons
  • DQMHistoTests: Total files compared: 49
  • DQMHistoTests: Total histograms compared: 3555538
  • DQMHistoTests: Total failures: 189
  • DQMHistoTests: Total nulls: 0
  • DQMHistoTests: Total successes: 3555327
  • DQMHistoTests: Total skipped: 22
  • DQMHistoTests: Total Missing objects: 0
  • DQMHistoSizes: Histogram memory added: 0.0 KiB( 48 files compared)
  • Checked 211 log files, 162 edm output root files, 49 DQM output files
  • TriggerResults: no differences found

NANO Comparison Summary

Summary:

  • No significant changes to the logs found
  • ROOTFileChecks: Some differences in event products or their sizes found
  • Reco comparison results: 100 differences found in the comparisons
  • DQMHistoTests: Total files compared: 11
  • DQMHistoTests: Total histograms compared: 10839
  • DQMHistoTests: Total failures: 132
  • DQMHistoTests: Total nulls: 0
  • DQMHistoTests: Total successes: 10707
  • DQMHistoTests: Total skipped: 0
  • DQMHistoTests: Total Missing objects: 0
  • DQMHistoSizes: Histogram memory added: 0.0 KiB( 10 files compared)
  • Checked 23 log files, 10 edm output root files, 11 DQM output files

Nano size comparison Summary:

Sample kb/ev ref kb/ev diff kb/ev ev/s/thd ref ev/s/thd diff rate mem/thd ref mem/thd
2500.31 2.240 2.209 0.031 ( +1.4% ) 9.54 9.18 +3.9% 1.475 1.466
2500.311 2.330 2.329 0.001 ( +0.0% ) 9.16 8.84 +3.7% 1.839 1.831
2500.312 2.284 2.284 0.000 ( +0.0% ) 9.32 9.04 +3.0% 1.836 1.823
2500.33 1.101 1.096 0.005 ( +0.5% ) 21.78 20.52 +6.2% 1.641 1.646
2500.331 1.396 1.396 0.001 ( +0.0% ) 15.99 15.22 +5.0% 1.791 1.791
2500.332 1.328 1.328 0.000 ( +0.0% ) 17.90 16.87 +6.1% 1.748 1.747
2500.401 2.161 2.164 -0.004 ( -0.2% ) 10.32 9.82 +5.0% 1.157 1.162
2500.501 1.726 1.729 -0.003 ( -0.2% ) 16.60 15.12 +9.8% 1.088 1.090
2500.511 1.134 1.137 -0.003 ( -0.3% ) 30.17 25.80 +16.9% 1.349 1.406
2500.601 2.073 2.076 -0.003 ( -0.1% ) 12.47 12.29 +1.5% 1.136 1.155

from Configuration.Eras.Modifier_run3_RPC_cff import run3_RPC

Run3 = cms.ModifierChain(Run2_2018.copyAndExclude([run2_GEM_2017, ctpps_2018, run2_egamma_2018]),
Run3 = cms.ModifierChain(Run2_2018.copyAndExclude([run2_GEM_2017, ctpps_2018, run2_egamma_2018, run2_HLTconditions_2018]),
Copy link
Contributor

Choose a reason for hiding this comment

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

@missirol @Martin-Grunewald could you please confirm that this is ok for HLT in Run3?

Copy link
Contributor

@missirol missirol Jan 10, 2023

Choose a reason for hiding this comment

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

could you please confirm that this is ok for HLT in Run3?

Yes, it is okay for HLT.

To my knowledge, nothing that concerns HLT directly depends on this modifier. It is used in the HLT Offline DQM (FYI: @cms-sw/dqm-l2 (@ckoraka)), e.g.
https://github.com/cms-sw/cmssw/blob/a9b836229a8b2869e3d342b70dd1332e45e0681a/DQMOffline/Trigger/python/TopMonitoring_cff.py

For reference, this change was already discussed in #39082 and #39084 (neither of which was merged, despite what the GitHub labels say).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks, looking at the discussion in #39084 it looks like people actually wanted this change to be included then; not clear why it was reverted in #39119...

@perrotta
Copy link
Contributor

+1

@cmsbuild
Copy link
Contributor

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 be automatically merged.

@cmsbuild cmsbuild merged commit 165f805 into cms-sw:master Jan 11, 2023
@smuzaffar
Copy link
Contributor

smuzaffar commented Jan 11, 2023

@swertz, many workflows (140.xxx series) in latest 13.0.X are failing due to [a]. Looks like this PR has deleted the PhysicsTools/NanoAOD/V10 directory. Can you please look in to this and provide a fix? thanks

[a]

raceback (most recent call last):
  File "/cvmfs/cms-ib.cern.ch/sw/x86_64/nweek-02767/el8_amd64_gcc11/cms/cmssw/CMSSW_13_0_X_2023-01-11-0800/bin/el8_amd64_gcc11/cmsDriver.py", line 51, in <module>
    run()
  File "/cvmfs/cms-ib.cern.ch/sw/x86_64/nweek-02767/el8_amd64_gcc11/cms/cmssw/CMSSW_13_0_X_2023-01-11-0800/bin/el8_amd64_gcc11/cmsDriver.py", line 28, in run
    configBuilder.prepare()
  File "/cvmfs/cms-ib.cern.ch/sw/x86_64/nweek-02767/el8_amd64_gcc11/cms/cmssw/CMSSW_13_0_X_2023-01-11-0800/src/Configuration/Applications/python/ConfigBuilder.py", line 2165, in prepare
    self.addStandardSequences()
  File "/cvmfs/cms-ib.cern.ch/sw/x86_64/nweek-02767/el8_amd64_gcc11/cms/cmssw/CMSSW_13_0_X_2023-01-11-0800/src/Configuration/Applications/python/ConfigBuilder.py", line 790, in addStandardSequences
    getattr(self,"prepare_"+stepName)(stepSpec = '+'.join(stepSpec))
  File "/cvmfs/cms-ib.cern.ch/sw/x86_64/nweek-02767/el8_amd64_gcc11/cms/cmssw/CMSSW_13_0_X_2023-01-11-0800/src/Configuration/Applications/python/ConfigBuilder.py", line 1724, in prepare_NANO
    _,_nanoSeq,_nanoCff = self.loadDefaultOrSpecifiedCFF(stepSpec,self.NANODefaultCFF,self.NANODefaultSeq)
  File "/cvmfs/cms-ib.cern.ch/sw/x86_64/nweek-02767/el8_amd64_gcc11/cms/cmssw/CMSSW_13_0_X_2023-01-11-0800/src/Configuration/Applications/python/ConfigBuilder.py", line 1236, in loadDefaultOrSpecifiedCFF
    l=self.loadAndRemember(_cff)
  File "/cvmfs/cms-ib.cern.ch/sw/x86_64/nweek-02767/el8_amd64_gcc11/cms/cmssw/CMSSW_13_0_X_2023-01-11-0800/src/Configuration/Applications/python/ConfigBuilder.py", line 331, in loadAndRemember
    self.process.load(includeFile)
  File "/cvmfs/cms-ib.cern.ch/sw/x86_64/nweek-02767/el8_amd64_gcc11/cms/cmssw/CMSSW_13_0_X_2023-01-11-0800/src/FWCore/ParameterSet/python/Config.py", line 757, in load
    module = __import__(moduleName)
ModuleNotFoundError: No module named 'PhysicsTools.NanoAOD.V10'

@swertz
Copy link
Contributor Author

swertz commented Jan 11, 2023

Indeed, that directory is removed and I removed the associated nano-specific workflows from https://github.com/cms-sw/cmssw/blob/master/Configuration/PyReleaseValidation/python/relval_nano.py but I didn't notice that this step was making use of it: https://github.com/cms-sw/cmssw/blob/master/Configuration/PyReleaseValidation/python/relval_steps.py#L2800

How come this didn't come up in the PR tests?

I can make a PR ASAP to switch that step to use the default nano configuration instead of the V10 one.

@srimanob
Copy link
Contributor

srimanob commented Jan 11, 2023

We don't have the 2022 data workflow in runTheMatrix,
https://github.com/cms-sw/cmssw/blob/master/Configuration/PyReleaseValidation/scripts/runTheMatrix.py

Maybe we should add one, i.e. 140.106 for JetHT 2022B.

Line to fix, https://github.com/cms-sw/cmssw/blob/master/Configuration/PyReleaseValidation/python/relval_steps.py#L2800

@smuzaffar
Copy link
Contributor

PR test runs a selected number of workflows and I guess non of those workflows were using PhysicsTools/NanoAOD/V10/nano_cff

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.

8 participants