-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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
Fix GEM Offline DQM for GE2/1 Demonstrator (backport of #36883, 12_2_X) #36906
Fix GEM Offline DQM for GE2/1 Demonstrator (backport of #36883, 12_2_X) #36906
Conversation
A new Pull Request was created by @seungjin-yang (Seungjin Yang) for CMSSW_12_2_X. It involves the following packages:
@emanueleusai, @ahmad3213, @cmsbuild, @jfernan2, @pmandrik, @pbo0, @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-038172/22277/summary.html Comparison SummarySummary:
|
urgent |
+1 |
This pull request is fully signed and it will be integrated in one of the next CMSSW_12_2_X IBs (tests are also fine) and once validation in the development release cycle CMSSW_12_3_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) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I notice that there are two modules missing with respect to the corresponding version in the master:
gemEfficiencyAnalyzer
gemEfficiencyHarvester
Are they needed in 12_2, or not?
Could you please comment about the difference?
@@ -27,30 +27,34 @@ | |||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Where is defined (and modified for Phase2) the gemEfficiencyAnalyzer
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
GEMEfficiencyAnalyzer::fillDescriptions
automatically generates DQMOffline.Muon.gemEfficiencyAnalyzerDefault_cfi.py
. Then, DQMOffline.Muon.gemEfficiencyAnalyzer_cfi
loads GEMEfficiencyAnalyzer
module from DQMOffline.Muon.gemEfficiencyAnalyzerDefault_cfi
. Am I understanding your question correctly?
cmssw/DQMOffline/Muon/src/GEMEfficiencyAnalyzer.cc
Lines 55 to 117 in c24439e
void GEMEfficiencyAnalyzer::fillDescriptions(edm::ConfigurationDescriptions& descriptions) { | |
// beam scenario | |
{ | |
edm::ParameterSetDescription desc; | |
desc.addUntracked<std::string>("name", "GlobalMuon"); | |
desc.addUntracked<std::string>("folder", "GEM/Efficiency/type0"); | |
desc.add<edm::InputTag>("recHitTag", edm::InputTag("gemRecHits")); | |
desc.add<edm::InputTag>("muonTag", edm::InputTag("muons")); | |
desc.addUntracked<bool>("isCosmics", false); | |
desc.addUntracked<bool>("useGlobalMuon", true); | |
desc.add<bool>("useSkipLayer", true); | |
desc.add<bool>("useOnlyME11", false); | |
desc.add<double>("residualRPhiCut", 2.0); // TODO need to be tuned | |
desc.add<bool>("usePropRErrorCut", false); | |
desc.add<double>("propRErrorCut", 1.0); | |
desc.add<bool>("usePropPhiErrorCut", false); | |
desc.add<double>("propPhiErrorCut", 0.01); | |
desc.addUntracked<std::vector<double> >("ptBins", {20., 30., 40., 50., 60., 70., 80., 90., 100., 120.}); | |
desc.addUntracked<int>("etaNbins", 9); | |
desc.addUntracked<double>("etaLow", 1.4); | |
desc.addUntracked<double>("etaUp", 2.3); | |
desc.addUntracked<bool>("monitorGE11", true); | |
desc.addUntracked<bool>("monitorGE21", false); | |
desc.addUntracked<bool>("monitorGE0", false); | |
{ | |
edm::ParameterSetDescription psd0; | |
psd0.setAllowAnything(); | |
desc.add<edm::ParameterSetDescription>("ServiceParameters", psd0); | |
} | |
descriptions.add("gemEfficiencyAnalyzerDefault", desc); | |
} // beam scenario | |
// cosmic scenario | |
{ | |
edm::ParameterSetDescription desc; | |
desc.addUntracked<std::string>("name", "GlobalMuon"); // FIXME | |
desc.addUntracked<std::string>("folder", "GEM/Efficiency/type0"); | |
desc.add<edm::InputTag>("recHitTag", edm::InputTag("gemRecHits")); | |
desc.add<edm::InputTag>("muonTag", edm::InputTag("muons")); | |
desc.addUntracked<bool>("isCosmics", true); | |
desc.addUntracked<bool>("useGlobalMuon", false); | |
desc.add<bool>("useSkipLayer", true); | |
desc.add<bool>("useOnlyME11", true); | |
desc.add<double>("residualRPhiCut", 5.0); // TODO need to be tuned | |
desc.add<bool>("usePropRErrorCut", true); | |
desc.add<double>("propRErrorCut", 1.0); | |
desc.add<bool>("usePropPhiErrorCut", true); | |
desc.add<double>("propPhiErrorCut", 0.001); | |
desc.addUntracked<std::vector<double> >("ptBins", {0., 10., 20., 30., 40., 50., 60., 70., 80., 90., 100., 120.}); | |
desc.addUntracked<int>("etaNbins", 9); | |
desc.addUntracked<double>("etaLow", 1.4); | |
desc.addUntracked<double>("etaUp", 2.3); | |
desc.addUntracked<bool>("monitorGE11", true); | |
desc.addUntracked<bool>("monitorGE21", false); | |
desc.addUntracked<bool>("monitorGE0", false); | |
{ | |
edm::ParameterSetDescription psd0; | |
psd0.setAllowAnything(); | |
desc.add<edm::ParameterSetDescription>("ServiceParameters", psd0); | |
} | |
descriptions.add("gemEfficiencyAnalyzerCosmicsDefault", desc); | |
} // cosmics | |
} |
@@ -6,9 +6,9 @@ | |||
from DQMOffline.Muon.gemEfficiencyAnalyzer_cfi import gemEfficiencyAnalyzerSta as _gemEfficiencyAnalyzerSta | |||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
and the same for gemEfficiencyHarvester
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
same with the previous question. I know the configuration files are not currently following the style guide, but I will fix it later in the master branch.
cmssw/DQMOffline/Muon/src/GEMEfficiencyHarvester.cc
Lines 15 to 19 in c24439e
void GEMEfficiencyHarvester::fillDescriptions(edm::ConfigurationDescriptions& descriptions) { | |
edm::ParameterSetDescription desc; | |
desc.addUntracked<std::string>("folder", "GEM/Efficiency/type0"); | |
descriptions.add("gemEfficiencyHarvesterDefault", desc); | |
} |
@seungjin-yang in the two
If I am not wrong (and I am asking because I could be wrong) in the backport PR there are no modules called
My question is: unless those modules exist already somewhere else, aren't they needed in 12_2_X? |
Please ignore this comment. |
@perrotta We don't need The hierarchies are as follows.
Then, the last two modules ( |
Ok, thank you @seungjin-yang , that makes sense (and indeed the python configuration in this backport PR do correspond to what you write) |
+1 |
PR description:
Backport of #36883. This PR fixes MonitorElement: WARNING:setBinLabel caused by GEM offline DQM not handling the new GEM geometry with GE2/1 demonstrator. See #36860 (comment).
PR validation:
This PR is tested with 11634.911.
if this PR is a backport please specify the original PR and why you need to backport that PR:
This PR is a backport of #36883.