Update TrkPID model and producer

[michaelmackenzie]

This follows the updates made in Mu2e/MLTrain#9. I renamed the previous TrkPID model to v0 so the older model can still be accessed. I also updated the module to handle multiple parallel track collections to reduce the need for multiple module instances in the art jobs.

[oksuzian]

PR Review Summary — "Update TrkPID model and producer" (#8)

Reviewed at head 43f4d1f8 (2026-08-04). First review of this PR. Scope: deploy the MLTrain#9 retraining — new TrackPID v1 (E/P, Δt, p(χ²), dt/dt slope) and new calo-free TrackOnlyPID producer, both multi-collection; the old model renamed to v0; regenerated SOFIE headers and weights. Companion to the just-approved Mu2e/MLTrain#9, and the SOFIE headers here were regenerated today after the MLTrain float32/batch-1 fixes (v1 and TrkOnlyPID headers are batch-1, generated 2026-08-04) — the right sequencing.

Decision

• 🔴 Request changes. One S0 (merging this breaks the standard EventNtuple ntupling path until a coordinated EventNtuple PR lands) and three S1s (the old model's weights were overwritten in place, so v0 silently loads the wrong weights; the new module is absent from the CMake build; the deployed v1 slope feature is computed with a different estimator than the one the model was trained on, with no validation evidence in the PR).

Scope understood

• TrackPID_module reworked: validated multi-collection config (KalSeeds sequence + optional parallel KalSeedDtDts), MVAVersion selects v0 (calo-geometry features) or v1 (track-based features from MLTrain#9), one MVAResultCollection output instance per input collection.
• New TrackOnlyPID_module: same shape, tracker-only v0 model.
• Generated artifacts: TrackPID_v0.hxx (rename of old header, namespace/guard only), TrackPID_v1.hxx + TrkOnlyPID_v0.hxx (fresh batch-1 exports), TrackPID_v1.dat (overwritten), TrkOnlyPID_v0.dat (new).

Findings

1. 🔴 [S0] Merging this breaks EventNtuple's default ntupling path — a coordinated EventNtuple PR is required and none is referenced.

   • Evidence: EventNtuple/fcl/prolog.fcl runs TrkPIDAll (module_type: TrackPID) in its default Path and PathNoMC (lines 432/436), configured with DeltaTOffset, datFilename, and KalSeedPtrCollection (lines 54-68). The new validated Config removes DeltaTOffset, renames datFilename → DatFilename, replaces KalSeedPtrCollection with the required KalSeeds sequence, and adds required MVAVersion — so fhicl validation throws at module construction in every standard EventNtuple job at the next Analysis-musing build.
   • Three distinct breaks to coordinate: (a) the config schema above; (b) product identity — the module now produces with an instance name per input collection (e.g. TrkPIDAll:MergeKKAll), so EventNtuple's trkPIDTags: ["TrkPIDDeM"] (empty instance) no longer resolves even after the schema fix; (c) MVAVersion 1 requires KalSeedDtDt collections, and EventNtuple's path configures no TrackDtDt producers (the Offline producer exists — ParticleID/src/TrackDtDt_module.cc, merged in Offline#1873 — it just isn't wired anywhere).
   • Suggested fix: a companion EventNtuple PR (new schema, label:instance trkPIDTags, TrackDtDt producers in the path), merged in lockstep; name it in this PR's body. Until then this PR is not mergeable without breaking downstream.

2. 🟠 [S1] The v0 model's weights were overwritten in place — "the older model can still be accessed" is not actually true, and the failure is silent.

   • Evidence: on main, data/TrackPID_v1.dat held the weights of the then-only (now-v0) graph. This PR overwrites its values with the new v1 training's weights and adds no TrackPID_v0.dat. Because both graphs are the same 4→5→10→5→1 architecture and the keras export produced identical tensor names and sizes (verified: the .dat diff changes only values, and TrackPID_v0.hxx/TrackPID_v1.hxx sessions read the same tensor names), a v0 Session pointed at the new file loads without any error and evaluates v1 weights on v0 features (E−P, R(cluster), p·x, Δt) — silently wrong scores.
   • Suggested fix: restore the pre-PR file content as data/TrackPID_v0.dat so MVAVersion: 0 + DatFilename: .../TrackPID_v0.dat reproduces the old model, and note the pairing rule (version ↔ .dat) in the Config comment.

3. 🟠 [S1] TrackOnlyPID is missing from TrkDiag/CMakeLists.txt — the new module does not exist in CMake/spack builds.

   • Evidence: the scons SConscript builds plugins by globbing *_module.cc, so muse builds pick it up automatically; the CMakeLists builds plugins via explicit cet_build_plugin entries and has none for TrackOnlyPID. No configure or link error results — the module is just silently absent, failing only at runtime ("module type TrackOnlyPID not found"). Same scons↔CMake mirroring class as recent Offline findings; ArtAnalysis has no CI on either build system, so nothing catches it.
   • Suggested fix: add cet_build_plugin(TrackOnlyPID art::module REG_SOURCE src/TrackOnlyPID_module.cc LIBRARIES REG ArtAnalysis::TrkDiag Offline::ConfigTools Offline::RecoDataProducts), modeled on the TrackPID entry (whose SOFIE usage compiles with the same dependency set — TrackQuality is the in-repo precedent).

4. 🟠 [S1] The deployed v1 slope feature is computed with a different estimator than the training feature, and the PR carries no validation evidence.

   • Evidence: the model was trained on the slope from MLTrain's make_inputs.py fit — unweighted (flat 5 ns, absolute_sigma) least squares of ptoca + udt vs ptoca over all track hits. The deployed feature is KalSeedDtDt::slope() from Offline's TrackDtDt, which fits the same axes over the same hits but with inverse-variance weights (1/fitTocaVar per hit, TrackDtDt_module.cc:127-129). A differently-weighted estimator has a systematically different distribution; the MVA then runs on a feature it was not trained on, and any performance loss is silent. Secondary, smaller: training E/P used the momentum at the sid == 1 (tracker-middle) segment, while inference uses momentumAtCalo — a sub-percent shift, likely negligible against calo energy resolution, but worth stating.
   • Suggested fix: overlay TrackDtDt slope vs the training branch for the same tracks and show the model's performance is unchanged (or retrain on the producer's output — the more robust long-term choice). Also: ArtAnalysis has no CI, so please include build evidence (muse/scons at minimum, CMake if possible) and a smoke job in the PR body per the standard validation packet.

5. 🟡 [S2] The repo's own prolog.fcl TrkPID block is stale — pre-existing, but this PR widens the gap and the config surface is its subject.

   • Evidence: TrkDiag/fcl/prolog.fcl still configures MVAConfig: { MVAWeights: ...TrkCaloHitPID.weights.xml } — invalid against the module Config since before this PR, and further from valid now (TrkPIDDeM/TrkPIDDeP aliases equally broken). The only working example of the new schema is nowhere in the repo.
   • Suggested fix: update the block to a working v1 example (KalSeeds, KalSeedDtDts, DatFilename, MVAVersion) — it doubles as the migration reference for the EventNtuple companion PR.

6. 🟡 [S2] Label-only input resolution and the output-instance convention are undocumented contract.

   • Evidence: event.getByLabel(name, handle) treats each KalSeeds entry as a bare module label (no label:instance parsing), and the same string becomes the output instance name. Works for the intended MergeKK* labels; a user passing "label:instance" gets a silent lookup failure phrased as "no collection with label".
   • Suggested fix: say so in the KalSeeds Config comment ("bare module labels; also used as the output instance name"), or parse via art::InputTag and reject tags with instances explicitly.

7. ⚪ [S3] Housekeeping batch (none gate):

   • The renamed TrackPID_v0.hxx keeps the old batch-32 session while evaluateMVA_v0 passes a 4-float array to infer — a 128-float read from a 4-element array. mvaout[0] is still computed from the real features (row 0 of the GEMM), so results are correct in practice, but it is UB on paper. Pre-existing, preserved by the rename; regenerate v0 at batch 1 when restoring the v0 weights (finding 2).
   • Include hygiene (both modules): MVATools.hh, ProditionsHandle.hh, TFileService.h, make_tool.h, <float.h>, <functional> are unused; std::array is used without <array> (arrives transitively).
   • Dead code (both modules): initializeMVA is declared but never defined or called; printMVA_ is write-only since the SOFIE migration.
   • MVAVersion is range-checked only from above (> 1 / > 0); a negative value passes the constructor and throws per-track in the switch — validate != {0,1} (or != 0) at construction.
   • nActive via nHits(true) counts StrawHitFlag::active while training's nactive counted strawHitState() > inactive — near-equivalent, but counting state-based inside the existing nNull loop would make training parity exact and save the second pass. In that loop, the first clause of state > inactive && state == null is redundant (null = 0 > inactive = -2).
   • nActive == 0 gives NaN/inf features (no crash); a guard returning the invalid score would be tidier.
   • TrackOnlyPID's kalSeedDtDts is OptionalSequence but de-facto required (the only version throws without it).

Verified 🟢 (checked, no action needed)

• 🟢 Cross-repo dependency satisfied: KalSeedDtDt product, TrackDtDt producer, and dictionary entries (classes_def.xml) are all in Offline main (merged via #1873); slope() accessor matches the module's use.
• 🟢 The v1/TrkOnlyPID SOFIE headers are batch-1, float32 exports generated 2026-08-04 — after the MLTrain#9 mixed_float16/batch fixes, so the committed artifacts embody the corrected training.
• 🟢 Feature parity verified where checkable: dt is exact (EventNtuple fills trkcalohit.dt = tch._udt; the module uses tchs._udt); fitcon = fitConsistency(); nhits = nHits(false) = hits().size() and the null-ambiguity count match EventNtuple's TrkInfo definitions; the MaxDE gate sign is consistent between v0 and v1 paths.
• 🟢 Offline API usage checked against main: TrkCaloHitSeed::_udt/_rptoca, KalSeed::nHits(bool), nearestSegment(double), TrkStrawHitSeed::strawHitState(), WireHitState ordering — all exist with the assumed semantics.
• 🟢 The kalmanOK fit gate and the calo-cluster/momentum guards are preserved from the old module; v1 adds a mom == 0 guard the v0 path lacked. The per-collection KalSeeds↔KalSeedDtDts size cross-check throws with a clear message.
• 🟢 Dual KalSeed/KalSeedPtr input handling is correct, and output alignment by index matches the "aligned by index" contract in the header comment.
• 🟢 scons build: make_plugins globs the new module and the mainlib already links openblas/ROOTTMVASofie for the SOFIE calls.
• 🟢 Production and mu2e-trig-config: no references to this module's configuration — the only downstream consumer is EventNtuple (finding 1).

Validation check

• Build/tests run: none — ArtAnalysis has no CI on either build system, and this review did not compile the code. Static verification against Offline/EventNtuple main via the GitHub API.
• Config contract check: fail — findings 1 and 5.
• Cross-repo consistency: fail pending the EventNtuple companion PR (finding 1); Offline side is satisfied.

Residual risk

• The v1 model's real-world performance depends on finding 4; everything else is deterministic breakage with deterministic fixes.

Author follow-ups

1. Prepare and reference the EventNtuple companion PR (schema, label:instance tags, TrackDtDt wiring); merge in lockstep (finding 1).
2. Restore the old weights as data/TrackPID_v0.dat (finding 2).
3. Add the TrackOnlyPID cet_build_plugin entry (finding 3).
4. Show the slope-feature parity check (or retrain on TrackDtDt output) and include build evidence in the PR body (finding 4).
5. Update the repo's own prolog.fcl to a working v1 example (finding 5).