Skip to content

Update calo signal primaries with calo dts filter change#539

Open
michaelmackenzie wants to merge 2 commits into
Mu2e:mainfrom
michaelmackenzie:CaloPrimaries
Open

Update calo signal primaries with calo dts filter change#539
michaelmackenzie wants to merge 2 commits into
Mu2e:mainfrom
michaelmackenzie:CaloPrimaries

Conversation

@michaelmackenzie
Copy link
Copy Markdown
Contributor

This follows the changes in Offline PR Mu2e/Offline#1819. Primaries with calo signals can now be selected with total calo energy deposits, and the calo momentum in threshold on sims is lowered. This should reduce loss of viable signals that shower in the calo. The triggerable threshold is also reduced to 68 MeV for calo signals, reflecting the lowered energy reachable by the NN-based calo triggers for electrons and photons.

@FNALbuild
Copy link
Copy Markdown
Collaborator

Hi @michaelmackenzie,
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 ea19f16: build (Build queue - API unavailable)

About FNALbuild. Code review on Mu2e/Offline.

@FNALbuild
Copy link
Copy Markdown
Collaborator

☀️ The build tests passed at ea19f16.

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

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
Copy link
Copy Markdown
Collaborator

PR #539 Review — "Update calo signal primaries with calo dts filter change"

📎 Mu2e/Production#539 · author: @michaelmackenzie · base main ← head michaelmackenzie:CaloPrimaries

Summary

This PR adapts Production's primary-filter config to match the new calo-filtering knobs introduced in Mu2e/Offline#1819. It:

  • Adds new MinimumCaloPartMom / MaximumCaloPartMom / MinimumSumCaloE defaults to the shared Primary prolog.
  • Tunes per-channel overrides for MuCap1809keVCalo, RMCExternal, and FlatGammaCalo to reflect the new filter semantics.
  • Lowers TriggerableCalo.MinParticleEnergy 75 → 68 MeV (digitize prolog), matching the reach of the NN-based e/γ calo trigger.
Metric Value
Files changed 5 (+12 / −6)
Commits 2
Reviews 0
Inline comments 0
CI (FNALbuild) ✅ all green (build, ceSimReco, g4test_03MT, POT, cosmic*, ceDigi/Mix, clang-tidy, …)
Mergeable state blocked (awaiting approving review — mergeable: true)
Risk Low — configuration-only; physics-tuning knobs only

Core changes

1. New filter defaults in JobConfig/primary/prolog.fcl

      MinimumCrvSteps : 1
      MinimumPartMom : 50.0 # MeV/c
      MaximumPartMom : 1.0e6 # MeV/c
      MinimumCaloPartMom : 0.0 # MeV
      MaximumCaloPartMom : 1.0e6 # MeV
      KeepPDG : [ ] # Loop at steps from all particle types
      MinimumTrkSteps : 12 # primary must produce at least this many TrkSteps
      MinimumSumCaloStepE : 45.0 # or at least this much calo energy by a single sim particle
      MinimumSumCaloE : 45.0 # or at least this much calo total energy by accepted sim particles

Good additions, and the clarifying comment on MinimumSumCaloStepE ("by a single sim particle") vs MinimumSumCaloE ("total energy by accepted sim particles") is exactly what readers needed.

