MTD geometry and reconstruction: update locaPosition in RectangularMTDTopology and adapt cluster accordingly #40049
Conversation
…dingly, use appropriate interface in Validation
|
-code-checks Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-40049/33012
Code check has found code style and quality issues which could be resolved by applying following patch(s)
|
|
@parbol FYI |
|
+code-checks Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-40049/33013
|
|
A new Pull Request was created by @fabiocos (Fabio Cossutti) for master. It involves the following packages:
@civanch, @Dr15Jones, @clacaputo, @bsunanda, @makortel, @mandrenguyen, @emanueleusai, @ianna, @ahmad3213, @cmsbuild, @AdrianoDee, @srimanob, @jfernan2, @mdhildreth, @syuvivida, @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-d480e5/28977/summary.html Comparison SummarySummary:
|
|
DQM histogram differences in ETL Monitoring Elements using hits, just as expected. The only difference in DQM histos I do not understand is this one in Tracks/TrackPathLenghtvsEta: |
|
@emanueleusai this PR affects directly ETL digitization in the determination of the hit radius in https://github.com/cms-sw/cmssw/blob/master/SimFastTiming/FastTimingCommon/src/ETLElectronicsSim.cc#L51 and this should explain the minor fluctuations observed in the ETL plots. This might as well induce some minor fluctuation in the forward clusters, which are mostly single-pixel but not always, and as a consequence occasionally affect the track-cluster match, which define the collection of tracks and the value of the observable which enters into the affected plot. BTW this appears in the sample with PU, where many more hist and tracks are expected, but the PU is a premixed sample not affected by the change in this PR. |
|
@fabiocos thank you very much for the explanation! |
|
+1
|
|
+geometry |
|
@cms-sw/reconstruction-l2 @cms-sw/upgrade-l2 questions or comments? |
|
+reconstruction |
|
+upgrade |
cmsbuild
added
fully-signed
upgrade-approved
and removed
pending-signatures
upgrade-pending
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. @perrotta, @dpiparo, @rappoccio (and backports should be raised in the release meeting by the corresponding L2) |
|
+1 |
PR description:
The
locaPosition,localXandlocalYmethods ofRectangularMTDTopologydo not provide, as naively expected, the center of a pixel as its reference position, but its left-bottom corner. This is counterintuitive and forces to some odd coding in the clustering algorithm. It is compensated by a shift by 0.5 pixel in the calculation of the cluster x and y coordinates in the corresponding DataFormat. In order to have a clean situation, the shift is moved from the cluster position calculation to the basic topology code itself.This should have no net impact, given the limited use of these methods, and the fact that its use inside the BTL clustering is effectively dummy so far. In preparation of a further PR which updates the clustering code, I propose initially this update.
With the occasion, I update the local reconstruction validation code to properly use the cluster parameter estimator code to extract the position, without replicating its methods (which are going to change in next developments).
PR validation:
The code was tested with single muon workflow 20903 (scenario D88), showing that the cluster resolution and pull remains consistent with the existing result. The shift in tracking hits position printed from debug statements looks consistent with the proposed update of reference position.