Gen2 Production PR: Manual Fix for GENIE timing bug in CAFMaker + Extend existing-data shift in CAFMaker by VCLanNguyen · Pull Request #675 · SBNSoftware/sbncode

VCLanNguyen · 2026-09-25T17:38:14Z

Description:

Adds SRTrueInteraction.prod_time/.dk2gen field
Adds SRMeVPrtl.prod_time
Assigns rec.mc.nu.generator in CAFMaker_module.cc — previously unset
Adds CorrectMCTiming() (MC only, nu.generator == kGENIE): computes a per-true-neutrino offset_ns = nu.prod_time + nu.dk2gen × 100/c, applies each slice's matched neutrino's offset to slc.truth, opt0/opt0_sec, barycenterFM, all fmatch*, crumbs_result, correctedOpFlash.*, matched CRT tracks
Extends pre-existing data-side shift (SBNDShiftCRTReference/SBNDShiftPMTReference) to also cover opt0_sec, barycenterFM, fmatch*, crumbs_result

Validation:

See slide 1-26 on docdb#49588

Dependency:

Gen2 Production:

sbncode: #675
sbnanaobj: SBNSoftware/sbnanaobj#199
sbndcode: SBNSoftware/sbndcode#967

Develop:

sbncode: #676 -- to mirror description bullet 1/2/3
sbnanaobj: SBNSoftware/sbnanaobj#200 -- to mirror description bullet 1/2/3

Checklist:

Have you added a label? (bug/enhancement/physics etc.)
Have you assigned at least 1 reviewer?
Is this PR related to an open issue / project?
Does this PR affect CAF data format? If so, please assign a CAF maintainer as additional reviewer. @Gianluca Petrillo and @thomas Jones
Does this PR require merging another PR in a different repository (such as sbnanobj/sbnobj etc.)? If so, please link it in the description.
Are you submitting this PR on behalf of someone else who made the code changes? If so, please mention them in the description.

…ycenterFM, fmatch variants, and CRUMBS features Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Lxqnx6r8HqPPywzXcxBahz

…Maker Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Lxqnx6r8HqPPywzXcxBahz

…coped reco objects Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Lxqnx6r8HqPPywzXcxBahz

Fields shifted by CorrectMCTiming()/SBNDShiftCRTReference()/SBNDShiftPMTReference()/ FixPMTReferenceTimes() came from optional slice-scoped associations (OpT0Finder candidate, TPCPMTBarycenterMatch, SimpleFlashMatch, CRUMBSResult) that aren't guaranteed to exist for a given slice. Shifting an absent association's field turned its honest "not filled" sentinel into a bogus near-default value that looked filled but wasn't. Each field is now guarded against its own struct's actual unfilled convention (NaN for structs with no setDefault(), -9999. for setDefault()-based structs and the upstream SimpleFlashMatch producer's own sentinel), confirmed individually from source and empirically rather than assumed uniform.

CorrectMCTiming() recovers a dk2nu-specific bug (missing meson decay time in the stored vertex time) and so should only ever touch interactions that came from GENIE. But rec.mc.nu is filled from whatever generator produced MCTruth under the configured producer label, not GENIE specifically -- HNL/dissonant- Higgs productions (MeVPrtlGen) reuse the same "generator" label and push their own MCTruth there too. The correction happened to still skip those in current production configs, but only because MeVPrtlGen never produces the bsim::Dk2Nu product the correction depends on -- incidental, not an explicit guarantee. Sets SRTrueInteraction.generator (declared, previously never assigned) from whether a simb::GTruth was associated with the interaction -- GTruth only ever comes from GENIE -- and gates the correction on it directly.

SBNDShiftPMTReference() was shifting slc.correctedOpFlash.OpFlashT0/ .OpFlashT0Corrected by the SBND frame shift, but for data these fields are already reference-frame corrected upstream: LightPropagationCorrection_module.cc computes them from the real SPECTDC RWM/trigger timestamps in its data-only (!fIsMC) branch. Shifting them again here double-corrected the reference time. Same category as soft_trig.flash_peaktime/sbnd_crt_veto.sp_time, already left alone for this reason. MC is unaffected: for MC that module's !fIsMC branch is skipped, so correctedOpFlash is unshifted at the producer level and CorrectMCTiming()'s own dk2nu-offset correction of it remains necessary.

…default simb::MCFlux::fdk2gen defaults to -999. (its own Reset()), a different sentinel than the -9999. convention every other guard in this function checks. Without this, an interaction with an unfilled dk2gen would compute a wrong-but-not-NaN offset_ns and silently corrupt every field CorrectMCTiming shifts, the same failure class as the earlier -9999. guard fix. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

…tions Trim CorrectMCTiming()/SBNDShiftCRTReference()/SBNDShiftPMTReference()/ FixPMTReferenceTimes() comments from prose paragraphs to short per-field labels, and record that SRSBNDCRTVeto.sp_time, SRSoftwareTrigger.flash_peaktime, and correctedOpFlash.OpFlashT0/.OpFlashT0Corrected (data path) are already reference-frame corrected upstream in their own producer modules (CRTVeto, SoftwareTrigger, LightPropagationCorrection) rather than left as an open watch-list item. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

…nerator Previously any MCTruth without an associated GTruth (i.e. non-GENIE) was tagged kUnknownGenerator, even though MeVPrtlGen_module.cc pushes a matching MeVPrtlTruth into the same event at the same index as its MCTruth (confirmed in its produce(): the two collections are always pushed back-to-back in the same loop iteration). Use that index alignment -- the same assumption this function already relies on for mcfluxes.at(i)/dk2nus.at(i) -- to tag those entries kMeVPrtl instead. caf::kMeVPrtl already existed in SREnums.h but nothing was setting it. CorrectMCTiming()'s existing `generator != caf::kGENIE` guard needs no change: kMeVPrtl still isn't kGENIE, so those slices are still skipped. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

This function runs unconditionally for both MC and data with no detector gate (called for ICARUS too), so guarding it is out of scope for this SBND-only dk2nu/gate-shift project -- back to unconditionally shifting fmatch.time/barycenterFM.flashTime/.flashFirstHit, matching pre-guard-pass behavior byte-for-byte (comment included), so ICARUS CAF output is unaffected.

Companion to the sbnanaobj SRMeVPrtl::prod_time field: sources it from MeVPrtlTruth::meson_dpos_beamcoord.T(), the clean pre-beam-offset meson decay time (already threaded from the BNB flux ntuple's ini_t[0]). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

VCLanNguyen · 2026-10-07T20:30:32Z

Hi @tjones79 and/or @PetrilloAtWork can you guys have a look to review? I have attached the docdb that has validation plots to cover all the expected changes (and also unchanges).

tjones79

Just the one comment. Everything else looks good to me. Thanks :)

tjones79 · 2026-10-12T10:08:14Z

+    offset_ns[i] = nu.prod_time + nu.dk2gen * 100. / kSpeedOfLight;
+    double const delta_us = offset_ns[i] / 1000.;
+
+    nu.time += delta_us;


Looking at your validation plots (Thank you very much for these; they are extremely helpful!!)...

Your plots for rec.mc.nu.time and the prim truth times (Slides 7 and 8) show a clear ~350 ns shift. But the current code in the PR adds delta_us to these fields, which, naively, I would expect only to shift them by ~0.35 ns. Should this not actually be += offset_ns[i]?

(Looking at your other PR: https://github.com/SBNSoftware/sbnanaobj/pull/200/changes#diff-14e572d921caad14ab8a3f1b14c6af56c1dec7e9d457dce906c7024ed0e89d1bL57 The interaction time is also explicitly defined in ns in your comment)

The units are in us so the correction offset is ~350ns or ~0.35us.
I corrected the comments and also clearly labelled the units of the offset variables

PetrilloAtWork

I feel I am missing an important point of this PR.
It unconditionally shifts all (or many) times to the time reference of the decay of the meson which originated the neutrino/particle. Is that correct? if so, why?
I see the mention of a bug in dk2nu, but it's not clear to me how this correction would leave alone input data not affected by that bug.
I have left some technical comments across the code. Once those are resolved, for me this PR is good to go in the target branch. But until the questions above are answered, I think the PR in this state should not be merged into develop.

PetrilloAtWork · 2026-10-12T21:32:04Z

+    for (SRTrueParticle &part: nu.prim) {
+      if (part.genT   > -9998.) part.genT   += delta_us;
+      if (part.startT > -9998.) part.startT += delta_us;
+      if (part.endT   > -9998.) part.endT   += delta_us;


For your information: I have introduced an helper that should retrieve the default value of a field (in this case the test would look would look like: part.genT != caf::SRDefaults::For(part).genT).
As usual in C++, it is a mouthful but it saves you from putting magic values in the code.
However, that helper is not integrated in the branch you are targeting (and I think not yet in develop either).

I can't locate SRDefaults in the production/sbnd-gen2 branch so will keep this guard.

- Fill SRTrueInteraction.baseline/dk2gen natively in cm (util::kMeterToCentimeter) instead of converting with a magic *100 inside CorrectMCTiming - Replace CorrectMCTiming's duplicated truth-time shift blocks with shiftMCParticleTimes/shiftTruthTimes lambdas, reused for both rec.mc.nu and the slice-matched slc.truth - Swap the local kSpeedOfLight constant for util::kc; add Doxygen docs on CorrectMCTiming - Demote the CorrectMCTiming and fSubRunPOT/frameApplyAtCaf debug std::cout prints to mf::LogDebug - Fetch dk2nu via a raw product handle instead of an art::Ptr vector; use fParams.GenLabel() for both the mcflux and dk2nu handle fetches instead of a hardcoded "generator" string - const-qualify the placeholder baddk2nu; de-duplicate the generator_ enum assignment via a local reference Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

Converts dk2nu's raw ns-valued ancestor startt to us at fill time, matching the README's Time=us convention and the sibling time field's existing ns->us fill pattern. CorrectMCTiming()'s offset formula reworked to match: prod_time is multiplied back by 1000 to recombine with the still-ns-valued dk2gen/util::kc term, so offset_vector_ns keeps its existing ns meaning unchanged -- only how it's assembled changed. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

VCLanNguyen · 2026-10-14T19:09:26Z

I feel I am missing an important point of this PR. It unconditionally shifts all (or many) times to the time reference of the decay of the meson which originated the neutrino/particle. Is that correct? if so, why? I see the mention of a bug in dk2nu, but it's not clear to me how this correction would leave alone input data not affected by that bug. I have left some technical comments across the code. Once those are resolved, for me this PR is good to go in the target branch. But until the questions above are answered, I think the PR in this state should not be merged into develop.

@PetrilloAtWork

This PR targets the production/sbnd-gen2 branch.
The mirror PR for develop, listed below, only cover the description bullet 1/2/3, which just extends fields of sbnanaobj.
They do NOT cover description bullet 4/5, and therefore do NOT introduce any corrections automatically.

In the scope of production/sbnd-gen2 , SBND Gen2 production file is already generated with this dk2nu bug (~10M MC events).
The SBND Gen2 production respin is not planned to run from generation stage again, but only reco2->caf.
So the proposal is to put a bug fix at CAFMaker for production/sbnd-gen2 , which is to run ONLY SBND Gen2 production files.

In the CorrectionMCTiming function, several guards are placed:
Pre-function:

if SBND -> to avoid ICARUS. However, this is not a joint production with ICARUS.
if MC -> to avoid data

Inside-function:
3. if GENIE -> to avoid other generator types, i.e. MeVPrtl
4. if valid truth variables to compute correction (prodtime & dk2gen) -> only apply correction if can be computed

VCLanNguyen and others added 4 commits September 25, 2026 12:34

Extend SBND data-stream CRT/PMT reference-time shift to opt0_sec, bar…

a8a5c66

…ycenterFM, fmatch variants, and CRUMBS features Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Lxqnx6r8HqPPywzXcxBahz

Fill SRTrueInteraction.prod_time and .dk2gen from dk2nu/MCFlux in CAF…

28ff628

…Maker Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Lxqnx6r8HqPPywzXcxBahz

Correct dk2nu v01_11_00 vertex-time bug for SBND MC truth and slice-s…

865a4b1

…coped reco objects Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Lxqnx6r8HqPPywzXcxBahz

VCLanNguyen marked this pull request as draft September 25, 2026 17:38

VCLanNguyen added dependent An issue or PR depending on another bugfix Addresses one or more bugs labels Sep 25, 2026

VCLanNguyen changed the title ~~Bugfix gen2 mctiming merge correctedopflash~~ BugFix in CAFMaker for dk2nu v1_11_00 Sep 25, 2026

VCLanNguyen changed the title ~~BugFix in CAFMaker for dk2nu v1_11_00~~ Bug Fix in CAFMaker for dk2nu v1_11_00 Sep 25, 2026

VCLanNguyen mentioned this pull request Sep 25, 2026

Gen2 Production PR: Manual Fix for GENIE timing bug in CAFMaker + Extend existing-data shift in CAFMaker SBNSoftware/sbnanaobj#199

Open

5 tasks done

VCLanNguyen added 2 commits September 25, 2026 12:54

VCLanNguyen assigned VCLanNguyen and jorge-romeo Sep 25, 2026

VCLanNguyen and others added 8 commits September 28, 2026 17:48

Drop redundant flash-match guard comment in FixPMTReferenceTimes

9752c09

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

Comment MeVPrtl prod_time fill to match dev tree wording

0831e92

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

Add debug print to CorrectMCTiming to confirm it runs

31fad5a

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

VCLanNguyen changed the title ~~Bug Fix in CAFMaker for dk2nu v1_11_00~~ Gen2 Production PR: Sep 30, 2026

VCLanNguyen changed the title ~~Gen2 Production PR:~~ Gen2 Production PR: 1. Manual Fix for GENIE timing bug in CAFMaker, 2. Extend existing-data shift in CAFMaker, 3. Update to SBND LightPropagation Module, 3. Sep 30, 2026

VCLanNguyen mentioned this pull request Sep 30, 2026

Gen2 Production PR: Update to SBND LightPropagation Module SBNSoftware/sbndcode#967

Open

4 of 7 tasks

Add debug print for fSubRunPOT/frameApplyAtCaf gate

c6c16ea

VCLanNguyen marked this pull request as ready for review October 1, 2026 18:55

VCLanNguyen requested a review from PetrilloAtWork October 1, 2026 18:56

VCLanNguyen requested a review from tjones79 October 1, 2026 18:56

tjones79 requested changes Oct 12, 2026

View reviewed changes

PetrilloAtWork requested changes Oct 12, 2026

View reviewed changes

linyan-w added this to SBND 2025 Gen2 Production Oct 13, 2026

linyan-w moved this to Waiting on Reviewer in SBND 2025 Gen2 Production Oct 13, 2026

VCLanNguyen and others added 2 commits October 14, 2026 13:56

VCLanNguyen force-pushed the bugfix_gen2_mctiming_merge_correctedopflash branch from 2989cb1 to 9246e33 Compare October 14, 2026 19:00

Navigation Menu

Repository navigation

Gen2 Production PR: Manual Fix for GENIE timing bug in CAFMaker + Extend existing-data shift in CAFMaker - #675

Gen2 Production PR: Manual Fix for GENIE timing bug in CAFMaker + Extend existing-data shift in CAFMaker#675
VCLanNguyen wants to merge 17 commits into
production/sbnd-gen2from
bugfix_gen2_mctiming_merge_correctedopflash

VCLanNguyen commented Sep 25, 2026 •

edited by PetrilloAtWork

Loading

Uh oh!

VCLanNguyen commented Oct 7, 2026

Uh oh!

tjones79 left a comment

Uh oh!

tjones79 Oct 12, 2026 •

edited

Loading

Uh oh!

VCLanNguyen Oct 14, 2026

Uh oh!

PetrilloAtWork left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

PetrilloAtWork Oct 12, 2026

Uh oh!

VCLanNguyen Oct 14, 2026

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

VCLanNguyen commented Oct 14, 2026 •

edited

Loading

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

Conversation

VCLanNguyen commented Sep 25, 2026 • edited by PetrilloAtWork Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description:

Validation:

Dependency:

Gen2 Production:

Develop:

Checklist:

Uh oh!

VCLanNguyen commented Oct 7, 2026

Uh oh!

tjones79 left a comment

Choose a reason for hiding this comment

Uh oh!

tjones79 Oct 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

VCLanNguyen Oct 14, 2026

Choose a reason for hiding this comment

Uh oh!

PetrilloAtWork left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

PetrilloAtWork Oct 12, 2026

Choose a reason for hiding this comment

Uh oh!

VCLanNguyen Oct 14, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

VCLanNguyen commented Oct 14, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

VCLanNguyen commented Sep 25, 2026 •

edited by PetrilloAtWork

Loading

tjones79 Oct 12, 2026 •

edited

Loading

VCLanNguyen commented Oct 14, 2026 •

edited

Loading