Iss1847 -- Track objects use global coordinates and MeV

[bloodyyugo]

This resolves issue #1847

Added a rotation from tracking-->global coordinates, plus conversion of momentum to MeV, into TrackingUtils. This is done for CKF and GSF tracking. Also fixed up the plot limits in DQM to accommodate.

[tomeichlersmith]

🎉 Thank You

Two general comments:

• What about the TrackStates? Did I miss something or were those already in detector coordinates?
• Can you increment the version in the ClassDef of Track? Since this changes the meaning of the member variables, I want to signal to ROOT that things have changed. (We could also look at doing this conversion ourselves with schema evolution)

[bloodyyugo]

This does not put all of the parameters into detector coordinates. Really, the only thing that changes is the 3-momentum. The helix parameters don't really make any sense to rotate (the B-field sets the coordinate system) plus we use these parameters/covariances later on for GSF tracking and vertexing. So I'd like for those to stay as they are (even q/p, in GeV).

We use the The TrackStates only contain the 5 helix parameters, covariance, and the reference point used. I've incremented the classdef.

🎉 Thank You

Two general comments:

• What about the TrackStates? Did I miss something or were those already in detector coordinates?
• Can you increment the version in the ClassDef of Track? Since this changes the meaning of the member variables, I want to signal to ROOT that things have changed. (We could also look at doing this conversion ourselves with schema evolution)

[tomeichlersmith]

All of that makes sense, can you add doxygen comments to the fields of Track giving this reasoning?

About the TrackStates - this shows my own misunderstanding, so I'm going to ask for what I want. How would I get a track's projected position at the ECal? I had mistakenly assumed that was stored in the TrackState, but do I need to call some function to calculate it for me?

[tvami]

The QoP is also in MeV now right?

[tvami]

Suggested change

	"#sigma_{qOp} [GeV^{-1}]",nbins,0,1)
	"#sigma_{qOp} [MeV^{-1}]",nbins,0,1)

[tvami]

Suggested change

	"truth q/p [GeV^{-1}]",nbins,qopmin,qopmax)
	"truth q/p [MeV^{-1}]",nbins,qopmin,qopmax)

[bloodyyugo]

I'm keeping q/p (which is a track fit parameter) in GeV so that it aligns with ACTs.

[tvami]

About the TrackStates - this shows my own misunderstanding, so I'm going to ask for what I want. How would I get a track's projected position at the ECal? I had mistakenly assumed that was stored in the TrackState, but do I need to call some function to calculate it for me?

I think the track states paramaters are phi, theta, QoP so X vs Y vs Z is not there, on the other hand the GeV to MeV should be important, Matt please edit away the 1000 in

ldmx-sw/Ecal/src/Ecal/EcalHelper.cxx

Line 31 in 25e45c0

float p_track_state = (-1 / ecal_track_state.params_[4]) * 1000;

in case the tracks state QoP is in MeV now

[bloodyyugo]

You bring up a good point that I've been going back-and-forth with...ACTs (and every other tracking code I've seen) parameterizes the helix using the perigee of the helix wrt a reference point. The perigee of the helix is the POCA to the tracking-z axis (global y).

We actually don't save the perigee state but instead project the track to a surface...the target surface for the default track parameters, but also e.g. the ecal surface, sensor surfaces, etc for different track states. The parameters we save are then not strictly perigee parameters but the parameters at that planar surface. ACTs knows all about this; the parameters always have a corresponding surface associated with them (PerigeeSurface, PlanarSurface etc.) so you can easily move from one to another without screwing things up.

Anyway, we want to hide this stuff from the end user since 99% of them want to know things like you suggested: where is the track position and at the target or ECal. These are actually stored in the d0/z0 (or perigee_pars_[0]/[1]) of the Track object where d0-->global X, z0-->global Y. Same with the track state.

I think we probably want to make this more explicit and add getPositionAtTarget() and getPositionAtEcal() methods that just return the global positions. Do we add these to the Track object? As a utility function?

[bloodyyugo]

That line is still correct.

I think we should move away from using q/p and just use getMomentum() and get the magnitude from that vector. Or we could add a getPMag() method to Track.

