-
Notifications
You must be signed in to change notification settings - Fork 613
[Common] Consider propagation of Tracks in TrackPropagationModule #14114
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
base: master
Are you sure you want to change the base?
Conversation
|
O2 linter results: ❌ 2 errors, |
[Common] Please consider the following formatting changes to AliceO2Group#14114
|
Thanks a lot @romainschotter ! I would perhaps ask someone with experience with the track tuner to also check if all remains correct in case the propagator module is dealing tracks that are already |
|
@ddobrigk pointed out that the track tuner was not used for prepropagated tracks (thanks a lot for noticing!) |
[Common] Please consider the following formatting changes to AliceO2Group#14114
| // TVertex type: either math_utils::Point3D<value_t> or o2::dataformats::VertexBase | ||
| // TDCA type: either dim2_t or o2::dataformats::DCA | ||
| template <typename TTrackPar, typename TVertex, typename TDCA> | ||
| bool calculateDCA(TTrackPar trackPar, const TVertex& vtx, double b, TDCA* dca, double maxD) |
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.
What is the reason to pass the trackPar by copy rather than by reference?
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.
Hi @shahor02 ! That's a mistake from my side, thank you for pointing that out! I will change it tomorrow morning
| double d = gpu::CAMath::Abs(x * snp - y * csp); | ||
| if (d > maxD) { | ||
| // provide default DCA for failed propag | ||
| if constexpr (requires { trackPar.getSigmaY2(); vtx.getSigmaX2(); }) { | ||
| dca->set(o2::track::DefaultDCA, o2::track::DefaultDCA, | ||
| o2::track::DefaultDCACov, o2::track::DefaultDCACov, o2::track::DefaultDCACov); | ||
| } else { | ||
| (*dca)[0] = o2::track::DefaultDCA; | ||
| (*dca)[1] = o2::track::DefaultDCA; | ||
| } | ||
| return false; | ||
| } |
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.
If I understand correctly, the assumption is that the track is already propagated to the PCA to the vertex and just needs a rotation to the frame where the track is normal to the DCA vector. Then (1) it is not clear why the code above is needed (2) how the track can be in another alpha frame if it was already propagated to the DCA.
If it is already in that frame, then one just rotated the PV to the same frame, i.e. L394 is enough.
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.
This function is essentially a generalization of the propagateToDCABxByBz in the Detectors/Base/src/Propagator.cxx for TrackPar and TrackParCov, where the propagation call has been removed. All the rest remained the same.
I agree that l.398-409 and l.421-432 can be removed. However, shouldn't we keep the remaining parts for the DCA covariance calculation?
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.
If the track was already propagated to the PCA and rotated to the frame normal to the DCA vector, then the only code needed is for the rotation of the PV to the same frame, i.e.
float sn, cs, alp = trackPar.getAlpha();
o2::math_utils::detail::sincos(alp, sn, cs);
float xv = vtx.getX() * cs + vtx.getY() * sn, yv = -vtx.getX() * sn + vtx.getY() * cs, zv = vtx.getZ();
if constexpr (requires { trackPar.getSigmaY2(); vtx.getSigmaX2(); }) {
o2::math_utils::detail::sincos(alp, sn, cs);
auto s2ylocvtx = vtx.getSigmaX2() * sn * sn + vtx.getSigmaY2() * cs * cs - 2. * vtx.getSigmaXY() * cs * sn;
dca->set(trackPar.getY() - yv, trackPar.getZ() - zv, trackPar.getSigmaY2() + s2ylocvtx, trackPar.getSigmaZY(), trackPar.getSigmaZ2() + vtx.getSigmaZ2());
} else {
(*dca)[0] = trackPar.getY() - yv;
(*dca)[1] = trackPar.getZ() - zv;
}
If the track was propagated to the PCA but not rotated (but I don't see under which circumstances this may be the case, unless there was some rotation applied after the initial propagateToDCA), then the full code is indeed needed.
And in all other case (including track modification by the MC tuner mentioned by @mfaggin ) the full propagation and rotation will be needed (though there is no reason to do this in double).
|
Hi @romainschotter @ddobrigk apologize for my late reaction. There is one important thing that must be addressed in MC if the trackTuner is enabled. If the track tuner is used, the track-parameter smearing is performed at the production point, i.e. the point where the MC particle associated to the current track is used (see [1]). This point is the PV in case of particles coming directly from there, but it corresponds to the mother decay point in case of particles coming from decays. In other words, the function [1] https://github.com/AliceO2Group/O2Physics/blob/master/Common/Tools/TrackTuner.h#L782-L783 |
Just to give credit where it's due, it was @fgrosa who pointed it out to me :-) (thanks!) |
[Common] Please consider the following formatting changes to AliceO2Group#14114
|
Hi @shahor02 and @mfaggin ! Thank you very much for the comments! I have also tried removing the rotation of the track but the DCA distribution starts to differ from the case without prepropagation, as you can see here: |
| alp += gpu::CAMath::ASin(sn); | ||
| if (!trackPar.rotate(alp)) { |
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.
Did you simply suppress these 2 lines? If the track would not be rotated to the frame of the PCA after the propagation, the difference would be much larger than what your pdf shows.
Is it possible that you are related the same preprogated track to two different compatible PVs?
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.
I actually tried different approaches:
- I considered only the rotation of the PV to the same frame as the track at the PCA (using the exactly the code you suggested 3days ago, i.e.
float sn, cs, alp = trackPar.getAlpha();
o2::math_utils::detail::sincos(alp, sn, cs);
float xv = vtx.getX() * cs + vtx.getY() * sn, yv = -vtx.getX() * sn + vtx.getY() * cs, zv = vtx.getZ();
if constexpr (requires { trackPar.getSigmaY2(); vtx.getSigmaX2(); }) {
o2::math_utils::detail::sincos(alp, sn, cs);
auto s2ylocvtx = vtx.getSigmaX2() * sn * sn + vtx.getSigmaY2() * cs * cs - 2. * vtx.getSigmaXY() * cs * sn;
dca->set(trackPar.getY() - yv, trackPar.getZ() - zv, trackPar.getSigmaY2() + s2ylocvtx, trackPar.getSigmaZY(), trackPar.getSigmaZ2() + vtx.getSigmaZ2());
} else {
(*dca)[0] = trackPar.getY() - yv;
(*dca)[1] = trackPar.getZ() - zv;
}
)
- I also tried removing the following line entirely:
if (!trackPar.rotate(alp)) {
In both cases, I got the same DCA distribution which corresponds to the one shown in open star markers in the PDF attached in my previous message.
I suppose it is possible that I am calculating the DCA of a prepropagated track with respect to another compatible PV. Is there a way to check that?
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.
I see that some analyses are using track.compatibleCollIds().size() > 0 to check if the track is ambiguous, but I don't know if getting this track attribute requires some special. Btw, in my code snippet, which you quoted, I've wrongly left unnecessary sincos call, should be
float sn, cs, alp = trackPar.getAlpha();
o2::math_utils::detail::sincos(alp, sn, cs);
float xv = vtx.getX() * cs + vtx.getY() * sn, yv = -vtx.getX() * sn + vtx.getY() * cs, zv = vtx.getZ();
if constexpr (requires { trackPar.getSigmaY2(); vtx.getSigmaX2(); }) {
auto s2ylocvtx = vtx.getSigmaX2() * sn * sn + vtx.getSigmaY2() * cs * cs - 2. * vtx.getSigmaXY() * cs * sn;
dca->set(trackPar.getY() - yv, trackPar.getZ() - zv, trackPar.getSigmaY2() + s2ylocvtx, trackPar.getSigmaZY(), trackPar.getSigmaZ2() + vtx.getSigmaZ2());
} else {
(*dca)[0] = trackPar.getY() - yv;
(*dca)[1] = trackPar.getZ() - zv;
}
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.
Hi @shahor02 ! Apologies for the late reply!
I tried the correct code snippet this time (without the remaining unnecessary sincos call) but the DCA distribution looked the same as before, meaning that I still obtain the open star markers in the PDF attached in my previous message.
Also, I repeated the comparison focusing only on collisions without any other compatible PV (by requiring track.compatibleCollIds().size() > 0) and now the DCA distribution with your code snipped (without rotation) looks the same as with rotation and the same as without prepropagated tracks.
DCAxy_StdVsPrepropagate_WithoutRotation_withoutCompatibleColl_Strictly0.pdf
It seems that, indeed, I was calculating the DCA of some prepropagated tracks with respect to another compatible PV.
However, I am not entirely sure to understand how that's possible. I am wondering if it is not possible that the tracks are always prepropagated to the mean vertex (https://github.com/AliceO2Group/AliceO2/blob/4090041b401c7aa6c919ca923126fff950cbccd1/Detectors/AOD/src/AODProducerWorkflowSpec.cxx#L2858) instead of the PV of its assigned collision. If I am saying something wrong, please don't hesitate to correct me
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.
Hi @romainschotter ,
The extra sincos could not change the results, it was simply not needed.
Concerning this:
Also, I repeated the comparison focusing only on collisions without any other compatible PV (by requiring
track.compatibleCollIds().size() > 0) and now the DCA distribution with your code snipped (without rotation) looks the same as with rotation and the same as without prepropagated tracks.
Do you mean track.compatibleCollIds().size() == 1 (for the tracks compatible with 1 PV only)?
The track is propagated to the meanvertex only if it is not associated to any real PV. But in case it is associated to >1 PV, the propagation will be done only to 1st one (since the track is saved only once).
But if this is the case, then in your method just rotation would not be enough;
in case of the ambiguous vertex assignment also the propagation would be needed, as it is done in case of trackTuning.
I am wondering if your reference distribution is not suffering from a similar problem.
I think it is worth comparing for ambiguous tracks only (i.e. track.compatibleCollIds().size() > 0) the DCA distribution with old propagate to dca, and your method with rotation only and with rotation + propagation.
Consider propagation of Tracks + add method to calculate DCA without propagation
For more details, please have a look at: https://its.cern.ch/jira/browse/O2-6210