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.

Sign up for GitHub

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

Jump to bottom

Improvements and fixes for the BPH-trigger offline DQM code #23738

Merged
merged 4 commits into from Jul 12, 2018
Merged

Improvements and fixes for the BPH-trigger offline DQM code #23738

merged 4 commits into from Jul 12, 2018

Conversation

aboletti
Copy link
Contributor

@aboletti aboletti commented Jul 3, 2018

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)

@aboletti
Unique commit including several fixes, the main ones: fixed trigger-m…
04496f4 
…atching function, new correction for prescales, new structure of the code (numerator-histo filling nested in denominator-histo filling section), general fixes of selection requirements, removal of some unused efficiency modules
@aboletti
fixed missing prescale correction in one numerator filling
7bb915c
@aboletti
Moved the Prescale function to be a double variable, avoiding the unn…
098d34d 
…eded approximation
@cmsbuild cmsbuild added this to the CMSSW_10_2_X milestone Jul 3, 2018
@cmsbuild
Copy link
Contributor

cmsbuild commented Jul 3, 2018

The code-checks are being triggered in jenkins.

@cmsbuild
Copy link
Contributor

cmsbuild commented Jul 3, 2018

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-23738/5408

@cmsbuild
Copy link
Contributor

cmsbuild commented Jul 3, 2018

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

It involves the following packages:

DQMOffline/Trigger

@vazzolini, @kmaeshima, @dmitrijus, @cmsbuild, @jfernan2, @vanbesien can you please review it and eventually sign? Thanks.
@battibass, @mtosi, @jhgoh, @calderona, @HuguesBrun, @drkovalskyi, @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

jfernan2 commented Jul 3, 2018

please test

@cmsbuild
Copy link
Contributor

cmsbuild commented Jul 3, 2018 •
edited

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

@cmsbuild
Copy link
Contributor

cmsbuild commented Jul 3, 2018

-1

Tested at: 098d34d

You can see the results of the tests here:
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-23738/28991/summary.html

I found follow errors while testing this PR

Failed tests: RelVals

  • RelVals:

When I ran the RelVals I found an error in the following worklfows:
4.53 step3

