Skip to content

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

Open
zwl0331 wants to merge 1 commit into
Mu2e:mainfrom
zwl0331:calo-entrant-truth-main
Open

Add CaloEntrantTruthMaker: calo-entrant truth for calo MC hits#1911
zwl0331 wants to merge 1 commit 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 files:

  • MCDataProducts/inc/CaloHitEntrant.hh: transient product (no dictionary, same pattern as CaloHitGraph). One entry per CaloHitMC, holding a vector of Ptr<SimParticle> entrants aligned with energyDeposits(). The collection is index-parallel to the input CaloHitMCCollection.
  • CaloMC/src/CaloEntrantTruthMaker_module.cc: the producer. Walks the parent chain for each (SimParticle, disk), stopping on a null parent (compression), and caches results per (simId, disk).

Registered in both CMake and SConscript.

To get the disk of each hit 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 index-parallel pairing; CaloHitMC entries not referenced by any cluster get null entrants in that mode. If collection sizes do not line up the module throws rather than truncating.

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).

Two things I would like input on:

  • Should the product record which CaloHitMCCollection it was made from (a ProductID, or a Ptr<CaloHitMC> per entry)? Right now consumers have to trust the configuration.
  • Is a prolog FCL entry wanted, or should consumers wire the module explicitly?

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
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