About the TrackStates - this shows my own misunderstanding, so I'm going to ask for what I want. How would I get a track's projected position at the ECal? I had mistakenly assumed that was stored in the TrackState, but do I need to call some function to calculate it for me?

I think the track states paramaters are phi, theta, QoP so X vs Y vs Z is not there, on the other hand the GeV to MeV should be important, Matt please edit away the 1000 in

ldmx-sw/Ecal/src/Ecal/EcalHelper.cxx

Line 31 in 25e45c0

float p_track_state = (-1 / ecal_track_state.params_[4]) * 1000;

in case the tracks state QoP is in MeV now

[tvami]

I think we should move away from using q/p and just use getMomentum() and get the magnitude from that vector. Or we could add a getPMag() method to Track.

That's fine, but in ECAL we should have the momentum as measured (well propagated) at the ECAL face. The getMomentum() returns it at the target track state, right?

[tomeichlersmith]

I think the real solution to this confusion is just to have two different event objects.

IntermediateTrack - holds things in tracking coordinates and tracking units so it can be passed around with Acts, useful for folks in Tracking
Track - converts to detector coordinates and detector units, not to be re-put into Acts, useful for everyone else not in Tracking

I'm not committed to these names, but I don't like the idea of mixing purposes or having live conversions done in the event object itself. The data stored on disk should be directly interpret-able by users of the data.

[bloodyyugo]

Since this has come up, why don't we do something like this...

The Track object itself has only stuff that is universal to the track:
charge
PDG ID
track_id_ (for MC)
list of ids for hits included on track
list of ids for outliers on track
list of layers for holes on track
list of track states

The TrackState objects contain:
global position (x,y,z) of that track state (at target, at ecal, at sensor, whatever)
----- this will be the point on the track at the intersection of that surface
global momentum (px,py,pz) at that point
the 6x6 covariance matrix at that point

We can write a converter to/from this ldmx::Track/State to an ACTS::Track so that we can store the ldmx::Track and still get back to the ACTS::Track if we want do something like Vertexing or track re-fitting later on.

I think the real solution to this confusion is just to have two different event objects.

IntermediateTrack - holds things in tracking coordinates and tracking units so it can be passed around with Acts, useful for folks in Tracking Track - converts to detector coordinates and detector units, not to be re-put into Acts, useful for everyone else not in Tracking

I'm not committed to these names, but I don't like the idea of mixing purposes or having live conversions done in the event object itself. The data stored on disk should be directly interpret-able by users of the data.

[tomeichlersmith]

That sounds good to me, would Track still own TrackState or would we keep TrackState's separate?

If Track still owns TrackState, then maybe we just have different member variables for the different states? A vector feels like overkill when we have to change the C++ to add a new TrackState anyways...

[bloodyyugo]

@tomeichlersmith I'm not sure I know enough to have an opinion about TrackState being owned by Track or not.

As far as just having Track just having members for the different track states, this feels a bit inflexible (though I may not be fully understanding). Right now, for the Tagger I just have the atTarget state while for recoil it's atTarget and atECal. In the future we may want to add more (atTS or atHCal?). And I know there will be studies we'll want to do where we keep the track state at each sensor. To add those, I figure we can just add those track states to the vector from whatever processor is making the tracks for whatever study we are doing. Is using a vector for this not efficient?

[tomeichlersmith]

Your extra context about different track states for future studies answers my confusion - let's leave it as a vector of TrackStates.

We could avoid having the TrackStates owned by the Track if the TrackStates can function without knowing which Track they came from. Upon further reflection, I also retract this comment since I don't think this is possible.

I like your idea of having the data-on-disk be in global/detector coordinates and having utilities that convert to/from ACTS objects (that also convert into Tracking coordinates).

[bloodyyugo]

Ok, I will code this new Track object up as described (keeping TrackState in Track). This may take some time to change and test everything.

[tomeichlersmith]

That's okay, lets move slow and fix things. I've made broken enough things trying to move fast.

Edit: I converted this PR to a draft. Mark it as "Ready for Review" and Tamas and I will get notifications and we'll come back and review it again.