SBND Timing Reconstruction Refactor

[VCLanNguyen]

Description

• Timing reconstruction is refactored in this PR, affecting the decode/reconstruction workflow of CRT/PMT/XA.
• After the refactoring, the @FrameShiftModule@ runs first in the decode chain, outputs timing products to be used at PMT/XA decoder and CRTStrip reconstruction.
• Relevant PMT reconstruction modules at Reco1/Reco2 are updated.
• The timing correction applied at CAFMakeer is undone. - The timing correction applied at CAFMakeer is undone. This should ressolve this issue: Converge timing correction in CAFMaker for SBND and ICARUS sbncode#567
• Details and validation plots can before in the linked docdb.

Link(s) to docdb describing changes (optional)

https://sbn-docdb.fnal.gov/cgi-bin/sso/ShowDocument?docid=46654

Relevant PR links (optional)

This PR needs the XA decoder PR in sbndcode to go in first:
#847

This PR needs to be merged together with this group of PRs:

• sbndcode: SBND Timing Reconstruction Refactor #915
• sbncode: SBND Timing Reconstruction Refactor sbncode#637
• sbnobj: SBND Timing Reconstruction Refactor sbnobj#170
• sbnanaobj:SBND Timing Reconstruction Refactor sbnanaobj#188

Checklist

• Added at least 1 label from available labels.
• Assigned at least 1 reviewer under Reviewers,
• Assigned all contributers including yourself under Assignees
• Linked any relevant issues under Developement
• Does this PR affect CAF data format? If so, please assign a CAF maintainer (PetrilloAtWork or JosiePaton) as additional reviewer.
• Does this affect the standard workflow?
• Is this PR a patch for the ongoing production? If so, separate PR must also be made for production/v10_06_00 branch!

[aantonakis]

crt veto looks good

[lynnt20]

approved for PMT side!

[aliciavr]

All good on XA's side!

[VCLanNguyen]

@JosiePaton @PetrilloAtWork can you guys review for CAF?

[PetrilloAtWork]

Some changes suggested here and there as usual, plus:

1. something that looks like a bug (which must be fixed, hence the rejection)
2. a questionable implementation of the difference of timestamps

A reiterated comment/recommendation: please format all the relevant comments on the usage of the functions and data member to Doxygen format.

[VCLanNguyen]

Hi @PetrilloAtWork , I resolved all comments except for 1 remaining that I'm sending to the author of the code.

Can you clarify if the two statements below are part of the comments that I have resolved, or they are separate and I have missed them. Thank you

1. something that looks like a bug (which must be fixed, hence the rejection)
   

2. a questionable implementation of the difference of timestamps
   

[PetrilloAtWork]

something that looks like a bug (which must be fixed, hence the rejection)

I think that was the resize() call that does not reset values to 0. That was fixed.

a questionable implementation of the difference of timestamps

That is the implementation of TimingUtils::SubtractUTCTimestmap(). I don't see my original comment any more... either GitHub ate it, or it never left my computer. It was something like:

How is this implementation different from:

double sbn::TimingUtils::SubtractUTCTimestmap(uint64_t ts1, uint64_t ts2) {
  return (ts1 > ts2)? (double)(ts1 - ts2): -(double)(ts2 - ts1);
}