runTheMatrix-results/4.53_RunPhoton2012B+RunPhoton2012B+HLTD+RECODR1reHLT+HARVESTDR1reHLT/step3_RunPhoton2012B+RunPhoton2012B+HLTD+RECODR1reHLT+HARVESTDR1reHLT.log
136.731 step3 
runTheMatrix-results/136.731_RunSinglePh2016B+RunSinglePh2016B+HLTDR2_2016+RECODR2_2016reHLT_skimSinglePh_HIPM+HARVESTDR2/step3_RunSinglePh2016B+RunSinglePh2016B+HLTDR2_2016+RECODR2_2016reHLT_skimSinglePh_HIPM+HARVESTDR2.log
9.0 step3 
runTheMatrix-results/9.0_Higgs200ChargedTaus+Higgs200ChargedTaus+DIGI+RECO+HARVEST/step3_Higgs200ChargedTaus+Higgs200ChargedTaus+DIGI+RECO+HARVEST.log
25.0 step3 
runTheMatrix-results/25.0_TTbar+TTbar+DIGI+RECOAlCaCalo+HARVEST+ALCATT/step3_TTbar+TTbar+DIGI+RECOAlCaCalo+HARVEST+ALCATT.log
136.788 step3 
runTheMatrix-results/136.788_RunSinglePh2017B+RunSinglePh2017B+HLTDR2_2017+RECODR2_2017reHLT_skimSinglePh_Prompt+HARVEST2017/step3_RunSinglePh2017B+RunSinglePh2017B+HLTDR2_2017+RECODR2_2017reHLT_skimSinglePh_Prompt+HARVEST2017.log
1306.0 step3 
runTheMatrix-results/1306.0_SingleMuPt1_UP15+SingleMuPt1_UP15+DIGIUP15+RECOUP15+HARVESTUP15/step3_SingleMuPt1_UP15+SingleMuPt1_UP15+DIGIUP15+RECOUP15+HARVESTUP15.log
136.85 step3 
runTheMatrix-results/136.85_RunEGamma2018A+RunEGamma2018A+HLTDR2_2018+RECODR2_2018reHLT_skimEGamma_Prompt_L1TEgDQM+HARVEST2018_L1TEgDQM/step3_RunEGamma2018A+RunEGamma2018A+HLTDR2_2018+RECODR2_2018reHLT_skimEGamma_Prompt_L1TEgDQM+HARVEST2018_L1TEgDQM.log
1000.0 step2 
runTheMatrix-results/1000.0_RunMinBias2011A+RunMinBias2011A+TIER0+SKIMD+HARVESTDfst2+ALCASPLIT/step2_RunMinBias2011A+RunMinBias2011A+TIER0+SKIMD+HARVESTDfst2+ALCASPLIT.log
1001.0 step2 
runTheMatrix-results/1001.0_RunMinBias2011A+RunMinBias2011A+TIER0EXP+ALCAEXP+ALCAHARVDSIPIXELCALRUN1+ALCAHARVD1+ALCAHARVD2+ALCAHARVD3+ALCAHARVD4+ALCAHARVD5/step2_RunMinBias2011A+RunMinBias2011A+TIER0EXP+ALCAEXP+ALCAHARVDSIPIXELCALRUN1+ALCAHARVD1+ALCAHARVD2+ALCAHARVD3+ALCAHARVD4+ALCAHARVD5.log
1330.0 step3 
runTheMatrix-results/1330.0_ZMM_13+ZMM_13+DIGIUP15+RECOUP15_L1TMuDQM+HARVESTUP15_L1TMuDQM/step3_ZMM_13+ZMM_13+DIGIUP15+RECOUP15_L1TMuDQM+HARVESTUP15_L1TMuDQM.log
10042.0 step3 
runTheMatrix-results/10042.0_ZMM_13+ZMM_13TeV_TuneCUETP8M1_2017_GenSimFull+DigiFull_2017+RecoFull_2017+HARVESTFull_2017+ALCAFull_2017/step3_ZMM_13+ZMM_13TeV_TuneCUETP8M1_2017_GenSimFull+DigiFull_2017+RecoFull_2017+HARVESTFull_2017+ALCAFull_2017.log
10024.0 step3 
runTheMatrix-results/10024.0_TTbar_13+TTbar_13TeV_TuneCUETP8M1_2017_GenSimFull+DigiFull_2017+RecoFull_2017+HARVESTFull_2017+ALCAFull_2017/step3_TTbar_13+TTbar_13TeV_TuneCUETP8M1_2017_GenSimFull+DigiFull_2017+RecoFull_2017+HARVESTFull_2017+ALCAFull_2017.log
25202.0 step3 
runTheMatrix-results/25202.0_TTbar_13+TTbar_13+DIGIUP15_PU25+RECOUP15_PU25+HARVESTUP15_PU25/step3_TTbar_13+TTbar_13+DIGIUP15_PU25+RECOUP15_PU25+HARVESTUP15_PU25.log
11624.0 step3 
runTheMatrix-results/11624.0_TTbar_13+TTbar_13TeV_TuneCUETP8M1_2019_GenSimFull+DigiFull_2019+RecoFull_2019+HARVESTFull_2019+ALCAFull_2019/step3_TTbar_13+TTbar_13TeV_TuneCUETP8M1_2019_GenSimFull+DigiFull_2019+RecoFull_2019+HARVESTFull_2019+ALCAFull_2019.log
10824.0 step3 
runTheMatrix-results/10824.0_TTbar_13+TTbar_13TeV_TuneCUETP8M1_2018_GenSimFull+DigiFull_2018+RecoFull_2018+HARVESTFull_2018+ALCAFull_2018/step3_TTbar_13+TTbar_13TeV_TuneCUETP8M1_2018_GenSimFull+DigiFull_2018+RecoFull_2018+HARVESTFull_2018+ALCAFull_2018.log
10224.0 step3 
runTheMatrix-results/10224.0_TTbar_13+TTbar_13TeV_TuneCUETP8M1_2017PU_GenSimFull+DigiFullPU_2017PU+RecoFullPU_2017PU+HARVESTFullPU_2017PU/step3_TTbar_13+TTbar_13TeV_TuneCUETP8M1_2017PU_GenSimFull+DigiFullPU_2017PU+RecoFullPU_2017PU+HARVESTFullPU_2017PU.log
250202.181 step4 
runTheMatrix-results/250202.181_TTbar_13UP18+TTbar_13UP18+PREMIXUP18_PU25+DIGIPRMXLOCALUP18_PU25+RECOPRMXUP18_PU25+HARVESTUP18_PU25/step4_TTbar_13UP18+TTbar_13UP18+PREMIXUP18_PU25+DIGIPRMXLOCALUP18_PU25+RECOPRMXUP18_PU25+HARVESTUP18_PU25.log

@cmsbuild
Copy link
Contributor

cmsbuild commented Jul 3, 2018

Comparison not run due to runTheMatrix errors (RelVals and Igprof tests were also skipped)

@jfernan2
Copy link
Contributor

jfernan2 commented Jul 3, 2018

@aboletti Your PR does not run[1], please test your changes producing a valid root file and uploading it on a DQM test GUI before proceed. Thanks.

[1] An exception of category 'Configuration' occurred while
[0] Constructing the EventProcessor
[1] Validating configuration of module: class=BPHMonitor label='DM2_Jpsi_addTkMuTkMu_Phi'
Exception Message:
Illegal parameter found in configuration. The parameter is named:
'verbosityLevel'
You could be trying to use a parameter name that is not
allowed for this plugin or it could be misspelled.

@cmsbuild
Copy link
Contributor

cmsbuild commented Jul 11, 2018 •
edited

The tests are being triggered in jenkins.
https://cmssdt.cern.ch/jenkins/job/ib-any-integration/29073/console Started: 2018/07/11 11:37
https://cmssdt.cern.ch/jenkins/job/ib-any-integration/29074/console Started: 2018/07/11 13:44

@cmsbuild
Copy link
Contributor

There was an issue with git-cms-merge-topic you can see the log here:
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-23738/29074/git-merge-result

@fabiocos
Copy link
Contributor

@smuzaffar I think the bot got stuck on this test...

@fabiocos
Copy link
Contributor

please test

@cmsbuild
Copy link
Contributor

cmsbuild commented Jul 12, 2018 •
edited

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

@cmsbuild
Copy link
Contributor

+1
Tested at: 51d6fc6
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-23738/29086/summary.html

@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-23738/29086/summary.html

Comparison Summary:

  • No significant changes to the logs found
  • Reco comparison results: 4 differences found in the comparisons
  • DQMHistoTests: Total files compared: 31
  • DQMHistoTests: Total histograms compared: 2886290
  • DQMHistoTests: Total failures: 14
  • DQMHistoTests: Total nulls: 0
  • DQMHistoTests: Total successes: 2886086
  • 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

@fabiocos
Copy link
Contributor

+1

@cmsbuild cmsbuild merged commit d97b988 into cms-sw:master Jul 12, 2018
@Dr15Jones
Copy link
Contributor

I believe this pull request is causing the crash reported here #23787

@fabiocos
Copy link
Contributor

@Dr15Jones yes, it is, verified with a merge of this PR only on top of yesterday's IB. I am reverting it

@fabiocos fabiocos mentioned this pull request Jul 13, 2018
Merged
@aboletti aboletti mentioned this pull request Jul 17, 2018
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
code-checks-approved comparison-available dqm-approved fully-signed orp-approved tests-approved
Projects
None yet
Milestone
CMSSW_10_2_X
Development

Successfully merging this pull request may close these issues.

None yet
6 participants
@aboletti @cmsbuild @jfernan2 @anigamova @fabiocos @Dr15Jones