fix: data is private in MultiTrajectory GrowableColumns

[wdconinc]

When accessing parameters or covariance through the generic accessors (i.e. not predicted, filtered, or smoothed), the public col() function should be used instead of direct access to the private data member.

Marked WIP until some more tests complete and confirmed working as intended.

[codecov]

Codecov Report

Merging #1289 (910606f) into main (75f0835) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##             main    #1289   +/-   ##
=======================================
  Coverage   47.47%   47.47%           
=======================================
  Files         376      376           
  Lines       19824    19824           
  Branches     9312     9312           
=======================================
  Hits         9412     9412           
  Misses       4020     4020           
  Partials     6392     6392           

Impacted Files	Coverage Δ
Core/include/Acts/EventData/MultiTrajectory.ipp	69.64% <ø> (ø)

📣 Codecov can now indicate which changes are the most critical in Pull Requests. Learn more

[wdconinc]

Tested, confirmed.

[paulgessinger]

Hey @wdconinc, see #1262: this internal access will go away soon to allow abstracted MultiTrajectory storage backends.
We can merge this anyway if you prefer, but future changes will likely be needed on your end.

[wdconinc]

Sure, fine with me. Will #1262 go into 20, or when is it expected to land? Just trying to figure out how much time to spend on our downstream interfaces if this will require a rewrite soon...

[paulgessinger]

@wdconinc That work is at the top of my list now.
Can you help me understand what your access patterns/modes are?
The fitter code in ACTS itself only required few changes, as I kept the public API largely identical to before.

[wdconinc]

After CKF tracking, we use the MultiTrajectory to find the time of flight to a set of AC-LGAD TOF detectors in our barrel and endcap regions (link), to connect hadron tracks (through the ecal) from tracking to the hcal clusters, and to the active radiator region of the RICH detectors.

Right now that means something like

        mj.visitBackwards(trackTip, [&](auto&& trackstate) {
          auto pathLength = trackstate.pathLength();

          // get track state parameters and their covariances
          const auto& parameter = trackstate.predicted();
          const auto& covariance = trackstate.predictedCovariance();

          // convert local to global
          auto global = trackstate.referenceSurface().localToGlobal(
            m_geoContext,
            {parameter[Acts::eBoundLoc0], parameter[Acts::eBoundLoc1]},
            {0, 0, 0}
          );

          // etc...
        });

That's where I was thinking we were originally using just parameters and covariance.

On a related note, getting a covariance matrix back from localToGlobal which the uncertainty propagated from covariance[eBoundLoc0, eBoundLoc1] would be useful too.

[paulgessinger]

Hey @wdconinc. Ok this pattern should still work after #1262. The covariance conversion is a little more involved. This is certainly possible and implemented, but not in the surface classes themselves.

In any case, this PR is internal to MultiTrajectory, so I think it's fine to merge in any case.