HEXPLIT: adjust timing cut to respect t0

[veprbl]

Briefly, what does this PR introduce? Please link to any relevant presentations or discussions.

150 ns excess cut is relatively large for big chunk hadronic interactions, but we need to support case when t0 != 0, or this removes hits completely.

Related to #2081
Closes: #2802

What is the urgency of this PR?

• High (please describe reason below)
  For meson structure group
• Medium
• Low

What kind of change does this PR introduce?

• Bug fix (issue #__)
• New feature (issue #__)
• Optimization (issue #__)
• Updated parameters, constants (issue #__)
• Updated documentation
• other: __

Please check if any of the following apply

• This PR requires changes to geometry (epic PR: __)
• This PR requires changes to EDM4eic (EDM PR: __)
• This PR introduces breaking changes. Please describe changes users need to make below.
  Old plugin:factory:tmax name is replaced by plugin:factory:max_time_to_truth_t0
• This PR changes default behavior. Please describe changes below.
• AI was used in preparing this PR. Please describe usage below.
  Claude

[Copilot]

Pull request overview

This bug-fix updates HEXPLIT timing cuts to account for nonzero truth-particle production times.

Changes:

• Adds optional MCParticles input and applies timing relative to truth t0.
• Updates HEXPLIT configuration and ZDC/FHCAL wiring.
• Adds timing-focused tests.

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 6 comments.

Show a summary per file

File	Reviewed changes and final comments
src/tests/algorithms_test/calorimetry_HEXPLIT.cc	Adds truth fixtures and timing tests. Critical (1 vote): MutableMCParticle.h is unavailable; use MCParticle.h or the collection header.
src/factories/calorimetry/HEXPLIT_factory.h	Wires optional truth input and the renamed timing parameter. Moderate (3 votes): Renaming tmax breaks existing configurations. Moderate (2 votes): The added input breaks existing one-tag InputTags overrides.
src/detectors/ZDC/ZDC.cc	Enables truth timing input and updated HEXPLIT configuration.
src/detectors/FHCAL/FHCAL.cc	Enables truth timing input and updated HEXPLIT configuration.
src/algorithms/calorimetry/HEXPLITConfig.h	Renames the timing configuration field.
src/algorithms/calorimetry/HEXPLIT.h	Extends the algorithm input interface and declares t0 helpers.
src/algorithms/calorimetry/HEXPLIT.cc	Computes truth t0 and applies relative timing cuts. Critical (1 vote): Missing truth t0 removes the legacy absolute tmax rejection. Moderate (2 votes): Generator-level-only events lack a t0. Moderate (2 votes): Merged timeframes may use the wrong event’s primary time.

Suppressed comments (4)

src/algorithms/calorimetry/HEXPLIT.h:33

• The algorithm input type now has two elements, but the constructor still advertises only {"inputHits"}. This leaves the new MC-particle input unnamed in the algorithm metadata (and can make input validation/introspection inconsistent); add a corresponding name such as mcParticles to the constructor's input-name list.

    algorithms::Algorithm<algorithms::Input<const edm4eic::CalorimeterHitCollection,
                                            std::optional<edm4hep::MCParticleCollection>>,

src/algorithms/calorimetry/HEXPLITConfig.h:12

• Renaming the tmax field changes the exposed JANA parameter key from <factory>:tmax to <factory>:max_time_to_truth_t0. Existing user configurations and command lines that set tmax will no longer configure this cut, so this is a breaking API change; retain a compatibility alias/deprecation path or document the required migration and mark the breaking change.

  double max_time_to_truth_t0{325 * dd4hep::ns};

src/detectors/FHCAL/FHCAL.cc:89

• JOmniFactory::PreInit requires the InputTags vector to contain exactly one name per registered input. Adding MCParticles here therefore makes existing HcalEndcapPInsertSubcellHits:InputTags overrides that contain only the old rec-hit tag fail during initialization, even though the new input is optional. Please preserve a legacy one-tag form or document this input-tag change as breaking.

      "HcalEndcapPInsertSubcellHits", {"HcalEndcapPInsertRecHits", "MCParticles"},
      {"HcalEndcapPInsertSubcellHits"},

src/detectors/ZDC/ZDC.cc:179

• JOmniFactory::PreInit requires the InputTags vector to contain exactly one name per registered input. Adding MCParticles here therefore makes existing HcalFarForwardZDCSubcellHits:InputTags overrides that contain only the old rec-hit tag fail during initialization, even though the new input is optional. Please preserve a legacy one-tag form or document this input-tag change as breaking.

      "HcalFarForwardZDCSubcellHits", {"HcalFarForwardZDCRecHits", "MCParticles"},
      {"HcalFarForwardZDCSubcellHits"},

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[Copilot]

Pull request overview

Copilot reviewed 7 out of 7 changed files in this pull request and generated 3 comments.

Suppressed comments (3)

src/algorithms/calorimetry/HEXPLIT.cc:122

• The added test only exercises the mchitlinks == nullptr fallback and never calls get_t0. The central new behavior—subtracting a non-zero primary time and applying max_time_to_truth_t0 when links are present—is therefore untested. Add a valid raw-hit/sim-hit/link/MCParticle setup with a non-zero t0 and assert both an in-window and an out-of-window hit.

    const auto t0 = nav.has_value() ? get_t0(hit, *nav) : std::nullopt;
    if (hit.getEnergy() < Emin || (t0.has_value() && (hit.getTime() - *t0) > max_dt)) {

src/algorithms/calorimetry/HEXPLIT.h:34

• The Input tuple now contains both the reconstructed hits and an optional link collection, but the algorithm descriptor still supplies only {"inputHits"} to its constructor (line 41). This leaves the second input unnamed in the generic algorithm metadata/introspection; other multi-input algorithms enumerate every input (for example, CalorimeterTruthClustering.h:31). Add an inputHitLinks name for the new input.

using HEXPLITAlgorithm = algorithms::Algorithm<
    algorithms::Input<const edm4eic::CalorimeterHitCollection,
                      std::optional<edm4eic::MCRecoCalorimeterHitLinkCollection>>,
    algorithms::Output<edm4eic::CalorimeterHitCollection>>;

src/factories/calorimetry/HEXPLIT_factory.h:28

• This renames the externally configurable JANA parameter from tmax to max_time_to_truth_t0. Existing command lines/configuration files that set ...:tmax will no longer affect HEXPLIT, so this is a breaking configuration change despite the PR description saying that no breaking changes are introduced. Please preserve a deprecated tmax alias (with a defined precedence) or document this as a breaking change.

  ParameterRef<double> m_max_time_to_truth_t0{this, "max_time_to_truth_t0",
                                              config().max_time_to_truth_t0};

[Copilot]

Pull request overview

Copilot reviewed 7 out of 7 changed files in this pull request and generated 3 comments.

Suppressed comments (1)

src/algorithms/calorimetry/HEXPLIT.cc:267

• A raw hit can link to multiple simulation hits, and each simulation hit can contain multiple contributions. Breaking after the first contribution/link makes the selected primary time depend on link order rather than the time/energy policy used to form the reconstructed hit; a background link appearing first can therefore apply the wrong t0 and incorrectly reject or retain the hit. Select the contribution/simulation hit corresponding to the reconstructed time, or define a deterministic aggregation, and cover the multi-link case with a regression test.

    for (const auto& contrib : simhit.getContributions()) {
      t0 = get_primary(contrib).getTime();
      break;
    }
    break;

[DraTeots]

But... what happens if this timeout is fully removed? Are there analysis plots showing how bad everything is because of this?

Because again, in spirit of background meeting, if events might appear close to each other in time this cut will do the mess. Or it is seems so.

[veprbl]

But... what happens if this timeout is fully removed? Are there analysis plots showing how bad everything is because of this?

Not much in ZDC, but there appears to be a slight energy cut in forward insert. I don't know why timescales are different.

edit: My testing was specifically on neutron benchmarks.

[Copilot]

Pull request overview

Copilot reviewed 7 out of 7 changed files in this pull request and generated no new comments.

Suppressed comments (3)

src/algorithms/calorimetry/HEXPLIT.cc:270

• CalorimeterHitDigi can merge several SimCalorimeterHits into one raw hit and chooses the reconstructed timestamp from the most energetic linked sim hit (using its earliest contribution), but this loop always takes the first link and first contribution. For pileup/background cells, that t0 can belong to a different contributor than hit.getTime(), so a valid signal hit may be rejected—or a late hit retained—based on link order. Select the sim hit/contribution that determines the reconstructed timestamp, or explicitly combine all associated contributors according to the intended acceptance policy instead of stopping at the first one.

  for (const auto& [simhit, weight] : nav.getLinked(hit.getRawHit())) {
    for (const auto& contrib : simhit.getContributions()) {
      t0 = get_primary(contrib).getTime();
      break;
    }

src/algorithms/calorimetry/HEXPLIT.h:35

• The Input alias now contains two collections, but the algorithm descriptor below still advertises only {"inputHits"}. This leaves HEXPLIT's declared interface inconsistent with its input tuple and makes the optional link input absent from metadata/introspection; the other multi-input algorithms in this tree provide one name per input. Add a name for the link collection.

                      std::optional<edm4eic::MCRecoCalorimeterHitLinkCollection>>,

src/tests/algorithms_test/calorimetry_HEXPLIT.cc:107

• This contribution is a standalone mutable PODIO object and is never added to an edm4hep::CaloHitContributionCollection. addToContributions stores the object's (collectionID, index) relation, so simhit.getContributions() cannot resolve this object (or resolves an invalid ID); the timing tests can therefore pass without exercising get_t0(). Create the contribution in an owned contribution collection (kept alive in TruthChain) before adding it to the sim hit.

  edm4hep::MutableCaloHitContribution contrib;
  contrib.setParticle(secondary);
  contrib.setTime(hit_time_ns);
  contrib.setEnergy(energy);
  simhit.addToContributions(contrib);

[github-actions]

Capybara summary for PR 2892

• rec_dis_10x100_minQ2=0_craterlake_10x100_MT (Δ: 4495)
• rec_dis_10x100_minQ2=100_craterlake_tracking_only (Δ: 0)
• rec_dis_10x130_minQ2=1_craterlake_10x130 (Δ: 0)
• rec_dis_10x250_minQ2=1_craterlake_10x250 (Δ: 4314)
• rec_dis_18x275_minQ2=0_craterlake_18x275 (Δ: 0)
• rec_dis_18x275_minQ2=0_craterlake_18x275_MT (Δ: 361)
• rec_dis_18x275_minQ2=0_craterlake_18x275_ST_MT (Δ: 330)
• rec_dis_18x275_minQ2=100_craterlake_18x275 (Δ: 0)
• rec_dis_18x275_minQ2=100_craterlake_18x275_MT (Δ: 0)
• rec_dis_18x275_minQ2=100_craterlake_18x275_ST_MT (Δ: 444)
• rec_dis_5x41_minQ2=0_craterlake_5x41 (Δ: 4601)
• rec_e_1GeV_20GeV_craterlake (Δ: 0)
• rec_pi_1GeV_20GeV_craterlake (Δ: 3639)
• two_stage_raw_e_1GeV_20GeV_craterlake (Δ: 0)
• two_stage_rec_e_1GeV_20GeV_craterlake (Δ: 0)

Last updated 2026-08-25T13:52-04:00 c84d062

[Copilot]

Pull request overview

Copilot reviewed 7 out of 7 changed files in this pull request and generated 1 comment.

Suppressed comments (1)

src/tests/algorithms_test/calorimetry_HEXPLIT.cc:107

• The contribution proxy is never inserted into an edm4hep::CaloHitContributionCollection; simhit.addToContributions therefore records an invalid relation. As a result getContributions() cannot reach secondary, so the linked timing tests do not provide a t0 and the two rejection cases will pass all 60 hits instead of exercising the new cut. Keep a contribution collection in TruthChain and create the contribution with create().

  edm4hep::MutableCaloHitContribution contrib;
  contrib.setParticle(secondary);
  contrib.setTime(hit_time_ns);
  contrib.setEnergy(energy);
  simhit.addToContributions(contrib);