-
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
Bugfixes in simtrack to GEM/CSC digi and stub matching #30608
Bugfixes in simtrack to GEM/CSC digi and stub matching #30608
Conversation
The code-checks are being triggered in jenkins. |
-code-checks Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-30608/16867
Code check has found code style and quality issues which could be resolved by applying following patch(s)
|
The code-checks are being triggered in jenkins. |
-code-checks ERROR: Build errors found during clang-tidy run.
|
The code-checks are being triggered in jenkins. |
-code-checks ERROR: Build errors found during clang-tidy run.
|
The code-checks are being triggered in jenkins. |
+1 |
Comparison job queued. |
Comparison is ready Comparison Summary:
|
+1 |
@@ -137,15 +139,28 @@ void CSCStubMatcher::matchCLCTsToSimTrack(const CSCCLCTDigiCollection& clcts) { | |||
for (const auto& p : comps) { | |||
layer++; | |||
for (const auto& q : p) { | |||
if (verboseCLCT_) | |||
cout << "L" << layer << " " << q << " " << q.getHalfStrip() << " " << std::endl; |
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.
cout is not allowed, use messagelogger
if (clctComp == 65535) | ||
continue; | ||
if (verboseCLCT_) { | ||
std::cout << "\t" << clctComp << " " << endl; |
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.
cout is not allowed
nMatches++; | ||
if (verboseCLCT_) { | ||
cout << "\t\tnMatches " << nMatches << std::endl; |
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.
cout is not allowed
} | ||
} | ||
} | ||
if (verboseCLCT_) { | ||
cout << endl; |
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.
cout is not allowed
cout << "in LCT, getCLCT " << lct.getCLCT() << " getALCT " << lct.getALCT() << endl; | ||
|
||
if (verboseLCT_) { | ||
cout << ch_id << " " << ch_id2 << endl; |
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.
cout is not allowed
if (verboseCoPad_) | ||
cout << "CoPad: was matched! " << endl; | ||
superchamber_to_copads_[superch_id()].push_back(*pad); | ||
cout << "...was matched! " << endl; |
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.
cout is not allowed
@@ -31,6 +31,12 @@ void CSCSimHitMatcher::match(const SimTrack& track, const SimVertex& vertex) { | |||
// instantiates the track ids and simhits | |||
MuonSimHitMatcher::match(track, vertex); | |||
|
|||
// hard cut on non-CSC muons | |||
if (std::abs(track.momentum().eta()) < 0.9) |
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.
magic numbers
@@ -31,6 +31,10 @@ void GEMSimHitMatcher::match(const SimTrack& track, const SimVertex& vertex) { | |||
// instantiates the track ids and simhits | |||
MuonSimHitMatcher::match(track, vertex); | |||
|
|||
// hard cut on non-GEM muons | |||
if (std::abs(track.momentum().eta()) < 1.55) |
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.
magic number
const GEMDetId& p_id(h.detUnitId()); | ||
|
||
if (verbose_) | ||
std::cout << "Candidate GEM simhit " << p_id << " " << h << " " << h.particleType() << " " << h.processType() |
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.
cout is not allowed
const GEMDetId& p_id(h.detUnitId()); | ||
detid_to_hits_[h.detUnitId()].push_back(h); | ||
if (verbose_) | ||
std::cout << "...was matched" << std::endl; |
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.
cout is not allowed
@jfernan2 Thanks. Yes, for the time being, the plugin and analyzers are kept in https://github.com/gem-sw/GEMCode/blob/for-CMSSW_11_1_X/GEMValidation/plugins/GEMCSCAnalyzer.cc and https://github.com/gem-sw/GEMCode/tree/for-CMSSW_11_1_X/GEMValidation/interface/Analyzers. I want to move part of it to CMSSW, so that people can do GEM & CSC TP performance studies relatively quickly. |
+upgrade |
@jfernan2 Sure. I can do that. |
+1 |
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. @silviodonato, @dpiparo (and backports should be raised in the release meeting by the corresponding L2) |
+1 |
PR description:
Follow-up of #29830. I found a few more bugs while validating the CCLUT algorithm and the GEM trigger primitives in simulation.
PR validation:
Tested with Run-3 MC samples in various CMSSW_11_2_X pre-releases.
if this PR is a backport please specify the original PR and why you need to backport that PR:
N/A
@tahuang1991