Category II Differences

• StiStarDetectorBuilder
  • https://github.com/star-bnl/star-sti/pull/9/commits/bf425a9ba8d3612eeadf7aefbbd1846bc6261c86
  • Reject change: Code is useless.  The check is already made before call to Fgt() routine in which it is located.
• Sti/StiKalmanTrackFitter.cxx
  • In debug() mode, appends different message to targetNode if the target is the vertex instead of a detector
  • Accept change: Should not affect results as it is just a text string.  Appends useful debugging information.
• Sti/StiTrackNodeHelper.cxx
  • Positivity constraint added
  • What is the purpose in StiCA?  Why is it not needed in Sti?  (If it is needed in Sti, why isn't it already in?  Should have been a bug report here.)

• Sti/StTrackNodeHelper.cxx -- Matrix operations
  • This is not *just* changing how the matrix operations are carried out.  There are two branchings which return early
  • Positivity checked on line 506.  Returns early if failed.
  • Check for existence of an array on line 510.
  • These two checks return early, which means the block around line 507 -- if (PJ) { ... } -- cannot be reached.
  • Only thing which I see which could be acceptable is the positivity check.  But... If it is needed, why is this code review the first time we are learning about it?
• Sti/StiTrackNodeHelper.cxx -- trsinv replacement
  • Repeats the pattern of the above two differences --
    1. Replaces calls to cernlib matrix operations with STAR / ROOT wrapper
    2. Checks positivity of matrix
    3. Returns early (probably with a yuge chi^2 to make the node go away)
• Sti/StiTrackNodeHelper.cxx -- Sign change
  
  • Ok,  Victor should comment on correctness, but same question applies.  If this fixes a bug in Sti, why is this the first time we hear about it? 

Category III Diffs

• assert removed to allow for multiple use of hit -- no problems, just need a switch to enable / disable assert  
• assert replaced withn return -- again, no problems if we add a switch