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

exclude HLT-PNET DQM from HIon workflows #39051

Merged
merged 1 commit into from Aug 16, 2022

Conversation

missirol
Copy link
Contributor

@missirol missirol commented Aug 13, 2022

PR description:

After the integration of #38748, the HLT-Validation tests in IBs are failing in 12_5_X.

Afaiu, the issue is that the DQM modules added in #38748 can trigger the execution of modules from the offline reconstruction, to infer the PNET score of offline jets. Since some of those offline-jet collections aren't available in HIon wfs, this leads to exceptions due to missing products in such wfs of the HLT-Validation tests.

This PR provides a fix. If needed, a better solution will be provided by experts in the near future (see discussion at the end of #38748).

Some of the config files introduced in #38748 are re-organised in cfis and cffs for clarity.

No changes are expected in the outputs of PR tests, modulo #39051 (comment).

PR validation:

TSG tests.

If this PR is a backport, please specify the original PR and why you need to backport that PR. If this PR will be backported please specify to which release cycle the backport is meant for:

N/A

@cmsbuild
Copy link
Contributor

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-39051/31544

  • This PR adds an extra 20KB to repository

@cmsbuild
Copy link
Contributor

A new Pull Request was created by @missirol (Marino Missiroli) for master.

It involves the following packages:

  • DQMOffline/Trigger (dqm)

@emanueleusai, @ahmad3213, @cmsbuild, @jfernan2, @pmandrik, @micsucmed, @rvenditti can you please review it and eventually sign? Thanks.
@sscruz, @mtosi, @Fedespring, @HuguesBrun, @jhgoh, @trocino, @cericeci, @rociovilar this is something you requested to watch as well.
@perrotta, @dpiparo, @qliphy, @rappoccio you are the release manager for this.

cms-bot commands are listed here

@missirol
Copy link
Contributor Author

please test

@cmsbuild
Copy link
Contributor

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-575b48/26797/summary.html
COMMIT: fdb9ec2
CMSSW: CMSSW_12_5_X_2022-08-13-1100/el8_amd64_gcc10
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week1/cms-sw/cmssw/39051/26797/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: 2 differences found in the comparisons
  • DQMHistoTests: Total files compared: 51
  • DQMHistoTests: Total histograms compared: 3692476
  • DQMHistoTests: Total failures: 7
  • DQMHistoTests: Total nulls: 1
  • DQMHistoTests: Total successes: 3692446
  • DQMHistoTests: Total skipped: 22
  • DQMHistoTests: Total Missing objects: 0
  • DQMHistoSizes: Histogram memory added: 3.274 KiB( 50 files compared)
  • DQMHistoSizes: changed ( 11634.0,... ): 0.545 KiB HLT/HIG
  • DQMHistoSizes: changed ( 312.0 ): 0.004 KiB MessageLogger/Warnings
  • Checked 212 log files, 49 edm output root files, 51 DQM output files
  • TriggerResults: no differences found

@missirol
Copy link
Contributor Author

@cms-sw/dqm-l2 , please have a look at this PR.

It fixes HLT tests that are currently failing in IBs.

"eff_electron_eta 'Efficiency vs #eta(ele); #eta(ele); efficiency' electron_eta_numerator electron_eta_denominator",
"eff_dilepton_pt 'Efficiency vs p_{T}(ll); p_{T}(ll); efficiency' dilepton_pt_numerator dilepton_pt_denominator",
"eff_dilepton_mass 'Efficiency vs m(ll); m(ll); efficiency' dilepton_mass_numerator dilepton_mass_denominator",
"eff_ht 'Efficiency vs H_{T}; H_{T}; efficiency' ht_numerator ht_denominator",
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This line in master is currently missing a comma, and I guess this fix might lead to a few differences in PR outputs.

"eff_ht 'Efficiency vs H_{T}; H_{T}; efficiency' ht_numerator ht_denominator"

"eff_muon_eta 'Efficiency vs #eta(#mu); #eta(#mu); efficiency' muon_eta_numerator muon_eta_denominator",
"eff_electron_pt 'Efficiency vs p_{T}(ele); p_{T}(ele); efficiency' electron_pt_numerator electron_pt_denominator",
"eff_electron_eta 'Efficiency vs #eta(ele); #eta(ele); efficiency' electron_eta_numerator electron_eta_denominator",
"eff_ht 'Efficiency vs H_{T}; H_{T}; efficiency' ht_numerator ht_denominator",
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This line in master is currently missing a comma, and I guess this fix might lead to a few differences in PR outputs.

"eff_ht 'Efficiency vs H_{T}; H_{T}; efficiency' ht_numerator ht_denominator"

@missirol
Copy link
Contributor Author

@cms-sw/dqm-l2 , kind ping to review this PR.

It is needed to fix some of the HLT tests running in IBs.

@missirol
Copy link
Contributor Author

Another ping to @cms-sw/dqm-l2 to please review this PR.

The developer of the original PR is okay with this fix, see #38748 (comment).

This is needed asap to fix the HLT tests that are failing in IBs.

@emanueleusai
Copy link
Member

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

@perrotta
Copy link
Contributor

+1

@cmsbuild cmsbuild merged commit 3c62ba6 into cms-sw:master Aug 16, 2022
@makortel makortel mentioned this pull request Aug 16, 2022
@missirol missirol deleted the devel_fixPNETDQMForHIonTest branch August 16, 2022 14:50
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

4 participants