Skip to content

Add reflection selection scripts - #572

Open
brownd1978 wants to merge 3 commits into
Mu2e:mainfrom
brownd1978:refsel
Open

Add reflection selection scripts#572
brownd1978 wants to merge 3 commits into
Mu2e:mainfrom
brownd1978:refsel

Conversation

@brownd1978

Copy link
Copy Markdown
Collaborator

Add scripts to coarsely select reflecting cosmic rays. This scripts selects ~10% of the 'Signal' cosmic ray sample.

@brownd1978
brownd1978 requested a review from bonventre August 20, 2026 21:53
@FNALbuild

Copy link
Copy Markdown
Collaborator

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

  • JobConfig

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)

About FNALbuild. Code review on Mu2e/Offline.

@FNALbuild

Copy link
Copy Markdown
Collaborator

☀️ The build tests passed at 63e55b2.

Test Result Details
test with Command did not list any other PRs to include
merge Merged 63e55b2 at 56e87eb
build (prof) Log file. Build time: 04 min 21 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 63e55b2 after being merged into the base branch at 56e87eb.

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 — #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.fcl and
    JobConfig/recoMC/RefSelect.fcl, that run SelectReflections on the e and
    mu KalSeed collections and write the events that pass either.
  • They are a stripped-down copy of JobConfig/recoMC/RefRegrow.fcl: the regrow
    producers, the EventIDFilter, and the second output are dropped, and the two
    per-flavour outputs are merged into one.

Findings

  1. 🟠 [S1] Both files declare process_name: RefRegrow, the process name of the job they are most likely to feed.

    • Evidence: JobConfig/reco/RefSelect.fcl:9 and
      JobConfig/recoMC/RefSelect.fcl:9 both say process_name: RefRegrow,
      which fhicl-dump confirms resolves to process_name: "RefRegrow". The
      name already belongs to JobConfig/recoMC/RefRegrow.fcl:9, so
      git grep process_name JobConfig/reco JobConfig/recoMC now returns it three
      times. Every other job config in those two directories names itself:
      Regrow, Reconstruct, RecoRetrigger, stripMC. The file's own output
      is rec.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: RefSelect in both files.
    • The Regrow name appearing twice is not the same situation: those are the
      reco and recoMC variants of one job, and nobody chains one into the other.
  2. 🟡 [S2] Two dead output stubs, and nil is being written as a string.
    reco/RefSelect.fcl:6-7 sets outputs.CentralHelixOutput.fileName : nil and
    the same for LoopHelixOutput, but neither output module is defined by any
    prolog these files include — they exist only in JobConfig/reco/OnSpill.fcl,
    OffSpill.fcl and their recoMC twins. fhicl-dump shows the result:
    outputs.CentralHelixOutput: { fileName: "nil" }, a table with no
    module_type and a filename that is the four-character string nil, not
    FHiCL's @nil. Nothing is on RefEndPath but RefOutput, so art ignores
    both — they suppress nothing. Delete the two lines. The same two lines are in
    the donor at recoMC/RefRegrow.fcl:6-7 and are worth the same treatment
    there.

  3. 🟡 [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.fcl returns
    exactly the include line and the output tier. The established pattern for a
    reco/recoMC pair here is much smaller — recoMC/regrowLH.fcl is seven lines
    that include the reco version and override what differs, and
    recoMC/regrowKL.fcl is six. The copy also carried across the donor's header
    date, so a file added on 2026-08-20 reads original author: D. Brown, 6/25/26, and the reco variant's header at reco/RefSelect.fcl:2 still 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.fcl freezes outputs.RefOutput.outputCommands from the
      reco-flavoured Reconstruction.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.fcl and OffSpillTiming_epilog.fcl — included by each
      variant after its own prolog.

Validation check

  • Build/tests run: mu2e/buildtest is green at this head (FNALbuild table,
    2026-08-20T22:01:48Z). That does not cover these two files — the Production
    build test runs ceSimReco, ceDigi, ceMix, cosmicSimReco and the rest of
    its fixed job list, none of which reference RefSelect.fcl. As a cross-check of
    what CI does not reach, I ran fhicl-dump on both files under
    SimJob/MDC2025av with the PR head prepended to FHICL_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" with UpstreamKalSeedCollection /
    DownstreamKalSeedCollection accepted and the SimpleKalSeedSelector tool
    populated (MinMomentum: 40, MaxMomentum: 300, MinActiveHits: 15).
    RefOutput resolves with module_type: "RootOutput" and
    SelectEvents: ["RePath","RmuPath"].
  • Cross-repo consistency: pass. TrkReco.filters.SelectT0Reflections and
    Reconstruction.Output both 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 explicit keep mu2e::*_compressRecoMCs_*_* and this file adds
    no such line, which looked like a dropped copy. It is not: recoMC/prolog.fcl
    extends Reconstruction.Output, so the resolved outputCommands are 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 a KalSeedPtrCollection, which is how the donor consumes it
    (KalSeedPtrCollection : Reflecte at recoMC/RefRegrow.fcl:52).
  • 🟢 The output tiers match the directory: rec. from reco/, 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

  1. Rename the process to RefSelect in both files (finding 1).
  2. Drop the two nil output lines (finding 2).
  3. Consider an epilog so the two variants stop being a full copy (finding 3).
  4. For the record: which dataset were these run on to get the ~10% figure, and is
    RefRegrow the intended consumer of the output?

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.

3 participants