fix: avoid geoManager->GetCurrentMatrix in LGADChargeSharing::_global2Local #1811
Conversation
Capybara summary for PR 1811
|
wdconinc
force-pushed
the
LGADChargeSharing_global2Local
branch
from
d77347d to
6749de4
Compare
| @@ -163,18 +163,18 @@ double LGADChargeSharing::_integralGaus(double mean, double sd, double low_lim, | |||
| } | |||
|
|
|||
| dd4hep::Position LGADChargeSharing::_cell2LocalPosition(const dd4hep::rec::CellID& cell) const { | |||
There was a problem hiding this comment.
@ssedd1123 Did this and _global2Local come from some other place?
There was a problem hiding this comment.
Do you mean if I use _global2Local in any other classes? I don't think so. If you are asking where I get the inspiration for these codes, I get it by imitating the source code of CellIDPositionConverter.
Line 97 - 101 of https://github.com/AIDASoft/DD4hep/blob/ccb7365d16c219c18a2b345551bf8d02f5797662/DDRec/src/CellIDPositionConverter.cpp#L101 is where I learn how to convert between local and global coordinates.
There was a problem hiding this comment.
If it was taken from somewhere else, we'd want to have it fixed there as well. It looks like we can remove these functions and just use CellIDPositionConverter::positionNominal?
Briefly, what does this PR introduce?
This PR avoids a thread-unsafe geoManager->GetCurrentMatrix() call which relies on cached state in the geoManager object.
What kind of change does this PR introduce?
Please check if this PR fulfills the following:
Does this PR introduce breaking changes? What changes might users need to make to their code?
No.
Does this PR change default behavior?
No.