Skip to content

Save passive-material xings in the Extracted and OffSpill reco configs - #582

Merged
brownd1978 merged 3 commits into
Mu2e:mainfrom
RobMina:crv-strongback-val
Oct 23, 2026
Merged

brownd1978 merged 3 commits into
Mu2e:mainfrom
RobMina:crv-strongback-val

Conversation

@RobMina

@RobMina RobMina commented Oct 14, 2026 •

Copy link
Copy Markdown
Contributor

Follow-up to #580 / Mu2e/Offline#2023, reworked after review (@rlcee, @brownd1978): the setting now lives in the reco
configs rather than in the nightly validation configs.

ValKalSeed's NCRVSB (CRV strongback intersections) has been identically 0 in every nightly: KKExtrap adds the
strongbacks (and, for CentralHelix, the DS shells and concrete) as passive material Xings, which KKFit::sampleFit only
copies into KalSeed::intersections() when KKFitSettings.SaveMaterialXings is true (Offline prolog default false).

Changes:

  • JobConfig/reco/Extracted.fcl, JobConfig/recoMC/Extracted.fcl: KKLine.KKFitSettings.SaveMaterialXings : true,
    replacing the obsolete "TCRV" sample surface (a deprecated test plane that KinKalGeom no longer defines, so it
    sampled nothing).
  • JobConfig/reco/OffSpill.fcl, JobConfig/recoMC/OffSpill.fcl: KKCHmu.KKFitSettings.SaveMaterialXings : true.
  • The nightly Validation/offSpill.fcl and Validation/nightly/extracted_0*.fcl pick this up through their includes;
    no Validation file changes.

Resolved configs (fhicl-dump on the 2026-10-19 nightly build, this branch vs main): each job differs only in that key
(and, for Extracted, the dropped TCRV entry). 20-event art runs of recoMC/OffSpill.fcl and recoMC/Extracted.fcl
exit 0.

Cost:

  • Extracted (1000 events of extracted_00, 10-14): KKLine intersections per track 2.46 -> 2.92 (the strongbacks:
    NCRVSB = NCRV, 326/326); KalSeeds_KKLine +0.8 %, art file +0.32 %; CPU unchanged.
  • OffSpill production config (stock recoMC/OffSpill.fcl with CHFilter, 5000 events of
    CosmicSignalOffSpill.MDC2025an_best_v1_1, same host, 10-19): KKCHmu intersections per track 2.30 -> 11.53 on the
    CentralHelix output (31 tracks) and 2.21 -> 6.56 on the LoopHelix output (313 KKCHmu tracks); NCRVSB = NCRV;
    KalSeeds_KKCHmu +12.3 % / +7.3 %; art file +0.64 % (CentralHelix output, 9.8 MB) / +0.03 % (LoopHelix output,
    1.19 GB); CPU 790 -> 777 s (one run each); LoopHelix fits identical.

This changes the output of production campaigns that use these configs (mdc2025 / mdc2030 Extracted and OffSpill reco):
the KalSeeds gain the passive-material intersections; fits are unchanged.

Expect one nightly FAIL on NCRVSB/NInter for offSpill and extracted, then PERFECT.

… and KKLine (extracted) so CRV strongback crossings are histogrammed
@FNALbuild

Copy link
Copy Markdown
Collaborator

Hi @RobMina,
You have proposed changes to files in these packages:

  • Validation

which require these tests: build.

@Mu2e/write, @Mu2e/fnalbuild-users have access to CI actions on main.

⌛ The following tests have been triggered for a17e26d: build (Build queue - API unavailable)

About FNALbuild. Code review on Mu2e/Offline.

@FNALbuild

Copy link
Copy Markdown
Collaborator

☀️ The build tests passed at a17e26d.

