Skip to content

Add Trigger MDS fcl#532

Merged
oksuzian merged 2 commits intoMu2e:mainfrom
michaelmackenzie:MDSMenu
Apr 4, 2026
Merged

Add Trigger MDS fcl#532
oksuzian merged 2 commits intoMu2e:mainfrom
michaelmackenzie:MDSMenu

Conversation

@michaelmackenzie
Copy link
Copy Markdown
Contributor

This runs the Trigger MDS menu with example prescales for Trigger MDS studies

@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 c6661ea: build (Build queue - API unavailable)

About FNALbuild. Code review on Mu2e/Offline.

@FNALbuild
Copy link
Copy Markdown
Collaborator

☀️ The build tests passed at c6661ea.

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

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

oksuzian commented Apr 3, 2026

I'll review this pull request by examining its details and changes.Here is my review of PR #532 — "Add Trigger MDS fcl":

Summary

This PR adds 3 new FCL files for Trigger Mock Data Sample (MDS) digitization and updates the prolog to include the new MDS menu configurations. It adds 30 lines with no deletions across 4 files.


Issues Found

1. 🐛 Incorrect comment in TriggerMDSOffSpill.fcl

#include "Production/JobConfig/digitize/TriggerMDS.fcl"

# set the spill type to On-Spill
physics.producers.EWMProducer.SpillType : 0

The comment says "set the spill type to On-Spill" but SpillType : 0 corresponds to Off-Spill. Comparing with existing conventions in the repo:

  • OnSpill.fcl uses SpillType : 1
  • OffSpill.fcl uses SpillType : 0
  • NoField.fcl also uses SpillType : 0 (off-spill)

The comment should read # set the spill type to Off-Spill.

2. ⚠️ Redundant epilog includes in TriggerMDS.fcl

#include "Production/JobConfig/digitize/Digitize.fcl"
...
#include "Production/JobConfig/common/epilog.fcl"
#include "mu2e-trig-config/core/trigDigiInputsEpilog.fcl"

The base file Digitize.fcl already includes both Production/JobConfig/common/epilog.fcl (line 32) and mu2e-trig-config/core/trigDigiInputsEpilog.fcl (line 34). Re-including them in TriggerMDS.fcl is redundant. While FHiCL may tolerate duplicate includes, this differs from the pattern used by other similar files (e.g., OnSpill.fcl which inherits from Spill.fclDigitize.fcl and does not re-include these epilogs). This could lead to unexpected parameter override ordering.

3. 🔍 Inconsistency: TriggerableBits referenced but set to empty

physics.TriggerablePaths : [ ]
physics.TriggerableBits  : [ ]

# Add the trigger paths
physics.trigger_paths : ["DigitizePath", @sequence::physics.TriggerableBits, @sequence::Trig_mdsMenu.trigger_paths]

physics.TriggerableBits is explicitly set to an empty list [ ], then immediately used via @sequence::physics.TriggerableBits in trigger_paths. While this is intentional (the comment says "No MC triggerable paths"), it introduces a no-op expansion. Other files like OnSpill.fcl and OffSpill.fcl populate these lists with actual triggerable paths before referencing them. This is minor but worth confirming that completely excluding MC triggerable paths is the intended design.

4. 📝 Missing trailing newline in TriggerMDSOffSpill.fcl

The file TriggerMDSOffSpill.fcl appears to have a trailing blank line after SpillType : 0, while TriggerMDSOnSpill.fcl has the same structure. This is cosmetic but both files are consistent with each other, so this is fine.


What looks good

  • The prolog.fcl change correctly adds the two new MDS menu includes (trig_mdsMenuPSConfig.fcl and trig_mdsMenu.fcl) following the same pattern as existing entries.
  • The TriggerMDSOnSpill.fcl file correctly uses SpillType : 1 with the matching "On-Spill" comment.
  • The overall structure of the new files follows the established patterns in the repository.

Verdict

Issue #1 (wrong comment in OffSpill) is a clear copy-paste bug that should be fixed before merging. The other items are worth a quick discussion but are not blockers.

@michaelmackenzie
Copy link
Copy Markdown
Contributor Author

I addressed comments 1, 2, and 4. The Triggerable lists are intentionally included in case we want to add an MC selection later.

@oksuzian
Copy link
Copy Markdown
Collaborator

oksuzian commented Apr 4, 2026

@FNALbuild run build test

@FNALbuild
Copy link
Copy Markdown
Collaborator

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

@FNALbuild
Copy link
Copy Markdown
Collaborator

☀️ The build tests passed at a631980.

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

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 merged commit 5d74c0e into Mu2e:main Apr 4, 2026
14 checks passed
@michaelmackenzie michaelmackenzie deleted the MDSMenu branch April 4, 2026 15:01
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.

4 participants