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

[NanoAOD] Adding correct Iso filter name (without cut values) for Run3 #39230

Merged
merged 2 commits into from
Oct 10, 2022

Conversation

wonpoint4
Copy link

PR description:

Adding correct Iso filter name (without cut values) for Run3
Relevant JIRA ticket : https://its.cern.ch/jira/browse/CMSHLT-2312

And I was wondering that I have to make backport PRs to the both of 124X and 125X.

@cmsbuild
Copy link
Contributor

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-39230/31855

  • This PR adds an extra 16KB to repository

@cmsbuild
Copy link
Contributor

A new Pull Request was created by @wonpoint4 (Won Jun) for master.

It involves the following packages:

  • PhysicsTools/NanoAOD (xpog)

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

@perrotta
Copy link
Contributor

perrotta commented Sep 5, 2022

please test

@cmsbuild
Copy link
Contributor

cmsbuild commented Sep 5, 2022

-1

Failed Tests: UnitTests RelVals RelVals-INPUT
Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-50442b/27329/summary.html
COMMIT: 994c5b6
CMSSW: CMSSW_12_6_X_2022-09-04-2300/el8_amd64_gcc10
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week1/cms-sw/cmssw/39230/27329/install.sh to create a dev area with all the needed externals and cmssw changes.

Unit Tests

I found errors in the following unit tests:

---> test runtestPhysicsToolsNanoAOD had ERRORS

RelVals

----- Begin Fatal Exception 05-Sep-2022 09:10:43 CEST-----------------------
An exception of category 'Configuration' occurred while
   [0] Constructing the EventProcessor
   [1] Constructing module: class=TriggerObjectTableProducer label='triggerObjectTable'
Exception Message:
Expression parser error:Missing close parenthesis. (char 153)
----- End Fatal Exception -------------------------------------------------
----- Begin Fatal Exception 05-Sep-2022 09:15:21 CEST-----------------------
An exception of category 'Configuration' occurred while
   [0] Constructing the EventProcessor
   [1] Constructing module: class=TriggerObjectTableProducer label='triggerObjectTable'
Exception Message:
Expression parser error:Missing close parenthesis. (char 153)
----- End Fatal Exception -------------------------------------------------
----- Begin Fatal Exception 05-Sep-2022 09:30:29 CEST-----------------------
An exception of category 'Configuration' occurred while
   [0] Constructing the EventProcessor
   [1] Constructing module: class=TriggerObjectTableProducer label='triggerObjectTable'
Exception Message:
Expression parser error:Missing close parenthesis. (char 153)
----- End Fatal Exception -------------------------------------------------
Expand to see more relval errors ...

RelVals-INPUT

  • 136.7952136.7952_RunJetHT2017C_94Xv2NanoAODINPUT+RunJetHT2017C_94Xv2NanoAODINPUT+NANOEDM2017_94XMiniAODv2+HARVESTNANOAOD2017_94XMiniAODv2/step2_RunJetHT2017C_94Xv2NanoAODINPUT+RunJetHT2017C_94Xv2NanoAODINPUT+NANOEDM2017_94XMiniAODv2+HARVESTNANOAOD2017_94XMiniAODv2.log
  • 136.8523136.8523_RunJetHT2018C_nanoULremini+RunJetHT2018C_nanoULremini+NANOEDM2018_106Xv2+HARVESTNANOAOD2018_106Xv2/step2_RunJetHT2018C_nanoULremini+RunJetHT2018C_nanoULremini+NANOEDM2018_106Xv2+HARVESTNANOAOD2018_106Xv2.log
  • 136.8521136.8521_RunJetHT2018A_nano+RunJetHT2018A_nano+NANOEDM2018_102Xv1+HARVESTNANOAOD2018_102Xv1/step2_RunJetHT2018A_nano+RunJetHT2018A_nano+NANOEDM2018_102Xv1+HARVESTNANOAOD2018_102Xv1.log
Expand to see more relval errors ...

@cmsbuild
Copy link
Contributor

cmsbuild commented Sep 6, 2022

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-39230/32012

@cmsbuild
Copy link
Contributor

cmsbuild commented Sep 6, 2022

Pull request #39230 was updated. @cmsbuild, @swertz, @vlimant can you please check and sign again.

@vlimant
Copy link
Contributor

vlimant commented Sep 6, 2022

@vlimant
Copy link
Contributor

vlimant commented Sep 14, 2022

please test

@cmsbuild
Copy link
Contributor

-1

