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

Improvements and fixes for the BPH-trigger offline DQM code (v2) #23807

Conversation

aboletti
Copy link
Contributor

Same changes that were merged in #23738 and reverted in #23788 after a crash observed in 136.86 (see #23787 ), plus a sanity check to avoid that crash to happen and few additional minor fixes.
The present code has been successfully tested, also on 136.86

Here, for reference, the description of the original PR, listing the major changes to the code:

In this pull-request many features of the offline DQM code used for BPH triggers have been fixed and optimised, and few more functionalities have been added.
Here a list of the major changes:

  • New general structure of the code: for each efficiency type (defined as the "cases" in the big "switch") the numerator histos are filled in a nested environment just after the denominator filling, instead of defining two separate environments with specific selection criteria for each of them.
  • Fixed trigger matching feature: the bool BPHMonitor::matchToTrigger function is now using the HLTConfigProvider method to access the trigger-object collections.
  • New correction for prescaled triggers: while using the reference method to produce efficiencies, if the numerator path is prescaled it would flatly decrease the resulting efficiency by that prescale value. By filling the numerator histograms with weighted events, according to that prescale, the reduction effect is corrected.
  • Better tuning of the offline selections (especially the dimuon invariant masses), to select the correct regions where to compute the efficiency. This helps in some modules to select a pure region for the signal, in other modules (based on background events) to select the appropriate kinematic region.
  • Customised and optimised binning: to reduce the global number of bins, especially where they are not needed, the binning of the histograms has been refined
  • Removal of few meaningless efficiencies: some modules defined in the list of the cff file (devoted to the L1 efficiency of dR(MuMu) cut in 2mu+trk triggers) have been removed, since their efficiency definition is not correct (using at denominator a 2mu trigger, they will always be biased by the inefficiency of the track reconstruction at HLT)

@cmsbuild
Copy link
Contributor

The code-checks are being triggered in jenkins.

@cmsbuild
Copy link
Contributor

@cmsbuild
Copy link
Contributor

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

It involves the following packages:

DQMOffline/Trigger

@andrius-k, @kmaeshima, @schneiml, @dmitrijus, @cmsbuild, @jfernan2 can you please review it and eventually sign? Thanks.
@battibass, @mtosi, @jhgoh, @calderona, @HuguesBrun, @trocino, @folguera, @rociovilar this is something you requested to watch as well.
@davidlange6, @slava77, @fabiocos you are the release manager for this.

cms-bot commands are listed here

@jfernan2
Copy link
Contributor

please test

@cmsbuild
Copy link
Contributor

cmsbuild commented Jul 17, 2018

The tests are being triggered in jenkins.
https://cmssdt.cern.ch/jenkins/job/ib-any-integration/29136/console Started: 2018/07/17 10:41

@cmsbuild
Copy link
Contributor

@cmsbuild
Copy link
Contributor

Comparison job queued.

@cmsbuild
Copy link
Contributor

Comparison is ready
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-23807/29136/summary.html

Comparison Summary:

  • No significant changes to the logs found
  • Reco comparison results: 2 differences found in the comparisons
  • DQMHistoTests: Total files compared: 31
  • DQMHistoTests: Total histograms compared: 2886290
  • DQMHistoTests: Total failures: 2
  • DQMHistoTests: Total nulls: 0
  • DQMHistoTests: Total successes: 2886098
  • DQMHistoTests: Total skipped: 190
  • DQMHistoTests: Total Missing objects: 0
  • DQMHistoSizes: Histogram memory added: -1451.104 KiB( 30 files compared)
  • DQMHistoSizes: changed ( 25202.0,... ): -90.694 KiB HLT/BPH
  • Checked 128 log files, 14 edm output root files, 31 DQM output files

@fabiocos
Copy link
Contributor

please test 136.86

@fabiocos
Copy link
Contributor

please test workflow 136.86

@cmsbuild
Copy link
Contributor

cmsbuild commented Jul 18, 2018

The tests are being triggered in jenkins.
https://cmssdt.cern.ch/jenkins/job/ib-any-integration/29172/console Started: 2018/07/18 20:42

@cmsbuild
Copy link
Contributor

@cmsbuild
Copy link
Contributor

Comparison job queued.

@cmsbuild
Copy link
Contributor

Comparison is ready
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-23807/29172/summary.html

@slava77 comparisons for the following workflows were not done due to missing matrix map:

  • /build/cmsbld/jenkins/workspace/compare-root-files-short-matrix/results/JR-comparison/PR-23807/136.86_RunCharmonium2018A+RunCharmonium2018A+HLTDR2_2018+RECODR2_2018reHLT_skimCharmonium_Prompt+HARVEST2018

Comparison Summary:

  • No significant changes to the logs found
  • Reco comparison results: 0 differences found in the comparisons
  • DQMHistoTests: Total files compared: 31
  • DQMHistoTests: Total histograms compared: 2886290
  • DQMHistoTests: Total failures: 1
  • DQMHistoTests: Total nulls: 0
  • DQMHistoTests: Total successes: 2886099
  • DQMHistoTests: Total skipped: 190
  • DQMHistoTests: Total Missing objects: 0
  • DQMHistoSizes: Histogram memory added: -1451.104 KiB( 30 files compared)
  • DQMHistoSizes: changed ( 136.85,... ): -90.694 KiB HLT/BPH
  • Checked 128 log files, 14 edm output root files, 31 DQM output files

@andrius-k
Copy link

+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. @davidlange6, @slava77, @smuzaffar, @fabiocos (and backports should be raised in the release meeting by the corresponding L2)

@fabiocos
Copy link
Contributor

+1

@cmsbuild cmsbuild merged commit 4237fed into cms-sw:master Jul 19, 2018
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

6 participants