2. Trigger threshold for calo signals

    TriggerableCalo : {
      module_type : CaloShowerSimFilter
-     MinParticleEnergy : 75.0
+     MinParticleEnergy : 68.0

Consistent with the PR description — the NN-based e/γ trigger now reaches below 75 MeV.

3. Per-channel overrides

  • MuCap1809keVCalo.fcl — for the 1.809 MeV nuclear capture γ:
    physics.filters.PrimaryFilter.MinimumCaloPartMom  : 0.
    physics.filters.PrimaryFilter.MinimumSumCaloStepE : 0.1
    physics.filters.PrimaryFilter.MinimumSumCaloE     : 0.1
    
  • RMCExternal.fcl — keeps the track 80 MeV cut and adds matching 80 MeV cuts on both calo-energy variants.
  • FlatGammaCalo.fcl — drops the prior loose overrides (MinimumPartMom: 1, MinimumSumCaloStepE: 50) and now inherits the new defaults.

Issues / things to double‑check

  1. MuCap1809keVCalo.fclMinimumCaloPartMom : 0. is redundant. The new prolog default is already 0.0, so this line has no effect. Either drop it, or keep it deliberately as documentation — if kept, add a # explicit comment for future readers.

    physics.filters.PrimaryFilter.MinimumCaloPartMom  : 0.   # redundant with default
    physics.filters.PrimaryFilter.MinimumSumCaloStepE : 0.1
    physics.filters.PrimaryFilter.MinimumSumCaloE     : 0.1
    
  2. MuCap1809keVCalo.fclMinimumPartMom is still 50 MeV (inherited). A 1.8 MeV γ cannot pass that. This only works if the underlying Offline PrimaryFilter treats the track-path (MinimumPartMom + MinimumTrkSteps) and the calo-path (MinimumSumCaloStepE / MinimumSumCaloE) as a logical OR (the inline comment "or at least this much calo energy" implies so). Worth a quick confirmation against the merged Offline#1819 logic — if it's AND, no MuCap event will pass.

  3. FlatGammaCalo.fcl — boundary behaviour at 50 MeV. With startMom: 50 and the new inherited default MinimumPartMom: 50.0, the lower edge of the spectrum may be cut depending on whether the filter is > or >=. If the previous explicit MinimumPartMom: 1 was intentionally avoiding this, consider either bumping startMom slightly above 50, or restoring a small explicit override. Otherwise you may silently lose part of the lowest bin.

  4. RMCExternal.fclMinimumCaloPartMom not raised. Track path requires 80 MeV but calo path accepts any per-particle momentum (default 0) as long as the summed calo E ≥ 80 MeV. That's likely intentional (you want soft sims that aggregate to ≥80 MeV in the calo), but worth a sentence in the PR body so reviewers don't wonder.

  5. Formatting nit in RMCExternal.fcl. Alignment of : is good, but you mix MinimumPartMom (track) and MinimumSumCaloStepE / MinimumSumCaloE (calo) in one block — adding a one-line # track path / # calo path comment would make it obvious at a glance, especially since the same file now has overlapping 80‑MeV thresholds.

  6. Other calo trigger thresholds. You lowered TriggerableCalo.MinParticleEnergy to 68 MeV but I didn't see a corresponding change to any sibling filter (e.g. a TriggerableCaloSignal or analogous). If other filters in JobConfig/digitize/prolog.fcl exist with a 75 MeV threshold for the same physics reason, they should track this change. Worth grepping for MinParticleEnergy / 75.0 in the digitize prolog to confirm nothing was missed.

  7. Coverage of new knobs. MaximumCaloPartMom : 1.0e6 is added but no channel overrides it. That's fine for now, but it's a new public knob — consider documenting its intended use in a one-line comment near the parameter (e.g. "useful to veto e+/e- pairs above…").


Merge readiness

  • ✅ CI fully green (build + 14 integration logs + clang-tidy clean)
  • ✅ Mergeable (mergeable: true); blocked state is purely "needs approving review"
  • ⚠️ Hard-blocking item: confirm the Offline filter OR semantics relied on by item (2) above — that's the only thing that could turn a "low-risk config tune" into a "channel produces zero events" surprise.
  • No inline review comments, no requested reviewers — pinging a Production maintainer (or a calo-trigger expert who reviewed Offline#1819) would unblock it.

Want me to

  1. Pull the diff of Mu2e/Offline#1819 and verify the track-path OR calo-path semantics so we can resolve issue (2) with certainty.
  2. Grep JobConfig/digitize/ for other 75-MeV calo thresholds to check the trigger-threshold change is consistent (issue 6).
  3. Draft a short review comment you can post on the PR enumerating issues (1)–(3) as actionable change requests.
  4. Compare this PR against the previous calo-primary config (the parent commit on main) to check no per-channel file was missed.

Just reply with the number(s).

@michaelmackenzie
Copy link
Copy Markdown
Contributor Author

Responding to the AI comments:

  1. It's redundant for now, but it's crucial later changes don't increase it for this signal since it's so low.
  2. It's not a tracker signal so it's not important there.
  3. The tracks below 50 MeV weren't important, it was originally lowered for the sake of the calo clusters.
  4. This is fine.
  5. I think the fcl is clear, I don't think we really need the comments.
  6. What is TriggerableCaloSignal?
  7. I think the variable name makes the meaning clear without a comment.

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