Skip to content

Conversation

@VCLanNguyen
Copy link
Contributor

@VCLanNguyen VCLanNguyen commented Aug 30, 2025

Description

New module for timing reconstruction in introduced at reco2 in sbndcode.
Module make data products that are saved in CAF as new Standard Record products.
Data Timing variables in CAF, i.e. crt space point time, sbnd crt track time, opflash and opt0, are shifted for beam spill reconstruction.
Full description can be found in docdb: https://sbn-docdb.fnal.gov/cgi-bin/sso/RetrieveFile?docid=43090

Copy link
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.

There is a bug in need to be fixed.

In addition, I am raising a design issue (in the sense of "controversy" rather than "flaw") with respect to the use of caf::SRTrigger. The flame is in the old PR #562. As I see it, there are two parts to it:

  • not filling caf::SRTrigger: this has an immediate impact on the produced samples, and I would like it to be discussed immediately before proceeding;
  • using separate code for shifting times: this is about implementation internals (two pieces of code doing the same thing) and can be discussed in parallel without blocking this PR.

@VCLanNguyen
Copy link
Contributor Author

VCLanNguyen commented Sep 3, 2025

There is a bug in need to be fixed.

In addition, I am raising a design issue (in the sense of "controversy" rather than "flaw") with respect to the use of caf::SRTrigger. The flame is in the old PR #562. As I see it, there are two parts to it:

* not filling `caf::SRTrigger`: this has an immediate impact on the produced samples, and I would like it to be discussed immediately before proceeding;

* using separate code for shifting times: this is about implementation internals (two pieces of code doing the same thing) and can be discussed in parallel without blocking this PR.

I will create an issue on sbncode to address the second point. I think to converge SBND and ICARUS timing correction in caf, more testing is needed.
Issue: #567

@PetrilloAtWork PetrilloAtWork self-requested a review September 4, 2025 01:59
Copy link
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.

Approved!

@kjplows
Copy link
Contributor

kjplows commented Sep 4, 2025

trigger build ci_ref=v10_06_02 LArSoft/larsoft@LARSOFT_SUITE_v10_06_00_02 LArSoft/larwirecell@LARSOFT_SUITE_v10_06_00_02 LArSoft/lar*@LARSOFT_SUITE_v10_06_00 SBNSoftware/sbnobj#141 SBNSoftware/sbnanaobj#155 SBNSoftware/sbn*@release/SBN2025A SBNSoftware/sbndcode@v10_06_03

@FNALbuild
Copy link

✔️ CI build for LArSoft Succeeded on slf7 for e26:prof -- details available through the CI dashboard

@FNALbuild
Copy link

✔️ CI build for LArSoft Succeeded on slf7 for c14:prof -- details available through the CI dashboard

@FNALbuild
Copy link

❌ CI build for SBND Failed at phase build SBND on slf7 for c14:prof -- details available through the CI dashboard

🚨 For more details about the failed phase, check the build SBND phase logs

parent CI build details are available through the CI dashboard

@FNALbuild
Copy link

❌ CI build for ICARUS Failed at phase build ICARUS on slf7 for c14:prof -- details available through the CI dashboard

🚨 For more details about the failed phase, check the build ICARUS phase logs

parent CI build details are available through the CI dashboard

@FNALbuild
Copy link

❌ CI build for ICARUS Failed at phase ci_tests ICARUS on slf7 for e26:prof - ignored warnings for build -- details available through the CI dashboard

🚨 For more details about the failed phase, check the ci_tests ICARUS phase logs

parent CI build details are available through the CI dashboard

@FNALbuild
Copy link

⚠️ CI build for SBND Warning at phase ci_tests SBND on slf7 for e26:prof - ignored warnings for build -- details available through the CI dashboard

🚨 For more details about the warning phase, check the ci_tests SBND phase logs

parent CI build details are available through the CI dashboard

@kjplows kjplows merged commit 6413c97 into release/SBN2025A Sep 5, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

breaking change dependent An issue or PR depending on another enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants