Add reflection selection scripts - #572
Conversation
|
Hi @brownd1978,
which require these tests: build. @Mu2e/write, @Mu2e/fnalbuild-users have access to CI actions on main. ⌛ The following tests have been triggered for 63e55b2: build (Build queue - API unavailable) |
|
☀️ The build tests passed at 63e55b2.
N.B. These results were obtained from a build of this Pull Request at 63e55b2 after being merged into the base branch at 56e87eb. For more information, please check the job page here. |
oksuzian
left a comment
There was a problem hiding this comment.
PR Review Summary — #572
Reviewed at head 63e55b2b92ae78756786fe6b5b4395e782a6a4d1. First pass.
Decision
- 🟡 comment only
Both files resolve cleanly and the filter configuration is right. The one thing
worth acting on before merge is the process name, which was carried over from
recoMC/RefRegrow.fcl — the job this one is most likely to feed. The rest is
tidy-up.
Scope understood
- Two new job configs,
JobConfig/reco/RefSelect.fcland
JobConfig/recoMC/RefSelect.fcl, that runSelectReflectionson theeand
muKalSeed collections and write the events that pass either. - They are a stripped-down copy of
JobConfig/recoMC/RefRegrow.fcl: the regrow
producers, theEventIDFilter, and the second output are dropped, and the two
per-flavour outputs are merged into one.
Findings
-
🟠 [S1] Both files declare
process_name: RefRegrow, the process name of the job they are most likely to feed.- Evidence:
JobConfig/reco/RefSelect.fcl:9and
JobConfig/recoMC/RefSelect.fcl:9both sayprocess_name: RefRegrow,
whichfhicl-dumpconfirms resolves toprocess_name: "RefRegrow". The
name already belongs toJobConfig/recoMC/RefRegrow.fcl:9, so
git grep process_name JobConfig/reco JobConfig/recoMCnow returns it three
times. Every other job config in those two directories names itself:
Regrow,Reconstruct,RecoRetrigger,stripMC. The file's own output
isrec.user.RefSelect.config.seq.art, which contradicts it. - Impact: two consequences, one certain and one likely. Certain: products
written by RefSelect and by RefRegrow are indistinguishable by process name
in provenance, so a downstream"*_*_RefRegrow_*"selection cannot tell
them apart. Likely: art rejects an input file whose process history already
contains the current process name, so running RefRegrow on RefSelect output
— which is the obvious chain, given RefSelect exists to pick the ~10% that
RefRegrow would then regrow — aborts at the source. I did not run that chain
to confirm the abort; the provenance ambiguity stands either way. - Suggested fix:
process_name: RefSelectin both files. - The
Regrowname appearing twice is not the same situation: those are the
reco and recoMC variants of one job, and nobody chains one into the other.
- Evidence:
-
🟡 [S2] Two dead output stubs, and
nilis being written as a string.
reco/RefSelect.fcl:6-7setsoutputs.CentralHelixOutput.fileName : niland
the same forLoopHelixOutput, but neither output module is defined by any
prolog these files include — they exist only inJobConfig/reco/OnSpill.fcl,
OffSpill.fcland their recoMC twins.fhicl-dumpshows the result:
outputs.CentralHelixOutput: { fileName: "nil" }, a table with no
module_typeand a filename that is the four-character stringnil, not
FHiCL's@nil. Nothing is onRefEndPathbutRefOutput, so art ignores
both — they suppress nothing. Delete the two lines. The same two lines are in
the donor atrecoMC/RefRegrow.fcl:6-7and are worth the same treatment
there. -
🟡 [S2] The two files are byte-identical apart from two lines, where this directory's convention is a thin delta.
diff JobConfig/reco/RefSelect.fcl JobConfig/recoMC/RefSelect.fclreturns
exactly the include line and the output tier. The established pattern for a
reco/recoMC pair here is much smaller —recoMC/regrowLH.fclis seven lines
that include the reco version and override what differs, and
recoMC/regrowKL.fclis six. The copy also carried across the donor's header
date, so a file added on 2026-08-20 readsoriginal author: D. Brown, 6/25/26, and the reco variant's header atreco/RefSelect.fcl:2still says it starts "from standard recoMC output".- The naive form of the fix does not work here, so it is worth saying why:
reco/RefSelect.fclfreezesoutputs.RefOutput.outputCommandsfrom the
reco-flavouredReconstruction.Output, so a recoMC file that simply
included it would lose the MC keeps. The shape that does work is the one
already used in this directory — a shared epilog, as with
NoPileup_epilog.fclandOffSpillTiming_epilog.fcl— included by each
variant after its own prolog.
- The naive form of the fix does not work here, so it is worth saying why:
Validation check
- Build/tests run:
mu2e/buildtestis green at this head (FNALbuild table,
2026-08-20T22:01:48Z). That does not cover these two files — the Production
build test runsceSimReco,ceDigi,ceMix,cosmicSimRecoand the rest of
its fixed job list, none of which referenceRefSelect.fcl. As a cross-check of
what CI does not reach, I ranfhicl-dumpon both files under
SimJob/MDC2025avwith the PR head prepended toFHICL_FILE_PATH: both parse
and resolve (1359 and 1373 lines, no diagnostics). - Config contract check: pass. In the resolved config both filters come out as
module_type: "SelectReflections"withUpstreamKalSeedCollection/
DownstreamKalSeedCollectionaccepted and theSimpleKalSeedSelectortool
populated (MinMomentum: 40,MaxMomentum: 300,MinActiveHits: 15).
RefOutputresolves withmodule_type: "RootOutput"and
SelectEvents: ["RePath","RmuPath"]. - Cross-repo consistency: pass.
TrkReco.filters.SelectT0Reflectionsand
Reconstruction.Outputboth resolve from the Offline release the current
Musing pins; no Offline change is required.
Verified, no action needed
- 🟢 The MC variant does keep MC truth. I checked this specifically because the
donor needed an explicitkeep mu2e::*_compressRecoMCs_*_*and this file adds
no such line, which looked like a dropped copy. It is not:recoMC/prolog.fcl
extendsReconstruction.Output, so the resolvedoutputCommandsare 50 entries
against the reco variant's 36, a strict superset that adds
keep *_compressRecoMCs_*_*,keep *_SelectReco_*_*,
keep mu2e::PrimaryParticle_*_*_*,keep mu2e::CosmicLivetime_*_*_*and
keep mu2e::EventWeight_*_*_*. - 🟢 The appended
keep *_Reflect*_*_*is not a dead line:SelectReflections
produces aKalSeedPtrCollection, which is how the donor consumes it
(KalSeedPtrCollection : ReflecteatrecoMC/RefRegrow.fcl:52). - 🟢 The output tiers match the directory:
rec.fromreco/,mcs.from
recoMC/.
Residual risk
- I have not run either file on real input, so the ~10% selection efficiency in
the description is unverified here.
Author follow-ups
- Rename the process to
RefSelectin both files (finding 1). - Drop the two
niloutput lines (finding 2). - Consider an epilog so the two variants stop being a full copy (finding 3).
- For the record: which dataset were these run on to get the ~10% figure, and is
RefRegrow the intended consumer of the output?
Add scripts to coarsely select reflecting cosmic rays. This scripts selects ~10% of the 'Signal' cosmic ray sample.