Test Result Details
test with ✅ Command did not list any other PRs to include
merge ✅ Merged a17e26d at 0d1c370
build (prof) ✅ Log file. Build time: 09 min 05 sec
ceSimReco ✅ Log file.
g4test_03MT ✅ Log file.
transportOnly ✅ Log file.
POT ✅ Log file.
g4study ✅ Log file.
cosmicSimReco ✅ Log file.
cosmicOffSpill ✅ Log file.
ceSteps ✅ Log file.
ceDigi ✅ Log file.
muDauSteps ✅ Log file.
ceMix ✅ Log file.
rootOverlaps ✅ Log file.
g4surfaceCheck ✅ Log file.
FIXME, TODO ✅ TODO (0) FIXME (0) in 0 files
clang-tidy ✅ 0 errors 0 warnings

N.B. These results were obtained from a build of this Pull Request at a17e26d after being merged into the base branch at 0d1c370.

For more information, please check the job page here.
Build artifacts are deleted after 5 days. If this is not desired, select Keep this build forever on the job page.

@rlcee

rlcee commented Oct 15, 2026

Copy link
Copy Markdown
Collaborator

Did you ask (probably Dave or Yuri) if this can/should be put in Production/JobConfig/recoMC/Extracted.fcl?

@oksuzian oksuzian left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

PR Review Summary — #582

Reviewed at head a17e26d474f8a4657b8ffd339751d3aad488bfcc. first pass

Decision

  • 🟢 approve. Each job changes by exactly the one key described, and nothing else changes.

Scope understood

  • Nightly validation only. SaveMaterialXings is turned on for KKCHmu in offSpill and for KKLine in extracted. That fills ValKalSeed's NCRVSB histogram, which has read 0 in every nightly since #580 and Offline#2023.
  • The new Validation/extracted.fcl wraps recoMC/Extracted.fcl and database.fcl. The five nightly/extracted_0*.fcl include it instead of those two files.

Findings

  • None.

Verified

  • 🟢 One key per job, in the intended table. I ran fhicl-dump at this head against Offline main f122c5b3, and all rc are 0. Compared with Production main:

    • offSpill differs only in KKCHmu.KKFitSettings.SaveMaterialXings: true;
    • nightly/extracted_00 and extracted_03 differ only in KKLine.KKFitSettings.SaveMaterialXings: true;
    • their input file lists and output names are unchanged.

    The key is Mu2eKinKal::KKFitConfig::saveMatXings (KKFitSettings.hh:105), so validated fhicl accepts it.

  • 🟢 The histogram this feeds exists. ValKalSeed.cc:194-216 counts SurfaceIdDetail::CRV_StrongBack intersections into NCRVSB.

  • 🟢 The wrapper follows the existing pattern. nightly/reco_0* and nightly/offSpill_0* already include a Validation/<job>.fcl wrapper. Only extracted included the JobConfig file directly. Nothing outside the five nightly files includes Validation/extracted.fcl.

  • 🟢 No conflict with #583. Both PRs edit Validation/offSpill.fcl. The merged tree is clean, and its dump carries both changes and nothing else.

  • On @rlcee's question about putting this in JobConfig/recoMC/Extracted.fcl: that file is in production use. The prodtools mdc2025 reco campaign names it at six entries, and mdc2030 at one. Moving the setting there would change campaign output, about +0.8% on KalSeeds_KKLine by your numbers. Keeping it in Validation/ leaves production untouched, so it fits a validation-only PR. Whether production wants it is for the reco conveners.

Validation check

  • Build/tests run: FNALbuild passed at this head, but no buildtest job runs the Validation/offSpill or extracted jobs. Locally I ran fhicl-dump at main, at this head and merged with #583 (claude-scratch/pr583_582_review/fcldump.sh). I did not run an art job, because the inputs are on tape and I have no valid token. The author's 1000-event before/after comparison was not reproduced.
  • Config contract check: pass.
  • Cross-repo consistency: pass. Offline#2023 and Production#580 are merged.

Residual risk

  • Validation only. Expect one nightly FAIL on NCRVSB and NInter, as the PR says.

Author follow-ups

  • None.

@brownd1978 brownd1978 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

As suggested by @rlcee this fix belongs in the reco config.

Comment thread Validation/extracted.fcl Outdated
…KCHmu) reco configs instead of the nightly validation configs; drop the obsolete TCRV sample surface
@RobMina

