Strip rh valid fix #4912
Strip rh valid fix #4912
Conversation
|
A new Pull Request was created by @wmtford for CMSSW_7_2_X. Strip rh valid fix It involves the following packages: SimTracker/TrackerHitAssociation @civanch, @ojeda, @danduggan, @rovere, @cmsbuild, @nclopezo, @mdhildreth, @deguio, @Degano can you please review it and eventually sign? Thanks. |
|
+1 |
|
+1 |
|
This pull request is fully signed and it will be integrated in one of the next CMSSW_7_2_X IBs unless changes (tests are also fine). |
|
|
||
| //check if the recHit is a SiStripMatchedRecHit2D | ||
| bool isMatchedHit = false; | ||
| if(dynamic_cast<const SiStripMatchedRecHit2D *>(&thit)) |
There was a problem hiding this comment.
dynamic_casts can be expensive. Did you check there were no regressions performance wise?
There was a problem hiding this comment.
No. I copied from the method associateHitId in this same class, where
it has been present all along. This new instance in associateHit is
outside any loops once this function has been called, so it adds one
occurrence on top of one to three occurrences invoked by the upstream
call to associateHitId.
There is no gross change in the execution time (3:28 vs 4:07 minutes for
10 events (3 TeV QCD jets plus pileup), in batch with no control on processor speed), but I can run a
more controlled test.
...
Running 19 events (QCD 3-TeV jets, this time without pileup) on the same processor took 31 seconds vs 33 when I removed the test with the dynamic cast. (There are more hits processed downstream in the latter case.)
SiStripRecHitsValid: Fix a bug that was disabling filling of histos for no. of recHits by subdet. Adjust the binning of the now-enabled histograms.
For matched recHits, limit the simHits projected from the stereo sensor to those actually on the stereo sensor.
TrackerHitAssociator: For matched recHits, suppress ghost hits by requiring a simTrack association.