Issue #1251


Chris Rogers (chair/secretary)
Ed Santos
Ian Taylor
Ryan Bayes
Adam Dobbs


  • Specification
  • Data structure
    • SciFiTrack
    • SciFiTrackPoint
  • Class structure overview (implementation)
  • Code class by class
    • MapCppTrackerRecon
    • KalmanSeed
    • KalmanTrackFit
    • KalmanSite
    • KalmanTrack
      • StraightTrack
      • HelicalTrack
    • KalmanSciFiAlignment
    • SciFiTrack (datastructure)
    • SciFiTrackPoint (datastructure)
  • Tests
  • Documentation

Strikethrough indicates we didn't finish that item on the agenda - we got about half way through KalmanTrack. We looked at SciFiTrack and SciFiTrackPoint from a datastructure point of view but we never went through the source files.


Specification and Datastructure

Aim of the code is to fit tracks in the tracker volume. Input is up to 5 space points arising from Pattern Recognition. Output is a data structure holding

  • vector of track points per plane
    • Recon x, y, px, py
    • MC x, y, px, py
    • residual
    • plane id (index from -15 to 15)
  • filtered and smoothed chi2
  • tracker id
  • No scheme for chi2 when we have multiple adjacent channel hits - we took note but decided this was an action for tracker group (not necessarily Santos)
  • Need to remove Monte Carlo data from the tracker recon data structure - predicated on #1197 which was moved to Taylor
  • There was some uncertainty on number of degrees of freedom in chi2, at least as far as I was concerned. A bit of documentation here would probably help.
  • Track object should point back to the clusters that were use to construct it, using a TReference. We noted that the rest of the tracker data structure also needs to use TReferences for cross links. Ian Taylor to advise.
  • Trackpoint ID was done using an integer from -15 to 15; it should be done using tracker number/station number/plane number to be consistent with the rest of the tracker stuff.
  • Trackpoint missing time and pz from data structure
  • Missing error covariance matrix
  • Add "algorithm_used" enumerator to the track to indicate e.g. whether algorithm was kalman_straight fitter or kalman_helical fitter.

Class Structure Overview

Class structure goes like (greyed out items not explicitly in code review)

A process flow diagram here would have been useful.


KalmanSeed is interface class to take output from PatternRecognition and turn it into a zero iteration for Kalman algorithm. KalmanSeed supports interfaces for StraightTracks and HelicalTracks from PatternRecognition.

  • StraightTrack vs HelicalTrack is handled by a template function call in SciFiPatternRecognition. Preferable to do this using inheritance (it can be hard to figure out what functions are required when calling template functions, whereas this is clearly defined by "Virtual" keyword/etc in inheritance).
  • No comments in KalmanSeed
  • Several hardcoded constants like assumes 4 T in KalmanSeed::ComputeInitialStateVector. Probably indicates lack of testing e.g. need tests to check polarity/charge changes


KalmanTrackFit is manager class for the track fitting algorithm. Most of the actual fitting routines are in KalmanTrack.

  • Should comment a use case for why FilterVirtual exists (it is for SciFi alignment)
  • Misalignment study should go in another class using has_a to share code (or if needed and appropriate, inheritance).
    • type_of_dataflow; should throw an exception if trying to do alignment study in parallel processing mode rather than storing type_of_dataflow
    • Some discussion about how one could manage the geometry, in particular for an alignment study. Propose making a local version in memory of the geometry information, taken from MiceModules; this can be edited by the "alignment study" code, and read by the Kalman fitting code (and presumably other pieces of tracker code). Then following alignment study this can be written to disk and converted back to MiceModules, either automagically or by hand. This sort of thing has already been done elsewhere e.g. in cluster finding
    • Depending on complexity of the study this should probably be a written procedure.
  • Initialize function should probably go into the constructor
  • Should not require numb_measurements 15 (i.e. 5 space points should not be required)
    • Some discussion of the PR (side issue); request that PR should attempt to fit 3 point tracks as well (and leave it to Global recon to accept or reject tracks based on e.g. TOF information)
  • Should take most verbose of kalman_verbose_level or (global) verbose_level 0
  • Initial estimated error on x, y should be in datacards as (channel_width)^2/12; currently set to 1 mm^2 default
  • SciFiParams should all be in configuration defaults; enables systematics studies.


KalmanTrack has all the algorithms for actually doing the fit - i.e. propagating tracks etc

  • The class is very long - the header file has about 30 public member functions and the source file is about 700 lines. Would be better to split into KalmanExtrapolate, KalmanFilter, KalmanPropagator, and if required a KalmanTrack which provides interfaces/data/etc
  • SetResidual needs a comment
  • We noted that the track fit uses _mass and _charge. Note that we need to be able to propagate -ves and +ves.
    • Need to investigate effect of mis-id of charge by PatternRecognition; does Kalman then fail to fit the track (it might e.g. try to give things negative pz). May be better to use a continuous variable e.g. angular momentum to handle charge id. Then "nearly straight" tracks will get a weighted charge id fit (e.g. angular momentum will be small). May be an action for Santos successor, as it is a physics question.
    • Noted that _mass assumption of muon mass may bias the pid measurement, i.e. may expect better fits for muon assumption even when we have pions because the seed contains muon assumption. Issue to be studied by global group when they have pid algorithms.
  • KalmanTrack::Extrapolate should throw an exception if (i < 1 || i >= sites.size()); just that the chances of someone passing an out of range index are too large. It is in the inner loop but meh.
  • if (_n_parameters > 4) should properly use a _straight_track flag for clarity and correctness reasons
  • SubtractEnergyLoss algorithm should be \vec{p}_{new} = \vec{p}_old * (1-dp/p) current implementation looks pretty horrendous.

We stopped when at the beginning of Filtering Routines.

Note - we got through about 600 lines of code in 4 hours - so 2.5 lines of code per minute. From wikipedia Typical code review rates are about 150 lines of code per hour which is indeed, 2.5 lines of code per minute.

Updated by Rogers, Chris over 7 years ago ยท 20 revisions