-
Notifications
You must be signed in to change notification settings - Fork 4.4k
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
GEM Online & Offline Efficiency Update (backport of #37954, 12_3_X) #38330
GEM Online & Offline Efficiency Update (backport of #37954, 12_3_X) #38330
Conversation
A new Pull Request was created by @seungjin-yang (Seungjin Yang) for CMSSW_12_3_X. It involves the following packages:
@emanueleusai, @ahmad3213, @cmsbuild, @jfernan2, @pmandrik, @micsucmed, @rvenditti can you please review it and eventually sign? Thanks. cms-bot commands are listed here
|
please test |
+1 Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-dc1332/25471/summary.html Comparison SummarySummary:
|
testing at P5 |
type gem |
backport of #37954 |
tests at P5 completed successfully |
+1 |
This pull request is fully signed and it will be integrated in one of the next CMSSW_12_3_X IBs (tests are also fine) and once validation in the development release cycle CMSSW_12_5_X is complete. This pull request will now be reviewed by the release team before it's merged. @perrotta, @dpiparo, @qliphy (and backports should be raised in the release meeting by the corresponding L2) |
Just to understand: is this large PR needed as a whole in the data-taking release 12_3_X just to turn on the muon propagation error cuts? If that's the only reason, perhaps a simpler fix could have been enough. By the way, with respect to the PR in the master here the files that implement the GEMOfflineDQMBase class are not removed: was there any reason for it? |
Pull request #38330 was updated. @emanueleusai, @ahmad3213, @cmsbuild, @jfernan2, @pmandrik, @micsucmed, @rvenditti can you please check and sign again. |
Hi @perrotta, this PR not only turns on propagation error cuts but also provides a more reliable GEM detection efficiency measurement. So, I want to backport this PR to 12_3_X. |
please test |
+1 Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-dc1332/25612/summary.html Comparison SummarySummary:
|
+1 |
This pull request is fully signed and it will be integrated in one of the next CMSSW_12_3_X IBs (tests are also fine) and once validation in the development release cycle CMSSW_12_5_X is complete. This pull request will now be reviewed by the release team before it's merged. @perrotta, @dpiparo, @qliphy (and backports should be raised in the release meeting by the corresponding L2) |
+1
|
PR description:
This PR is a backport of #37954. This PR turns on the muon propagation error cuts, which mistakenly were turned off via #36883. Also, this PR simplifies the directory structure in the DQM output file and reduces redundant plots. In the offline DQM, the way to build the initial state muon propagations and a matching metric is changed. You can find more details here.
From a technical point of view, this PR adds base classes for online and offline efficiency monitors.
PR validation:
Because this PR affects both the online and the offline DQM, this PR was validated with two tests.
if this PR is a backport please specify the original PR and why you need to backport that PR:
Backport of #37954. In 12_3_X, the propagation error cuts mistakenly are turned off, resulting in a lower GEM detection efficiency than expected. So, I decided to backport #37954 to 12_3_X.