A set of fixes for Ph2 OT cluster/rechit validation histos #20682
Conversation
|
The code-checks are being triggered in jenkins. |
|
+code-checks |
|
A new Pull Request was created by @lowette (Steven Lowette) for master. It involves the following packages: RecoLocalTracker/Phase2TrackerRecHits @perrotta, @cmsbuild, @kpedro88, @slava77 can you please review it and eventually sign? Thanks. cms-bot commands are listed here |
| // Create histograms for the layer if they do not yet exist | ||
|
|
||
| // initialize the nhit counters if they don't exist for this layer | ||
| std::map< unsigned int, unsigned int >::iterator nhitit(nRecHits[det].find(layer)); |
| // initialize the nhit counters if they don't exist for this layer | ||
| std::map< unsigned int, unsigned int >::iterator nhitit(nRecHits[det].find(layer)); | ||
| if (nhitit == nRecHits[det].end()) { | ||
| nRecHits [det].insert(std::pair<unsigned int, unsigned int>(layer, 0)); |
There was a problem hiding this comment.
use emplace for all of these
| histogramLayer->second.deltaX_eta_P[det][nch]->Fill(eta, localPosClu.x() - localPosHit.x()); | ||
| histogramLayer->second.deltaY_eta_P[det][0] ->Fill(eta, localPosClu.y() - localPosHit.y()); | ||
| histogramLayer->second.deltaY_eta_P[det][nch]->Fill(eta, localPosClu.y() - localPosHit.y()); | ||
| histogramLayer->second.pullX_eta_P [det][0] ->Fill(eta, (localPosClu.x() - localPosHit.x())/sqrt(rechitIt->localPositionError().xx())); |
There was a problem hiding this comment.
try to avoid divide by 0 here
|
|
||
| // fill the counter histos per layer | ||
| for (unsigned int det = 1; det < 3; ++det) { | ||
| for (std::map<unsigned int, unsigned int>::const_iterator it = nRecHits[det].begin(); it != nRecHits[det].end(); ++it) { |
| // fill the counter histos per layer | ||
| for (unsigned int det = 1; det < 3; ++det) { | ||
| for (std::map<unsigned int, unsigned int>::const_iterator it = nRecHits[det].begin(); it != nRecHits[det].end(); ++it) { | ||
| std::map< unsigned int, RecHitHistos >::iterator histogramLayer(histograms_.find(it->first)); |
| unsigned int simTrackId(getSimTrackId(pixelSimLinks, detId, channel)); | ||
| clusterSimTrackIds.push_back(simTrackId); | ||
| std::vector<unsigned int> simTrackIds(getSimTrackId(pixelSimLinks, detId, channel)); | ||
| for (unsigned int i=0; i<simTrackIds.size(); ++i) { |
| for (edm::PSimHitContainer::const_iterator simhitIt(simHitsRaw[simhitidx]->begin()); simhitIt != simHitsRaw[simhitidx]->end(); ++simhitIt) { | ||
| if (rawid == simhitIt->detUnitId()) { | ||
| //std::cout << "=== " << rawid << " " << &*simhitIt << " " << simhitIt->trackId() << " " << simhitIt->localPosition().x() << " " << simhitIt->localPosition().y() << std::endl; | ||
| if (std::find(clusterSimTrackIds.begin(), clusterSimTrackIds.end(), simhitIt->trackId()) != clusterSimTrackIds.end()) { |
There was a problem hiding this comment.
using std::find() in a double-nested loop seems like a bad idea. Could the clusterSimTrackIds be kept sorted?
| if (!simhit) continue; | ||
|
|
||
| // only look at simhits from highpT tracks | ||
| std::map< unsigned int, SimTrack >::const_iterator simTrackIt(simTracks.find(simhit->trackId())); |
|
|
||
| // fill the counter histos per layer | ||
| for (unsigned int det = 1; det < 3; ++det) { | ||
| for (std::map<unsigned int, unsigned int>::const_iterator it = nClusters[det].begin(); it != nClusters[det].end(); ++it) { |
There was a problem hiding this comment.
range-based loops and auto for iterators
| if (DSViter == pixelSimLinks->end()) return retvec; | ||
| for (edm::DetSet< PixelDigiSimLink >::const_iterator it = DSViter->data.begin(); it != DSViter->data.end(); ++it) { | ||
| retvec.push_back(it->SimTrackId()); | ||
| if (channel == it->channel()) { |
There was a problem hiding this comment.
why push_back twice if it's the requested channel?
|
please test |
|
The tests are being triggered in jenkins. |
|
perhaps it's more practical to move necessary functionality to the Validation subsystem. |
|
assign dqm @dmitrijus please take a look if we have something similar in the Validation subsystem to pick it up with standard tools. |
|
New categories assigned: dqm @kmaeshima,@vanbesien,@vazzolini,@dmitrijus you have been requested to review this Pull request/Issue and eventually sign? Thanks |
|
+1 The following merge commits were also included on top of IB + this PR after doing git cms-merge-topic: |
|
Comparison job queued. |
|
Comparison is ready Comparison Summary:
|
|
@lowette |
|
Comparison job queued. |
|
Comparison is ready There are some workflows for which there are errors in the baseline: Comparison Summary:
|
|
+1
|
|
+1 |
|
hi @dmitrijus , please look at this PR , thanks |
|
+1 This validation currently does not use anything from, and is not a part of DQM.
|
cmsbuild
added
dqm-approved
fully-signed
and removed
dqm-pending
pending-signatures
labels
|
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. @davidlange6, @slava77, @smuzaffar (and backports should be raised in the release meeting by the corresponding L2) |
|
+1 |
This PR contains an update of the Ph2 OT cluster and rechit validation code that fixes several outstanding issues in the histograms: fix the number of rechits and clusters (separate issues), and fix the issue of saturation in TH1F's with large bin contents. Also the cluster validation code was streamlined to be much closer to the rechit validation.
@boudoul @delaere