RobMina commented Oct 19, 2026

Copy link
Copy Markdown
Contributor Author

I added the line to the production config, as requested. I tested the impact on CH OffSpill reco and found that it was negligible (no measurable change in CPU time, <1% increase in art output size).

@oksuzian oksuzian left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

PR Review Summary — #582

Reviewed at head 2889c7cae30fd447e995b6cbc7dba3e71bcf0c8c. re-review of a17e26d4

Decision

  • 🟡 comment for now; the code is approvable. Moving the setting into the reco configs is what @brownd1978 and @rlcee asked for. Each job changes only in SaveMaterialXings, plus the TCRV removal, which was already a no-op. The fit itself does not change. This now reaches production campaigns, so the scope is set out below for the record. There is no build at this head yet, so I am triggering one (with mu2e-trig-config#176, which Offline main currently needs) and will approve when it is green.

Scope understood

  • SaveMaterialXings : true moves from the two validation wrappers into JobConfig/reco{,MC}/Extracted.fcl (KKLine) and JobConfig/reco{,MC}/OffSpill.fcl (KKCHmu).
  • Validation/extracted.fcl is deleted, and the five extracted_0* nightlies include recoMC/Extracted.fcl plus database.fcl directly, as on main.
  • "TCRV" is dropped from KKLine.ModuleSettings.SampleSurfaces in both Extracted.fcl.

Findings

  1. ⚪ [S3] JobConfig/recoMC/NoFieldRun1B.fcl:11 still appends "TCRV" to KKLine's SampleSurfaces, with the same "temporary until extrapolation is working" comment. It is a no-op there too (see below), so dropping it in the same commit would finish the cleanup.

🟢 Verified at this head:

  • Only the intended keys change. I ran fhicl-dump on all 12 affected files (the four job configs, Validation/offSpill.fcl, the five extracted_0* nightlies, offSpill_00 and the deleted wrapper) at Production main (efebc0a2), at a17e26d4, at this head, and with this head merged onto main. Offline main was b8cfd1d7 throughout. Every dump parses.
    • Merged onto main, each Extracted.fcl and each nightly differs only in KKLine SaveMaterialXings: false → true and the missing "TCRV".
    • Each OffSpill.fcl differs only in KKCHmu SaveMaterialXings.
    • Validation/offSpill.fcl and offSpill_00 dump the same as at a17e26d4; the setting now comes from recoMC/OffSpill.fcl.
    • The branch predates #583. The merge with main is clean and keeps #583's field-correction settings.
  • The flag changes storage, not the fit. KKFit.hh:902 is the only reader of SaveMaterialXings. It copies the passive crossings that extrapolation already made (materialCylXings, materialPlaneXings) into KalSeed::intersections(). No new product is made, and keep mu2e::KalSeeds_K*_*_* already writes KKLine and KKCHmu. The size numbers in the description (+0.07% offSpill, +0.32% extracted) are yours; I did not rerun them.
  • Dropping TCRV changes nothing. TCRV (200) is marked "deprecated" in DataProducts/inc/SurfaceId.hh. No surface with that ID is ever added to the KinKalGeom map (GeometryService/src/KinKalGeomMaker.cc), so kkg.surfaces() returns nothing for it, and sampling at TCRV never produced an intersection. A Mu2e org code search finds no reader of the TCRV surface ID; the Stntuple hits are TCrv* class names.
  • Downstream readers.
    • ValKalSeed changes only NInter and NCRVSB, as you expect for the nightly.
    • ArtAnalysis CrvInference picks TT_Mid by surface ID, so the extra entries do not affect it.
    • EventNtuple fillTrkSegInfo copies every intersection into segs with its sid. Ntuples from these jobs will therefore carry extra segs entries: sid 280 (CRV_StrongBack) for extracted, and DS shell, coil, absorber and hatch-concrete IDs for off-spill. That is the intent, but anyone who counts segs without selecting on sid will see a change.

Blast radius

  • recoMC/Extracted.fcl is used by mdc2025 CosmicCRYExtracted(Triggered)Reco (6 entries in reco.json) and by mdc2030 (1 entry).
  • recoMC/OffSpill.fcl is used by mdc2025 CosmicSignalOffSpillTriggered (2 entries).
  • reco/Extracted.fcl (data) is included by PassN Tracker/TrkRecoFromDigi.fcl, whose epilog does not override SampleSurfaces or SaveMaterialXings. So PassN tracker reco gets the same change. reco/OffSpill.fcl has no consumer in the org.
  • The campaigns run pinned SimJob musings, so none of this reaches them until the next SimJob tag.

Validation check

  • Build/tests run: no FNALbuild run at this head yet (mu2e/buildtest: "This test has not been triggered yet"). a17e26d4 passed. Locally: fhicl-dump as above (claude-scratch/pr582_r2/fcldump.sh, dumps in dumps/). No art job run; your 1000-event before/after on gpvm is the only execution evidence.
  • Config contract check: pass.
  • Cross-repo consistency: pass. PassN inherits the change, which is consistent with what it does.

Author follow-ups

  1. Optional: drop the leftover "TCRV" in recoMC/NoFieldRun1B.fcl (finding 1).

@oksuzian

Copy link
Copy Markdown
Collaborator

@FNALbuild run build test with Mu2e/mu2e-trig-config#176

@FNALbuild

Copy link
Copy Markdown
Collaborator

⌛ The following tests have been triggered for 2889c7c: build (Build queue - API unavailable)

@FNALbuild

Copy link
Copy Markdown
Collaborator

☀️ The build tests passed at 2889c7c.

Test Result Details
test with ✅ Included Mu2e/mu2e-trig-config#176 @ 1b2e6bbd962c2d704d4b577fd46c4e0f507986c3 by merge
merge ✅ Merged 2889c7c at efebc0a
build (prof) ✅ Log file. Build time: 04 min 33 sec
ceSimReco ✅ Log file.
g4test_03MT ✅ Log file.
transportOnly ✅ Log file.
POT ✅ Log file.
g4study ✅ Log file.
cosmicSimReco ✅ Log file.
cosmicOffSpill ✅ Log file.
ceSteps ✅ Log file.
ceDigi ✅ Log file.
muDauSteps ✅ Log file.
ceMix ✅ Log file.
rootOverlaps ✅ Log file.
g4surfaceCheck ✅ Log file.
FIXME, TODO ✅ TODO (0) FIXME (0) in 0 files
clang-tidy ✅ 0 errors 0 warnings

N.B. These results were obtained from a build of this Pull Request at 2889c7c after being merged into the base branch at efebc0a.

For more information, please check the job page here.
Build artifacts are deleted after 5 days. If this is not desired, select Keep this build forever on the job page.

@oksuzian oksuzian left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

PR Review Summary — #582

Reviewed at head 2889c7cae30fd447e995b6cbc7dba3e71bcf0c8c. re-review of a17e26d4

Decision

  • 🟢 approve. Moving the setting into the reco configs is what @brownd1978 and @rlcee asked for. Each job changes only in SaveMaterialXings, plus the TCRV removal, which was already a no-op. The fit itself does not change. This now reaches production campaigns, so the scope is set out below for the record.

Scope understood

  • SaveMaterialXings : true moves from the two validation wrappers into JobConfig/reco{,MC}/Extracted.fcl (KKLine) and JobConfig/reco{,MC}/OffSpill.fcl (KKCHmu).
  • Validation/extracted.fcl is deleted, and the five extracted_0* nightlies include recoMC/Extracted.fcl plus database.fcl directly, as on main.
  • "TCRV" is dropped from KKLine.ModuleSettings.SampleSurfaces in both Extracted.fcl.

Findings

  1. ⚪ [S3] JobConfig/recoMC/NoFieldRun1B.fcl:11 still appends "TCRV" to KKLine's SampleSurfaces, with the same "temporary until extrapolation is working" comment. It is a no-op there too (see below), so dropping it in the same commit would finish the cleanup.

🟢 Verified at this head:

  • Only the intended keys change. I ran fhicl-dump on all 12 affected files (the four job configs, Validation/offSpill.fcl, the five extracted_0* nightlies, offSpill_00 and the deleted wrapper) at Production main (efebc0a2), at a17e26d4, at this head, and with this head merged onto main. Offline main was b8cfd1d7 throughout. Every dump parses.
    • Merged onto main, each Extracted.fcl and each nightly differs only in KKLine SaveMaterialXings: false → true and the missing "TCRV".
    • Each OffSpill.fcl differs only in KKCHmu SaveMaterialXings.
    • Validation/offSpill.fcl and offSpill_00 dump the same as at a17e26d4; the setting now comes from recoMC/OffSpill.fcl.
    • The branch predates #583. The merge with main is clean and keeps #583's field-correction settings.
  • The flag changes storage, not the fit. KKFit.hh:902 is the only reader of SaveMaterialXings. It copies the passive crossings that extrapolation already made (materialCylXings, materialPlaneXings) into KalSeed::intersections(). No new product is made, and keep mu2e::KalSeeds_K*_*_* already writes KKLine and KKCHmu. The size numbers in the description (+0.07% offSpill, +0.32% extracted) are yours; I did not rerun them.
  • Dropping TCRV changes nothing. TCRV (200) is marked "deprecated" in DataProducts/inc/SurfaceId.hh. No surface with that ID is ever added to the KinKalGeom map (GeometryService/src/KinKalGeomMaker.cc), so kkg.surfaces() returns nothing for it, and sampling at TCRV never produced an intersection. A Mu2e org code search finds no reader of the TCRV surface ID; the Stntuple hits are TCrv* class names.
  • Downstream readers.
    • ValKalSeed changes only NInter and NCRVSB, as you expect for the nightly.
    • ArtAnalysis CrvInference picks TT_Mid by surface ID, so the extra entries do not affect it.
    • EventNtuple fillTrkSegInfo copies every intersection into segs with its sid. Ntuples from these jobs will therefore carry extra segs entries: sid 280 (CRV_StrongBack) for extracted, and DS shell, coil, absorber and hatch-concrete IDs for off-spill. That is the intent, but anyone who counts segs without selecting on sid will see a change.

Blast radius

  • recoMC/Extracted.fcl is used by mdc2025 CosmicCRYExtracted(Triggered)Reco (6 entries in reco.json) and by mdc2030 (1 entry).
  • recoMC/OffSpill.fcl is used by mdc2025 CosmicSignalOffSpillTriggered (2 entries).
  • reco/Extracted.fcl (data) is included by PassN Tracker/TrkRecoFromDigi.fcl, whose epilog does not override SampleSurfaces or SaveMaterialXings. So PassN tracker reco gets the same change. reco/OffSpill.fcl has no consumer in the org.
  • The campaigns run pinned SimJob musings, so none of this reaches them until the next SimJob tag.

Validation check

  • Build/tests run: FNALbuild passed at this head, built with mu2e-trig-config#176 (which Offline main currently needs) and merged onto Production efebc0a2. Locally: fhicl-dump as above (claude-scratch/pr582_r2/fcldump.sh, dumps in dumps/). No art job run; your 1000-event before/after on gpvm is the only execution evidence.
  • Config contract check: pass.
  • Cross-repo consistency: pass. PassN inherits the change, which is consistent with what it does.

Author follow-ups

  1. Optional: drop the leftover "TCRV" in recoMC/NoFieldRun1B.fcl (finding 1).

@RobMina RobMina changed the title Nightly validation: save passive-material xings Save passive-material xings in the Extracted and OffSpill reco configs Oct 22, 2026
@RobMina

RobMina commented Oct 22, 2026

Copy link
Copy Markdown
Contributor Author

@FNALbuild run build test

@FNALbuild

Copy link
Copy Markdown
Collaborator

⌛ The following tests have been triggered for 0daa7b9: build (Build queue - API unavailable)

@FNALbuild

Copy link
Copy Markdown
Collaborator

☀️ The build tests passed at 0daa7b9.

Test Result Details
test with ✅ Command did not list any other PRs to include
merge ✅ Merged 0daa7b9 at ac5d9c9
build (prof) ✅ Log file. Build time: 04 min 35 sec
ceSimReco ✅ Log file.
g4test_03MT ✅ Log file.
transportOnly ✅ Log file.
POT ✅ Log file.
g4study ✅ Log file.
cosmicSimReco ✅ Log file.
cosmicOffSpill ✅ Log file.
ceSteps ✅ Log file.
ceDigi ✅ Log file.
muDauSteps ✅ Log file.
ceMix ✅ Log file.
rootOverlaps ✅ Log file.
g4surfaceCheck ✅ Log file.
FIXME, TODO ✅ TODO (0) FIXME (0) in 0 files
clang-tidy ✅ 0 errors 0 warnings

N.B. These results were obtained from a build of this Pull Request at 0daa7b9 after being merged into the base branch at ac5d9c9.

For more information, please check the job page here.
Build artifacts are deleted after 5 days. If this is not desired, select Keep this build forever on the job page.

@oksuzian oksuzian left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

PR Review Summary — #582

Reviewed at head 0daa7b9feaabbf1c5228a856c24045fbdbbe7218. re-review of 2889c7c

Decision

  • 🟢 approve

Scope understood

  • One new commit, 0daa7b9f, removes the same "temporary until extrapolation is working" TCRV sample-surface line from recoMC/NoFieldRun1B.fcl that this PR already removed from reco/ and recoMC/Extracted.fcl.

Verified

  • 🟢 I ran fhicl-dump of recoMC/NoFieldRun1B.fcl, reco/Extracted.fcl and recoMC/Extracted.fcl at 2889c7ca and at this head. The only difference anywhere is that "TCRV" leaves the three KKFitSettings.ModuleSettings.SampleSurfaces lists in NoFieldRun1B, which now end at TT_Outer. Both Extracted configs are identical.
  • 🟢 The CRV is still reached in that job: the KKLine ExtrapolationSettings in NoFieldRun1B has ToCRV: true, unchanged by this PR. That is the extrapolation the removed line was standing in for, and it matches @brownd1978's point that the line is obsolete.
  • 🟢 FNALbuild passed at this head (merged onto main at ac5d9c91).

Validation check

  • Build/tests run: the fhicl-dump comparisons above, against Offline from the CI build bb141c56_ac5d9c91_32aec90c. No events were processed, so I have not compared the CRV intersections in the output with and without the sample surface.
  • Config contract check: pass.
  • Cross-repo consistency: pass.

Residual risk

  • recoMC/NoFieldRun1B.fcl is the reco config of the Run1B campaign, so this changes production output: Run1B KalSeeds will carry the CRV intersection from extrapolation only, no longer also from surface sampling. I expect that to be the intended result, but it was not checked on events.

Note on merge state

  • @brownd1978's change request was made at a17e26d4, before the line moved into reco/Extracted.fcl and was added to the OffSpill configs. Both of his requests look done, but his review state still blocks the merge until he re-reviews.

@oksuzian oksuzian left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

PR Review Summary — #582

Reviewed at head 0daa7b9feaabbf1c5228a856c24045fbdbbe7218. Correction to my re-review at the same head.

Decision

  • 🟡 comment only. My approval stands; this corrects one statement in it.

I wrote under residual risk that Run1B KalSeeds would now carry the CRV intersection from extrapolation only, no longer also from surface sampling. That was wrong. On Offline main (78ea1fd5), TCRV is a deprecated SurfaceId (DataProducts/inc/SurfaceId.hh:25) that KinKalGeomMaker.cc never adds to the KinKal surface map, and KKFit::sampleFit only finds surfaces in that map (KinKalGeom::surfaces does a map_.equal_range). So sampling TCRV never produced an intersection, and dropping it from recoMC/NoFieldRun1B.fcl should not change the Run1B output. This is from reading the code; I did not run a job.

@brownd1978 brownd1978 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Thanks for making this change

@brownd1978
brownd1978 merged commit 42da13c into Mu2e:main Oct 23, 2026
14 checks passed
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.

5 participants