Skip to content

SBND Timing Reconstruction Refactor#188

Merged
hgreenlee merged 7 commits into
developfrom
feature/frame_shift_refactor_v10_00_14
May 27, 2026
Merged

SBND Timing Reconstruction Refactor#188
hgreenlee merged 7 commits into
developfrom
feature/frame_shift_refactor_v10_00_14

Conversation

@VCLanNguyen
Copy link
Copy Markdown
Contributor

@VCLanNguyen VCLanNguyen commented Mar 4, 2026

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. 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=45982

Relevant PR links (optional)

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

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

Quick checklist

  • Have you run git fetch and pulled the latest changes from the branch you're basing your PR against?
  • If you're adding new classes, have you added them to classes_def.xml in the relevant directory?
  • Have you added a checksum in classes_def.xml to any and all new classes you're implementing, and rebuilt?
  • If you're updating classes, have you incremented the ClassVersion by one compared to develop in classes_def.xml?

@VCLanNguyen VCLanNguyen changed the title Feature/frame shift refactor v10 00 14 SBND Timing Reconstruction Refactor Mar 4, 2026
@VCLanNguyen VCLanNguyen self-assigned this Mar 4, 2026
@VCLanNguyen VCLanNguyen requested a review from JosiePaton April 21, 2026 18:05
@VCLanNguyen
Copy link
Copy Markdown
Contributor Author

@JosiePaton @PetrilloAtWork can you guys review for CAF?

Copy link
Copy Markdown
Member

@PetrilloAtWork PetrilloAtWork left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I request the verification that the new code is able to correctly read files produced with the old code.
In other words, pick an old SBND or ICARUS CAF file, read it with the new code and verify that the results are reasonable. Repeat with a "flat" CAF for good measure.

If there were a problem so that old files can't be read, this would impair not only SBND but also ICARUS, since these data members are in the Standard Record for everybody. So let's make sure this is not the case.

Unfortunately on top of my head I don't know how to systematically verify that all the data except the two classes here are identical (there is a tool that produces a list of differences between CAF files, but I suspect that uses the same library version for both inputs, while we need both library versions with the same input). If you have ideas about how to do, great. If not, verify that you can read any quantity of your choice (e.g. the photoelectrons of the reconstructed flashes) and compare those with the old and new code.

There is one comment left on the changes themselves.

Comment thread sbnanaobj/StandardRecord/SRSBNDFrameShiftInfo.h Outdated
@VCLanNguyen
Copy link
Copy Markdown
Contributor Author

@PetrilloAtWork

I did the following validations and documented them here: https://docs.google.com/presentation/d/12G3pmgupLaAjWrZG_7vg9zIX8umdwiX52H9KWPdsQOs/edit?usp=sharing

The tests are:

  1. SBND data: old file + old CAFMaker
  2. SBND data: new file + new CAFMaker
  3. SBND data: old file + new CAFMaker
  4. ICARUS data: old file + old CAFMaker
  5. ICARUS data: old file + new CAFMaker

CAFMaker does not break and can read old files so it should not affect ICARUS. This CAFMaker changes only affects SBND workflow.

@VCLanNguyen VCLanNguyen requested a review from PetrilloAtWork May 6, 2026 23:03
Copy link
Copy Markdown
Member

@PetrilloAtWork PetrilloAtWork left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My comments were addressed. Thank you!

@hgreenlee hgreenlee merged commit 399590d into develop May 27, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants