Skip to content

TrackParamTruthInit: implement relaxed mode to include secondaries in B0TrackerTruthSeeds - #2901

Open
veprbl wants to merge 10 commits into
mainfrom
pr/relaxed_b0_truth_seeding
Open

TrackParamTruthInit: implement relaxed mode to include secondaries in B0TrackerTruthSeeds#2901
veprbl wants to merge 10 commits into
mainfrom
pr/relaxed_b0_truth_seeding

Conversation

@veprbl

@veprbl veprbl commented Aug 26, 2026

Copy link
Copy Markdown
Member

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

  1. This continues from Allow truth seeder to work with secondary particles #2797
  2. B0 truth seeding is in a different code path for which
    • The z cut is relaxed
    • Extrapolation to perigee surface is replaced with somewhat degenerate perigee case – surface is placed at the vertex itself
      • This does not seem to break anything, including the existing tracking for the primaries
  3. There is one known inefficiency, the tracks are still extrapolated to perigee at (0,0,0), which will not always work for the secondaries. We would probably need to provide "global" track mode for that.

Addresses #2746

What is the urgency of this PR?

  • High (please describe reason below)
  • 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.
    This change removes the legacy TrackerTruthSeeds / TrackerTruthSeedParameters collections from the default PODIO output list and replaces them with the new central/B0-specific names.
  • This PR changes default behavior. Please describe changes below.
    B0 tracking works slightly differently, but there should not be a visible change to the user analyses
  • AI was used in preparing this PR. Please describe usage below.
    Claude

Copilot AI lite review requested due to automatic review settings August 26, 2026 03:49
@github-actions github-actions Bot added the topic: tracking Relates to tracking reconstruction label Aug 26, 2026

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR updates the truth-seeding path for B0 tracking so that displaced secondary tracks (e.g., Λ decay daughters) can be seeded and reconstructed more effectively in the B0 tracker chain by relaxing vertex constraints and allowing the seed’s perigee reference point to be anchored at the production vertex.

Changes:

  • Add a dedicated B0TrackerTruthSeeds truth seeder with relaxed vertex-z acceptance and useVertexAsPerigee=true.
  • Introduce useVertexAsPerigee in TrackParamTruthInitConfig and expose it via factory parameters.
  • Propagate the seed’s perigee reference point into CKF initialization (PerigeeSurface center derived from the seed).

Reviewed changes

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

Show a summary per file
File Description
src/global/tracking/tracking.cc Reworks truth-seed factory wiring; adds a dedicated B0 truth seeder with relaxed configuration.
src/factories/tracking/TrackParamTruthInit_factory.h Exposes UseVertexAsPerigee as a configurable parameter.
src/algorithms/tracking/TrackParamTruthInitConfig.h Adds config flag and documentation for vertex-anchored perigee mode.
src/algorithms/tracking/TrackParamTruthInit.cc Implements vertex-anchored perigee option and updates seed perigee storage.
src/algorithms/tracking/CKFTracking.cc Builds the initial perigee surface from the seed’s stored perigee reference point.
Suppressed comments (1)

src/global/tracking/tracking.cc:63

  • "B0TrackerTruthSeeds" now comes from a separate TrackParamTruthInit instance that writes its parameters to "B0TrackerTruthSeedParameters". The default PODIO output collections include "B0TrackerTruthSeeds" but do not include "B0TrackerTruthSeedParameters", which is likely to create dangling relations (TrackSeed -> TrackParameters) in the output file.
  app->Add(new JOmniFactoryGeneratorT<TrackParamTruthInit_factory>(
      "B0TrackerTruthSeeds", {"EventHeader", "MCParticles"},
      {"B0TrackerTruthSeeds", "B0TrackerTruthSeedParameters"},
      TrackParamTruthInitConfig{

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

Comment thread src/global/tracking/tracking.cc Outdated
Comment thread src/algorithms/tracking/TrackParamTruthInit.cc
Comment thread src/algorithms/tracking/CKFTracking.cc Outdated
Copilot AI review requested due to automatic review settings August 26, 2026 03:54

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

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

Comment thread src/global/tracking/tracking.cc Outdated
… B0TrackerTruthSeeds (fix: iwyu) (#2902)

This PR applies the include-what-you-use fixes as suggested by
https://github.com/eic/EICrecon/actions/runs/32928057137.
Please merge this PR into the branch `pr/relaxed_b0_truth_seeding`
to resolve failures in PR #2901.

Auto-generated by [create-pull-request][1]

[1]: https://github.com/peter-evans/create-pull-request

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Copilot AI review requested due to automatic review settings August 26, 2026 14:42

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

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

Suppressed comments (2)

Previously missed (1) — in code that hasn't changed since the last review.

src/algorithms/tracking/TrackParamTruthInit.cc:113

  • Potential division-by-zero when computing the transverse PCA if the particle has pT == 0 (p.x == p.y == 0). That will produce NaNs in the seed parameters for exactly-forward tracks. Guard the denominator and fall back to using the vertex as the PCA in this degenerate case.
      perigee_center             = Acts::Vector3(0, 0, 0);
      auto linesurface_parameter = -(v.x * p.x + v.y * p.y) / (p.x * p.x + p.y * p.y);
      global = Acts::Vector3(v.x + linesurface_parameter * p.x, v.y + linesurface_parameter * p.y,
                             v.z + linesurface_parameter * p.z);

src/global/tracking/tracking.cc:50

  • This wiring no longer produces a collection named "TrackerTruthSeeds" (it now writes directly to "CentralTrackerTruthSeeds"). However, the PODIO default output list still includes "TrackerTruthSeeds" (see src/services/io/podio/JEventProcessorPODIO.cc:65), so this change can lead to missing/empty collections (and may break user scripts expecting TrackerTruthSeeds). Consider either continuing to produce TrackerTruthSeeds (and optionally aliasing it to CentralTrackerTruthSeeds) or updating the default output collection list accordingly.
  app->Add(new JOmniFactoryGeneratorT<TrackParamTruthInit_factory>(
      "CentralTrackerTruthSeeds", {"EventHeader", "MCParticles"},
      {"CentralTrackerTruthSeeds", "TrackerTruthSeedParameters"}, {}, app));

Comment thread src/global/tracking/tracking.cc Outdated
Copilot AI review requested due to automatic review settings August 26, 2026 14:48

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

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

Comment thread src/services/io/podio/JEventProcessorPODIO.cc
Comment thread src/algorithms/tracking/CKFTracking.cc
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings August 26, 2026 15:01

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

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

Comment thread src/services/io/podio/JEventProcessorPODIO.cc
Comment thread src/algorithms/tracking/TrackParamTruthInit.cc
Comment thread src/global/tracking/tracking.cc Outdated
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings August 28, 2026 17:08

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

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

Suppressed comments (2)

src/global/tracking/tracking.cc:65

  • maxEtaBackward = 0.0 only excludes negative-eta particles; with maxEtaForward = 6.0 this B0 truth seeder will accept all particles with 0 <= eta <= 6 (including many non-B0-acceptance tracks, e.g. eta≈0), which can dramatically increase the number of truth seeds (and CKF iterations) with no corresponding B0 hits. This also doesn’t match the inline comment “forward-going particles only” if the intent is the sub-50 mrad B0 region. Consider adding an explicit forward lower bound (e.g. minEtaForward or maxTheta) to TrackParamTruthInitConfig/algorithm, or reinstating an angular split/filter so B0 only seeds the intended small-theta region.
      TrackParamTruthInitConfig{
          .maxVertexX         = 120 * dd4hep::mm, // maxVertexZ * tan(20mrad) ~ 120mm
          .maxVertexY         = 120 * dd4hep::mm,
          .maxVertexZ         = 6000 * dd4hep::mm, // B0 tracker starts at ~6 m from IP
          .minMomentum        = 100 * dd4hep::MeV,
          .maxEtaForward      = 6.0,
          .maxEtaBackward     = 0.0, // forward-going particles only
          .momentumSmear      = 0.1,
          .useVertexAsPerigee = true, // anchor at decay vertex, not beam-axis PCA

src/algorithms/tracking/CKFTracking.cc:196

  • The initial Acts PerigeeSurface is now constructed at track_seed.getPerigee(), but later the code still extrapolates every found track to a fixed perigee surface at the IP (Acts::Vector3{0,0,0}) and drops the track if that extrapolation fails. For displaced truth seeds (where the perigee is intentionally moved to the decay vertex), this can still cause secondary tracks to be discarded after successful finding/smoothing, undermining the purpose of UseVertexAsPerigee. Consider extrapolating to a per-seed reference surface (matching the seed perigee) or accepting the smoothed track even if IP perigee extrapolation fails (e.g. keep it at the seed/measurement reference surface and log a warning).
    // Construct the perigee surface at the seed's reference point.
    // For IP-originating tracks this is (0,0,0); for displaced seeds (e.g. from
    // Lambda decay daughters) the seed carries the actual decay-vertex position
    // so the CKF propagates from there instead of from the IP.
    const auto& perigee_pos = track_seed.getPerigee();
    auto pSurface           = Acts::Surface::makeShared<const Acts::PerigeeSurface>(Acts::Vector3(
        perigee_pos.x * Acts::UnitConstants::mm, perigee_pos.y * Acts::UnitConstants::mm,
        perigee_pos.z * Acts::UnitConstants::mm));

Comment thread src/services/io/podio/JEventProcessorPODIO.cc
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings August 28, 2026 17:32

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

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

Suppressed comments (1)

src/algorithms/tracking/TrackParamTruthInit.cc:113

  • Potential division-by-zero in the PCA back-extrapolation when the particle momentum is parallel (or nearly parallel) to the beam axis: (p.x*p.x + p.y*p.y) can be 0, producing NaNs in linesurface_parameter/global and causing globalToLocal to fail.
      // Standard: perigee at beam axis origin, back-extrapolate to transverse PCA.
      perigee_center             = Acts::Vector3(0, 0, 0);
      auto linesurface_parameter = -(v.x * p.x + v.y * p.y) / (p.x * p.x + p.y * p.y);
      global = Acts::Vector3(v.x + linesurface_parameter * p.x, v.y + linesurface_parameter * p.y,
                             v.z + linesurface_parameter * p.z);

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

topic: tracking Relates to tracking reconstruction

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants