[13_3_X] Removal of StringCutObjectSelector from Muon trigger DQM #42348

caruta · 2023-07-25T07:13:48Z

PR description:

This PR removes the "StringCutObjectSelector" from the Muon trigger DQM, with the aim of solving the memory issues faced during the 2022 rereco.
The cuts on the reco muons and trigger objects are now applied explicitly, since they are few and identical for all trigger paths.

Most of the changes are in HLTMuonMatchAndPlot.cc and in HLTMuonMatchAndPlot.h to implement the new way of applying cuts on the muon and trigger objects
The configuration files HLTMuonOfflineAnalyzer_cfi.py and HLTMuonOfflineAnalyzer_cff.py have also been modified accordingly.

This PR will be backported down to the 124X release (used for the 2022 rereco).

PR validation:

Tested running the DQM module on some of the latest RelVal, and checked the agreement of the distributions. Also ran runTheMatrix.py -l limited -i all --ibeos and no failures were seen.

cmsbuild · 2023-07-25T07:21:50Z

-code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-42348/36370

This PR adds an extra 28KB to repository

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

code-format:
https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-42348/36370/code-format.patch
e.g. curl -k https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-42348/36370/code-format.patch | patch -p1
You can also run scram build code-format to apply code format directly

cmsbuild · 2023-07-25T08:05:17Z

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-42348/36372

This PR adds an extra 28KB to repository

cmsbuild · 2023-07-25T08:05:44Z

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

It involves the following packages:

DQMOffline/Trigger (dqm)

@nothingface0, @emanueleusai, @cmsbuild, @pmandrik, @syuvivida, @tjavaid, @micsucmed, @rvenditti can you please review it and eventually sign? Thanks.
@mtosi, @Fedespring, @missirol, @HuguesBrun, @jhgoh, @trocino, @cericeci, @rociovilar 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

emanueleusai · 2023-07-25T09:06:50Z

Please test

cmsbuild · 2023-07-25T11:34:17Z

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-fce6b2/33866/summary.html
COMMIT: 1b8c1d1
CMSSW: CMSSW_13_3_X_2023-07-24-2300/el8_amd64_gcc11
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week1/cms-sw/cmssw/42348/33866/install.sh to create a dev area with all the needed externals and cmssw changes.

Comparison Summary

There are some workflows for which there are errors in the baseline:
14234.0 step 2
The results for the comparisons for these workflows could be incomplete
This means most likely that the IB is having errors in the relvals.The error does NOT come from this pull request

Summary:

You potentially removed 10 lines from the logs
Reco comparison results: 4 differences found in the comparisons
DQMHistoTests: Total files compared: 48
DQMHistoTests: Total histograms compared: 3150117
DQMHistoTests: Total failures: 177
DQMHistoTests: Total nulls: 0
DQMHistoTests: Total successes: 3149918
DQMHistoTests: Total skipped: 22
DQMHistoTests: Total Missing objects: 0
DQMHistoSizes: Histogram memory added: 0.0 KiB( 47 files compared)
Checked 207 log files, 159 edm output root files, 48 DQM output files
TriggerResults: no differences found

DQMOffline/Trigger/src/HLTMuonMatchAndPlot.cc

Dr15Jones · 2023-07-25T12:28:29Z

Please remove commented out code.

cmsbuild · 2023-07-25T13:10:51Z

-code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-42348/36382

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)

code-format:
https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-42348/36382/code-format.patch
e.g. curl -k https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-42348/36382/code-format.patch | patch -p1
You can also run scram build code-format to apply code format directly

perrotta · 2023-07-28T10:05:30Z

Please squash all commits into one when you are done with the updates

@perrotta let me know if you're fine with the last commits, and if yes I'll proceed to propagate them to the other PRs for the back-porting. Thanks!

Thank you @caruta
The latest uodates are fine with me
It would be nice if you could further squash the 15 commits into a single one

JanFSchulte · 2023-07-28T12:10:33Z

Hi @caruta AFAIK the module was using ~150MB of memory before the fix according to studies shown to us by PPD. So the reduction you are seeing are in the same ballpark. Even a bit larger, but I assume there's fluctuations depending on what exactly is run on what kind of events. From that I would conclude that we can be pretty confident that your changes fix the issue and this can be merged. Thanks!

slava77 · 2023-07-28T12:30:28Z

I'm not sure this is enough and/or conclusive, but please let me know if you have other suggestions to test this!

I do not see RSS values, only virtual memory size . AFAIK, virtual size is not limiting the job execution.

JanFSchulte · 2023-07-28T12:37:47Z

I guess then we should just enable profiling for the PR tests?

slava77 · 2023-07-28T12:50:14Z

I guess then we should just enable profiling for the PR tests?

I'd leave it to @perrotta to decide; my past experience with the enable profiling is that they may slow down the test cycle to 24hrs and I'm not sure if the memory tests are working now.

perrotta · 2023-07-29T15:34:32Z

I guess then we should just enable profiling for the PR tests?

I'd leave it to @perrotta to decide; my past experience with the enable profiling is that they may slow down the test cycle to 24hrs and I'm not sure if the memory tests are working now.

We are in the week-end, and we can afford spending 24h for these tests, if we want to check it. Let do so.
In general, it can be much faster to run igprof privately on a relevant workflow and report here the result.
In any case, before merging the 15 commits must be squeezed into one.

perrotta · 2023-07-29T15:34:40Z

enable profiling

perrotta · 2023-07-29T15:35:13Z

please test

cmsbuild · 2023-07-29T19:29:43Z

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-fce6b2/33970/summary.html
COMMIT: 10e3032
CMSSW: CMSSW_13_3_X_2023-07-29-1100/el8_amd64_gcc11
Additional Tests: PROFILING
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week1/cms-sw/cmssw/42348/33970/install.sh to create a dev area with all the needed externals and cmssw changes.

Comparison Summary

Summary:

You potentially added 3 lines to the logs
Reco comparison results: 3644 differences found in the comparisons
DQMHistoTests: Total files compared: 48
DQMHistoTests: Total histograms compared: 3150821
DQMHistoTests: Total failures: 4319
DQMHistoTests: Total nulls: 55
DQMHistoTests: Total successes: 3146425
DQMHistoTests: Total skipped: 22
DQMHistoTests: Total Missing objects: 0
DQMHistoSizes: Histogram memory added: 0.0 KiB( 47 files compared)
Checked 207 log files, 159 edm output root files, 48 DQM output files
TriggerResults: found differences in 1 / 46 workflows

slava77 · 2023-07-31T17:37:05Z

Additional Tests: PROFILING

looking at igprofMEM_GC_step3 in 136.889 apparently the memory profiling does not work:
there are only 180 MB in MEM_LIVE.
I vaguely recall that this is an issue that appeared in EL8; and I guess there is no fix.

cmsbuild · 2023-07-31T18:05:36Z

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-42348/36437

This PR adds an extra 12KB to repository

cmsbuild · 2023-07-31T18:06:05Z

Pull request #42348 was updated. @nothingface0, @emanueleusai, @cmsbuild, @pmandrik, @syuvivida, @tjavaid, @micsucmed, @rvenditti can you please check and sign again.

caruta · 2023-08-01T08:42:11Z

Sorry, my bad: trying to make the squash working I've messed up things...
I can open a new PR with one clean commit with all the changes. Let me know if you agree! (@perrotta)

perrotta · 2023-08-01T10:02:36Z

Sorry, my bad: trying to make the squash working I've messed up things... I can open a new PR with one clean commit with all the changes. Let me know if you agree! (@perrotta)

Perfect, thank you @caruta (and sorry for the extra work, but clean commits will definitely help future developers of the package). Please go ahead

caruta · 2023-08-01T12:35:03Z

Sorry, my bad: trying to make the squash working I've messed up things... I can open a new PR with one clean commit with all the changes. Let me know if you agree! (@perrotta)

Perfect, thank you @caruta (and sorry for the extra work, but clean commits will definitely help future developers of the package). Please go ahead

A new PR has been created: 42437.

cmsbuild added this to the CMSSW_13_3_X milestone Jul 25, 2023

cmsbuild added dqm-pending pending-signatures tests-pending orp-pending code-checks-pending labels Jul 25, 2023

cmsbuild added code-checks-rejected code-checks-pending and removed code-checks-pending code-checks-rejected labels Jul 25, 2023

cmsbuild added code-checks-approved and removed code-checks-pending labels Jul 25, 2023

caruta marked this pull request as ready for review July 25, 2023 08:41

cmsbuild added tests-started and removed tests-pending labels Jul 25, 2023

cmsbuild added tests-approved and removed tests-started labels Jul 25, 2023

Dr15Jones suggested changes Jul 25, 2023

View reviewed changes

cmsbuild added tests-pending code-checks-pending and removed tests-approved code-checks-approved labels Jul 25, 2023

cmsbuild removed the code-checks-pending label Jul 25, 2023

caruta closed this Jul 28, 2023

caruta reopened this Jul 28, 2023

cmsbuild added tests-started and removed tests-pending labels Jul 29, 2023

cmsbuild added tests-approved and removed tests-started labels Jul 29, 2023

cmsbuild added tests-pending code-checks-pending and removed tests-approved code-checks-approved labels Jul 31, 2023

cmsbuild added code-checks-approved and removed code-checks-pending labels Jul 31, 2023

caruta closed this Aug 1, 2023

caruta force-pushed the master branch from 39290d9 to a8b2114 Compare August 1, 2023 08:33

caruta mentioned this pull request Aug 1, 2023

Removal of StringCutObjectSelector from Muon trigger DQM #42437

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[13_3_X] Removal of StringCutObjectSelector from Muon trigger DQM #42348

[13_3_X] Removal of StringCutObjectSelector from Muon trigger DQM #42348

caruta commented Jul 25, 2023

cmsbuild commented Jul 25, 2023

cmsbuild commented Jul 25, 2023

cmsbuild commented Jul 25, 2023

emanueleusai commented Jul 25, 2023

cmsbuild commented Jul 25, 2023

Dr15Jones commented Jul 25, 2023

cmsbuild commented Jul 25, 2023

perrotta commented Jul 28, 2023

JanFSchulte commented Jul 28, 2023

slava77 commented Jul 28, 2023

JanFSchulte commented Jul 28, 2023

slava77 commented Jul 28, 2023

perrotta commented Jul 29, 2023

perrotta commented Jul 29, 2023

perrotta commented Jul 29, 2023

cmsbuild commented Jul 29, 2023

slava77 commented Jul 31, 2023

cmsbuild commented Jul 31, 2023

cmsbuild commented Jul 31, 2023

caruta commented Aug 1, 2023

perrotta commented Aug 1, 2023 •

edited

Loading

caruta commented Aug 1, 2023

[13_3_X] Removal of StringCutObjectSelector from Muon trigger DQM #42348

[13_3_X] Removal of StringCutObjectSelector from Muon trigger DQM #42348

Conversation

caruta commented Jul 25, 2023

PR description:

PR validation:

cmsbuild commented Jul 25, 2023

cmsbuild commented Jul 25, 2023

cmsbuild commented Jul 25, 2023

emanueleusai commented Jul 25, 2023

cmsbuild commented Jul 25, 2023

Comparison Summary

Dr15Jones commented Jul 25, 2023

cmsbuild commented Jul 25, 2023

perrotta commented Jul 28, 2023

JanFSchulte commented Jul 28, 2023

slava77 commented Jul 28, 2023

JanFSchulte commented Jul 28, 2023

slava77 commented Jul 28, 2023

perrotta commented Jul 29, 2023

perrotta commented Jul 29, 2023

perrotta commented Jul 29, 2023

cmsbuild commented Jul 29, 2023

Comparison Summary

slava77 commented Jul 31, 2023

cmsbuild commented Jul 31, 2023

cmsbuild commented Jul 31, 2023

caruta commented Aug 1, 2023

perrotta commented Aug 1, 2023 • edited Loading

caruta commented Aug 1, 2023

perrotta commented Aug 1, 2023 •

edited

Loading