Skip to content

Add CaloEntrantTruthMaker: calo-entrant truth for calo MC hits - #1911

Open
zwl0331 wants to merge 3 commits into
Mu2e:mainfrom
zwl0331:calo-entrant-truth-main
Open

Add CaloEntrantTruthMaker: calo-entrant truth for calo MC hits#1911
zwl0331 wants to merge 3 commits into
Mu2e:mainfrom
zwl0331:calo-entrant-truth-main

Conversation

@zwl0331

@zwl0331 zwl0331 commented Jul 31, 2026

Copy link
Copy Markdown
Contributor

This adds a producer that assigns each energy deposit in a CaloHitMC to its "calo-entrant" ancestor, i.e. the highest ancestor in the Geant4 parent chain that also deposited energy in the same disk. The point is to group secondary shower products (brem photons, conversion pairs, etc.) with the particle that started the shower, rather than treating every SimParticle as its own truth object. I need this for GNN clustering truth labels, but it is a generic truth definition, so it lives in CaloMC rather than in my analysis code.

Background: I originally tried to do this at the ntuple level by persisting the ancestor chains (EventNtuple#366). That was closed since truth matching like this belongs in an art module. This is that module. It walks SimParticle::parent() live, and only the resulting assignment is stored, no genealogy anywhere.

New/changed files:

  • MCDataProducts/inc/CaloHitEntrant.hh (+ classes.h / classes_def.xml): a persistent data product with a ROOT dictionary, like every other Mu2e product. One entry per CaloHitMC, holding an art::Ptr<CaloHitMC> back-reference to the hit it was computed for plus a vector of Ptr<SimParticle> entrants aligned with energyDeposits(). The collection is index-parallel to the input CaloHitMCCollection, but consumers do not have to trust that or the configuration: the back-reference identifies the exact source collection and entry, and the companion EventNtuple fill verifies it per entry. (The earlier revision of this PR described the product as transient with no dictionary and cited a nonexistent precedent; both corrected after review.)
  • CaloMC/src/CaloEntrantTruthMaker_module.cc: the producer. Walks the parent chain for each (SimParticle, disk), stopping at the chain root, and caches results per (Ptr, disk). An unreadable depositor or ancestor Ptr (dropped collection) throws with a message naming the problem.

Registered in both CMake and SConscript.

Disk resolution: the module uses CaloHitMC::crystalID() when it is filled. In older productions it is not (MDC2025 predates the member, and the CaloHit/CaloHitMC Assns does not survive compression), so there it falls back to the CaloCluster/CaloClusterMC pairing. The cluster tags are optional and default to empty, so consumers wire the module explicitly and no prolog entry is needed; configure the tags only when the fallback is actually needed. The fallback takes only the disk, a cluster-level property, from the pairing (CaloClusterTruthMatch filters and re-sorts the per-cluster MC hit list, so hits are never paired positionally), tolerates truth-unmatched hits, and cross-checks the ProductID of every referenced CaloHitMC against caloHitMCTag, with key bounds and conflicting-disk checks on top. Because a cluster-wide disk is only meaningful for a disk-local cluster, the fallback validates every reco hit of each cluster against the cluster's diskID and aborts on a mixed-disk cluster (possible under cluster association strategy 2) rather than guessing; note this validation runs for all clusters whenever the tags are configured, even where crystalID() would have sufficed. CaloHitMC entries not referenced by any cluster get null entrants in fallback mode, and the module warns once per job if that leaves entrants unresolved with no fallback configured.

Validated on 200 MDC2025 events (run 001430): the assignment agrees with my existing Python implementation (which walks the ancestor chains persisted in files made with the old EventNtuple#366 patch) for all 6,554 contributions. Flattened through EventNtuple, the GNN truth labels built downstream come out byte-identical to my published pipeline (341/341 disk-graphs; a second file, 6,951 contributions, gives the same). The branch also builds cleanly and revalidates with identical output when merged with current main including the #1908 CalorimeterGeom rewrite, the persisted product has been written and read back through RootOutput, and the fallback's failure paths (mixed-disk cluster, out-of-range key, conflicting disk claim) have each been exercised with synthetic inputs.

Companion EventNtuple PR that flattens this into calohitsmc: Mu2e/EventNtuple#392

For each CaloHitMC energy deposit, walk SimParticle::parent() and store
the calo-entrant ancestor (highest ancestor that also deposited in the
same disk) in a new transient CaloHitEntrantCollection, index-parallel
to the input CaloHitMCCollection.

Crystal/disk resolution prefers CaloHitMC::crystalID() when filled;
files produced before that member existed (reads back -1) fall back to
the CaloCluster<->CaloClusterMC pairing. Cardinality or null-Ptr
violations of that pairing throw rather than degrade. Analysis choices
(purity cut, ambiguity, cluster IDs) deliberately stay downstream; the
product records ancestry facts only.

Registered in both build systems (CMakeLists cet_build_plugin +
SConscript; MF_MessageLogger added to the SCons plugin link list for
the mf logging call).

Validated byte-exact against the Python ancestry-walk reference on 200
MDC2025 events under envset p103: 6554/6554 contribution roots
identical (fingerprint-matched across ntuple formats).
@FNALbuild

Copy link
Copy Markdown
Collaborator

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

  • CaloMC
  • MCDataProducts

which require these tests: build.

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

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

About FNALbuild. Code review on Mu2e/Offline.

@FNALbuild

Copy link
Copy Markdown
Collaborator

☀️ The build tests passed at 3c8f625.

Test Result Details
test with Command did not list any other PRs to include
merge Merged 3c8f625 at f96d9fc
build (prof) Log file. Build time: 04 min 59 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.
trigger Log file.
check_cmake Log file.
FIXME, TODO TODO (0) FIXME (0) in 2 files
clang-tidy ➡️ 2 errors 2 warnings
whitespace check no whitespace errors found

N.B. These results were obtained from a build of this Pull Request at 3c8f625 after being merged into the base branch at f96d9fc.

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

1. [S2] The legacy fallback asserts a positional invariant CaloClusterTruthMatch does not provide — resolve the disk from cluster.diskID() instead.

The module's comment claims the cluster↔clusterMC hit lists are "positionally-matched … the invariant CaloClusterTruthMatch establishes." What that producer actually does (CaloMC/src/CaloClusterTruthMatch_module.cc:93-113) is:

  • push one CaloHitMC per cluster hit found in the truth Assns — a cluster hit with no truth entry (e.g. a noise digi) is silently skipped, so the per-cluster sizes can legitimately differ, and this module's hits.size() != hitmcs.size() throw would then kill the job on a soft condition it labels "a wiring error";
  • then std::sorts the list by totalEnergyDep() — so caloHitMCs() is energy-ordered, not positionally matched to caloHitsPtrVector().

The per-index pairing hits[ih] ↔ hitmcs[ih] is therefore not guaranteed. It happens to work here only because every hit in a cluster is on the cluster's single disk — and the disk is the only thing the pairing is used for. Which suggests the fix: in the fallback, map each hitmcs[ih].key() to clusters[ic].diskID() directly (skip the CaloHit side entirely). That is simpler, immune to the ordering issue, tolerant of unmatched hits (drop the size throw or demote it), and produces identical results wherever the current code works. The 200-event validation would not have caught this: with no unmatched hits and single-disk clusters, the current pairing gives the right disk regardless of ordering.

2. [S2] The product has no dictionary — and the cited precedent (CaloHitGraph) does not exist.

MCDataProducts/src/classes_def.xml gains no CaloHitEntrant entry, and there is no CaloHitGraph anywhere in Offline (the description's "same pattern as CaloHitGraph" cites a class that isn't in the repo — worth correcting either way). As far as I can tell this would be the first Mu2e data product without a dictionary. Consequences to be explicit about: the product can never be written to an art file (a RootOutput stream configured to keep it will fail — e.g. any debug job with default keep-everything output), and every consumer (including EventNtuple#392) must run the producer in-job. That may be exactly the intent — but it interacts with the author's own open question about recording the source CaloHitMCCollection: adding the dictionary (classes.h + classes_def.xml, wrapper + the Ptr instantiations) would make the product persistable and make a Ptr<CaloHitMC> per entry the natural provenance answer. Recommend deciding this explicitly in the PR rather than by omission, and fixing the phantom reference in the description.

3. [S2] The cluster collections are hard dependencies even when they are not needed.

produce does getValidHandle on caloClusterTag/caloClusterMCTag unconditionally, so the module cannot run where clustering is absent (e.g. digi-level CaloHitMC straight from CaloHitTruthMatch) even on new files where crystalID() makes the pairing unnecessary. For a product advertised as "a generic truth definition," make the legacy tags optional — empty tag → skip the pairing map (and answer the author's second open question: with optional tags, no prolog entry is needed; consumers wire it explicitly).

4. [S3] Robustness / style

  • simDisks and entrantCache are keyed by bare SimParticle::id().asInt(). That silently collides if the deposits ever reference more than one SimParticle collection (the id is only unique per collection). Standard compressed Production files have a single collection, so this is latent — but keying by art::Ptr<SimParticle> (ProductID + key) costs nothing and removes the assumption. The ntuple-facing id convention can stay at the EventNtuple boundary.
  • Consider verifying hitmcs[ih].id() == <caloHitMC handle>.id() (ProductID equality) in the fallback — fails loudly if caloClusterMCTag was built against a different CaloHitMCCollection than caloHitMCTag (the same config-trust gap the author's open question is about).
  • The parent walk stops on a null parent() but dereferences non-null Ptrs unconditionally; on a file where an ancestor's collection was dropped (dangling Ptr), p->parent() throws a generic art exception mid-walk. Fail-loud is fine, but an isAvailable() check with a message naming the module would diagnose faster.
  • Add the conventional using Parameters = art::EDProducer::Table<Config>; alias (works as-is, but the alias is the Mu2e/art idiom and helps tooling).
  • CaloHitEntrant exposes a public data member; Mu2e data-product convention prefers private + accessor. Minor for a transient struct, but cheap to conform.
  • Merge coordination: Refactoring of the caloriemter base code and bug fixes #1908 rewrites CalorimeterGeom (Crystal::diskID() survives but changes intunsigned, and Calorimeter becomes DiskCalorimeter proditions). No conflict at today's heads; whichever lands second should re-verify compilation.

@FNALbuild

Copy link
Copy Markdown
Collaborator

📝 The HEAD of main has changed to afcbaf1. Tests are now out of date.

@oksuzian
oksuzian requested a review from bechenard July 31, 2026 22:44
@oksuzian

oksuzian commented Aug 1, 2026

Copy link
Copy Markdown
Collaborator

@FNALbuild run build test

@FNALbuild

Copy link
Copy Markdown
Collaborator

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

@FNALbuild

Copy link
Copy Markdown
Collaborator

☔ The build is failing at 3c8f625.

scons: *** [build/al9-prof-e29-p103/Offline/tmp/CosmicReco/src/LineFinder_module.os] Error 1
Test Result Details
test with Command did not list any other PRs to include
merge Merged 3c8f625 at e9b2fbd
build (prof) Log file.
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.
trigger Log file.
check_cmake 〰️ Log file.
FIXME, TODO TODO (0) FIXME (0) in 2 files
clang-tidy ➡️ 2 errors 4 warnings
whitespace check no whitespace errors found

N.B. These results were obtained from a build of this Pull Request at 3c8f625 after being merged into the base branch at e9b2fbd.

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 commented Aug 1, 2026

Copy link
Copy Markdown
Collaborator

@FNALbuild run build test

@FNALbuild

Copy link
Copy Markdown
Collaborator

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

@FNALbuild

Copy link
Copy Markdown
Collaborator

☀️ The build tests passed at 3c8f625.

Test Result Details
test with Command did not list any other PRs to include
merge Merged 3c8f625 at ed58b12
build (prof) Log file. Build time: 04 min 19 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.
trigger Log file.
check_cmake Log file.
FIXME, TODO TODO (0) FIXME (0) in 2 files
clang-tidy ➡️ 2 errors 4 warnings
whitespace check no whitespace errors found

N.B. These results were obtained from a build of this Pull Request at 3c8f625 after being merged into the base branch at ed58b12.

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.

zwl0331 added 2 commits August 2, 2026 11:07
Review feedback on Mu2e#1911:

- Legacy fallback no longer pairs cluster hit lists positionally.
  CaloClusterTruthMatch filters unmatched hits and re-sorts the MC list
  by energy, so only the collection-level zip is contracted. Each
  CaloHitMC referenced by a CaloClusterMC now inherits
  CaloCluster::diskID() directly (the disk is all the pairing was used
  for), tolerating unmatched hits instead of throwing on them. A
  ProductID cross-check rejects a CaloClusterMC built against a
  different CaloHitMCCollection than caloHitMCTag.

- CaloHitEntrant gets a ROOT dictionary (classes.h + classes_def.xml)
  and is persistable like every other Mu2e data product. Each entry
  carries an art::Ptr<CaloHitMC> back-reference, so the product is
  self-describing and consumers can verify source identity instead of
  trusting configuration. Members are private with accessors.

- caloClusterTag/caloClusterMCTag are optional (mayConsume, empty
  default, must be set together): on files with CaloHitMC::crystalID()
  filled the module runs without any cluster products. A once-per-job
  LogWarning flags legacy files with no fallback configured (entrants
  null).

- simDisks/entrantCache keyed by art::Ptr<SimParticle> (ProductID +
  key) instead of bare SimParticle id, removing the single-collection
  assumption; the parent walk throws a clear message on an unreadable
  parent Ptr (dropped ancestor collection); conventional Parameters
  alias added.

Re-validated against the Python ancestry-walk reference on 200 MDC2025
events: 3903/3903 MC hits matched, 6554/6554 contribution roots
identical. Smoke-tested: no-cluster-tags job runs with all-null
entrants plus warning; a keep-everything RootOutput job writes the
product branch (write-side smoke; full read-back done in independent
review).
Independent review of the previous commit found three gaps:

- The cluster-wide disk fallback assumed disk-local clusters, but
  association strategy 2 (ClusterAssociator) can merge proto-clusters
  across disks while CaloCluster keeps only the seed's disk; such a
  cluster would have received silently wrong truth. The fallback now
  verifies every reco hit of a cluster sits on the cluster's disk and
  throws on a mixed-disk cluster (the filtered, energy-sorted MC list
  cannot recover per-hit disks there). Configure the cluster tags only
  when the fallback is actually needed.

- The HMC-key -> disk map was unchecked last-write-wins. Keys are now
  bounds-checked against the collection, and a conflicting second disk
  assignment throws with both cluster indices and both disks.

- The parent-chain walk guarded ancestor Ptrs but dereferenced the
  depositor SimParticle Ptr unguarded; sim.isAvailable() is now checked
  before the first dereference, with the same diagnostic style.

No behavior change on disk-local, uniquely-referenced input: the same
200 MDC2025 events revalidate identically (200/200 events, 3903/3903
MC hits, 6554/6554 contribution roots, PARITY OK; no-tags and
RootOutput smokes re-pass).
@zwl0331

zwl0331 commented Aug 2, 2026

Copy link
Copy Markdown
Contributor Author

Thanks for the careful review. All four points are addressed in the two commits now on the branch, and the description is updated to match.

  1. Positional pairing: adopted your fix. The fallback now maps each CaloHitMC referenced by clusterMCs[ic] to clusters[ic].diskID() directly, so the per-cluster hit lists are never paired positionally, truth-unmatched hits are tolerated instead of throwing, and every referenced Ptr is cross-checked against caloHitMCTag's ProductID. Following the same thread further, the disk-local assumption itself can break: association strategy 2 checks only the time-of-flight relation, so a mixed-disk cluster is valid producer output while the final CaloCluster keeps only the seed's disk. The fallback now validates every reco hit of each cluster against the cluster's diskID and throws on a mixed cluster rather than guessing (the filtered, energy-sorted MC list has no per-hit disk information left to do better). Hardened while there: hmc.key() is bounds-checked against the collection, and a second cluster claiming the same CaloHitMC with a different disk throws with both cluster indices and both disks; same-disk duplicates stay accepted since the map value is unambiguous. I exercised the three failure paths with hand-built inputs: mixed-disk, out-of-range key, and conflicting claim each abort with the intended diagnostic, and a same-disk duplicate control runs clean. Those fixtures are not part of the PR; happy to turn them into proper fail-path tests if you want them in.

  2. Dictionary: added (classes.h + classes_def.xml), so the product is persistable like everything else. The precedent I cited does not exist upstream, it lives in an unpushed branch of mine, sorry about that; the description is fixed. Making it a real product also settled the provenance question the right way: each entry now carries an art::Ptr<CaloHitMC> back-reference to the hit it was computed for, and the EventNtuple fill verifies per entry (ProductID and index) that it is flattening the same collection the producer read. The round trip is verified with a keep-everything RootOutput job plus a separate reader process that fetches by both APIs and dereferences every back-Ptr and entrant Ptr (5 events, 94 entries, 154 entrant Ptrs).

  3. Optional tags: done. caloClusterTag/caloClusterMCTag default to empty (mayConsume), must be set together, and the module runs cluster-free wherever crystalID() is filled, so consumers wire it explicitly and no prolog entry is needed. On a legacy file with no fallback configured it warns once per job and emits null entrants. One deliberate consequence, also in the description: when the tags are configured, every cluster is validated for disk-locality, even where crystalID() would have sufficed, so the tags should only be set when the fallback is actually needed.

  4. The robustness batch: simDisks/entrantCache are keyed by art::Ptr<SimParticle> (ProductID + key), so deposits referencing a second SimParticle collection can never collide; the parent walk guards the depositor Ptr itself as well as every ancestor with isAvailable() and throws naming the module and the dropped collection; the Parameters alias is in; the product members are private with accessors. On the Refactoring of the caloriemter base code and bug fixes #1908 coordination: it merged before this update, so I merged current main locally and did a full build and rerun; no conflicts, the build is clean, and the output is identical, the entrantSimIds branch comes out byte-identical between the pre- and post-Refactoring of the caloriemter base code and bug fixes #1908 geometry.

Revalidated end to end after the changes: 200 MDC2025 events, 3,903 MC hits, 6,554 contributions, all entrant assignments identical to my Python reference; the no-tags job (5 events, 94 hits, 154 contributions, all -1, one warning) and the write/read-back job pass as above.

@oksuzian

oksuzian commented Aug 2, 2026

Copy link
Copy Markdown
Collaborator

@FNALbuild run build test

@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 — re-review of "Add CaloEntrantTruthMaker" (#1911)

Reviewed at head af6d54c1 (2026-08-02). Delta since the previously reviewed 3c8f6257: two commits (d3ba04d8, af6d54c1), 4 files, +185/−78, responding to the four-finding review posted 2026-07-31. Every prior finding was re-verified in the code at the new head, not taken from the reply. EventNtuple#392 companion was updated in lockstep (abd1e279 verifies entrant source identity via the product's new CaloHitMC Ptr).

Decision

  • 🟢 Approve. All four prior findings are fixed and verified; the two new observations are nits. The only gate left is procedural: FNALbuild has not been triggered at af6d54c1 (author reports a clean local build + byte-identical revalidation post-#1908 merge, but the repo's own CI must confirm before merge).

Carry-forward accounting (prior review of 3c8f6257, posted on the PR 2026-07-31)

1. 🟢 [was S2] Positional cluster↔clusterMC pairing — FIXED in d3ba04d8/af6d54c1, verified.
The fallback now zips the two collections only at collection level (one CaloClusterMC per CaloCluster in input order — the invariant CaloClusterTruthMatch actually provides) and maps each referenced CaloHitMC key to clusters[ic].diskID() directly. No per-hit positional pairing remains; truth-unmatched hits are tolerated (the per-cluster size throw is gone; the surviving clusters.size() != clusterMCs.size() throw checks a real invariant). Beyond the prescription, the fallback is hardened fail-closed: ProductID cross-check of every referenced Ptr against caloHitMCTag, key bounds check, conflicting-disk-claim throw naming both clusters, and — a genuinely new catch by the author — per-cluster disk-locality validation, since cluster-association strategy 2 can merge proto-clusters across disks while CaloCluster keeps only the seed's disk. A mixed-disk cluster now throws rather than receiving a guessed disk. Failure paths exercised with synthetic inputs per the reply.

2. 🟢 [was S2] No dictionary + phantom CaloHitGraph precedent — FIXED in d3ba04d8, verified.
classes.h include added; classes_def.xml gains CaloHitEntrant, the collection, and the art::Wrapper. Member-type dictionaries confirmed pre-existing (art::Ptr<mu2e::CaloHitMC> line 193, std::vector<art::Ptr<mu2e::SimParticle>> line 57). The product is now persistable, verified by the author with a write/read-back RootOutput job. The provenance question is settled the right way: each entry carries an art::Ptr<CaloHitMC> back-reference, making the product self-describing. The PR description no longer cites the nonexistent precedent and says so explicitly.

3. 🟢 [was S2] Hard cluster dependencies — FIXED in d3ba04d8, verified.
caloClusterTag/caloClusterMCTag default to empty with mayConsume; a ctor check requires both-or-neither; the module runs cluster-free wherever crystalID() is filled, warns once per job when unresolved entrants remain with no fallback, and needs no prolog entry (consumers wire it explicitly — this also answers the author's original open question). Documented consequence, stated in the PR body: when the tags are set, disk-locality validation runs for all clusters, so set them only when the fallback is needed.

4. 🟢 [was S3] Robustness/style batch — FIXED in d3ba04d8/af6d54c1, verified.
simDisks/entrantCache keyed by art::Ptr<SimParticle> (ProductID+key); ProductID cross-check implemented (see 1); isAvailable() guards on both the depositor and every ancestor, throwing with the module name and the dropped collection; using Parameters = art::EDProducer::Table<Config> alias in; CaloHitEntrant members private with accessors.

No other reviewers' change requests exist on the PR; nothing else to carry.


New findings (delta 3c8f6257..af6d54c1)

1. ⚪ [S3] The new disk-locality loop dereferences reco-hit Ptrs without an isAvailable() guard.

  • Evidence: CaloEntrantTruthMaker_module.cc fallback validation — hit.isNull() is checked but hit->crystalID() would throw a generic art exception on a file where the CaloHitCollection was dropped.
  • Impact: potential risk only — legacy mcs files that carry clusters carry their hits; this is the same fail-loud-but-anonymous pattern the SimParticle walk just got guards for.
  • Suggested fix: mirror the isAvailable() + named-module throw used for the SimParticle Ptrs, for consistency. Optional.

2. ⚪ [S3] Redundant dictionary selection.

  • Evidence: classes_def.xml lists both mu2e::CaloHitEntrantCollection and std::vector<mu2e::CaloHitEntrant> — the same type through the alias.
  • Impact: harmless duplicate selection; neighboring product blocks list only one form.
  • Suggested fix: drop one line. Optional.

Validation check

  • Build/tests: 🟡 FNALbuild has not been triggered at af6d54c1 (mu2e/buildtest: "This test has not been triggered yet"); the last green CI is at the old head. Author's local evidence is strong (full build vs current main incl. #1908, byte-identical entrantSimIds, 200-event revalidation, no-tags job, write/read-back job, synthetic fail-path fixtures) but CI must confirm.
  • Config contract: 🟢 validated Config, optional tags with both-or-neither enforcement, no prolog entry by design.
  • Cross-repo: 🟢 EventNtuple#392 updated in lockstep (abd1e279); merge order Offline first, unchanged.

Residual risk

  • None blocking. The fallback is now strictly fail-closed; the cost is that legacy strategy-2 (cross-disk-cluster) files abort rather than proceed — a deliberate, documented choice.

Author follow-ups

  1. Trigger @FNALbuild run build test at af6d54c1 (required for merge anyway).
  2. Optional ⚪s: isAvailable() guard on reco-hit Ptrs in the validation loop; dedupe the dictionary selection.
  3. The synthetic fail-path fixtures would make good permanent fail-path tests — worth a follow-up PR if cheap to port.

@FNALbuild

Copy link
Copy Markdown
Collaborator

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

@FNALbuild

Copy link
Copy Markdown
Collaborator

☀️ The build tests passed at af6d54c.

Test Result Details
test with Command did not list any other PRs to include
merge Merged af6d54c at f408474
build (prof) Log file. Build time: 04 min 20 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.
trigger Log file.
check_cmake Log file.
FIXME, TODO TODO (0) FIXME (0) in 2 files
clang-tidy ➡️ 2 errors 2 warnings
whitespace check no whitespace errors found

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

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.

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