Failed Tests: RelVals-INPUT
Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-50442b/27536/summary.html
COMMIT: 5d1b6c6
CMSSW: CMSSW_12_6_X_2022-09-13-2300/el8_amd64_gcc10
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week0/cms-sw/cmssw/39230/27536/install.sh to create a dev area with all the needed externals and cmssw changes.

RelVals-INPUT

The relvals timed out after 4 hours.

Comparison Summary

Summary:

  • No significant changes to the logs found
  • Reco comparison results: 10 differences found in the comparisons
  • DQMHistoTests: Total files compared: 51
  • DQMHistoTests: Total histograms compared: 3618326
  • DQMHistoTests: Total failures: 66
  • DQMHistoTests: Total nulls: 0
  • DQMHistoTests: Total successes: 3618238
  • DQMHistoTests: Total skipped: 22
  • DQMHistoTests: Total Missing objects: 0
  • DQMHistoSizes: Histogram memory added: 0.0 KiB( 50 files compared)
  • Checked 212 log files, 49 edm output root files, 51 DQM output files
  • TriggerResults: no differences found

@wonpoint4
Copy link
Author

Dear experts,
I couldn't understand the origin of error (The relvals timed out after 4 hours.)
Could you give me an advice or suggestion here?

@perrotta
Copy link
Contributor

please test

@cmsbuild
Copy link
Contributor

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-50442b/27750/summary.html
COMMIT: 5d1b6c6
CMSSW: CMSSW_12_6_X_2022-09-22-2300/el8_amd64_gcc10
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week1/cms-sw/cmssw/39230/27750/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: 8 differences found in the comparisons
  • DQMHistoTests: Total files compared: 51
  • DQMHistoTests: Total histograms compared: 3624368
  • DQMHistoTests: Total failures: 60
  • DQMHistoTests: Total nulls: 0
  • DQMHistoTests: Total successes: 3624286
  • DQMHistoTests: Total skipped: 22
  • DQMHistoTests: Total Missing objects: 0
  • DQMHistoSizes: Histogram memory added: 0.0 KiB( 50 files compared)
  • Checked 212 log files, 49 edm output root files, 51 DQM output files
  • TriggerResults: no differences found

@swertz
Copy link
Contributor

swertz commented Oct 10, 2022

+xpog

And I was wondering that I have to make backport PRs to the both of 124X and 125X.

That won't be necessary, 124X is closed for Nano and there will be no re-nano in 125X.

@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 now be reviewed by the release team before it's merged. @perrotta, @dpiparo, @rappoccio (and backports should be raised in the release meeting by the corresponding L2)

@rappoccio
Copy link
Contributor

+1

@rappoccio rappoccio merged commit c9b6f18 into cms-sw:master Oct 10, 2022
@silviodonato
Copy link
Contributor

Related to https://its.cern.ch/jira/browse/CMSHLT-2312

And I was wondering that I have to make backport PRs to the both of 124X and 125X.

That won't be necessary, 124X is closed for Nano and there will be no re-nano in 125X.

This creates problem about the maintenance of the PromptNano AOD when the HLT menu changes during the datataking. We need a larger discussion about this.

In any case, @wonpoint4 is your PR compatible with the HLT menu both before and after CMSHLT-2312, or only after CMSHLT-2312 (ie. it should be compatible with both HLT v1.2 and HLT v1.3)

@silviodonato
Copy link
Contributor

cc: @jordan-martins

@wonpoint4
Copy link
Author

In any case, @wonpoint4 is your PR compatible with the HLT menu both before and after CMSHLT-2312, or only after CMSHLT-2312 (ie. it should be compatible with both HLT v1.2 and HLT v1.3)

Yes, this PR is supposed to be compatible with HLT menu before/after CMSHLT-2312, like compatible with menu during the whole Run3, and even the Run2.

@swertz
Copy link
Contributor

swertz commented Oct 27, 2022

And I was wondering that I have to make backport PRs to the both of 124X and 125X.

We have now decided that we would backport changes to the trigger object filter names to 124X to allow prompt nano to pick up those changes, even if it breaks compatibility with Run2 or existing V10 samples. Can you please make a backport to 124X @wonpoint4 ?

In principle the same changes should then also be applied to the frozen V10 config in master: https://github.com/cms-sw/cmssw/blob/master/PhysicsTools/NanoAOD/python/V10/triggerObjects_cff.py

@swertz
Copy link
Contributor

swertz commented Nov 2, 2022

@wonpoint4 with the backport merged we now have an inconsistency between the nanoV10 in 124X and the frozen V10 config in master. Can I ask you to make another PR to master to apply the same changes to https://github.com/cms-sw/cmssw/blob/master/PhysicsTools/NanoAOD/python/V10/triggerObjects_cff.py ?

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.

7 participants