fix: switch from OrthogonalSeeding to Seeding2 TripletSeeder

[wdconinc]

Briefly, what does this PR introduce?

This PR updates the default seeding method selection logic so that the modern Seeding2 TripletSeeder is used with Acts 45.3+, and fixes a bug in the Seeding2 path that caused ~20% more seeds compared to the Orthogonal seeder.

Changes:

1. TrackSeedingConfig.h: Updated the preprocessor condition from Acts_VERSION_MAJOR <= 46 to Acts_VERSION_MAJOR < 45 || (Acts_VERSION_MAJOR == 45 && Acts_VERSION_MINOR < 3), so Seeding2 is the default for Acts ≥ 45.3.

2. TrackSeeding.cc (bug fix): The Seeding2 path was calling createSeedsFromGroup twice per middle space point — once for low-to-high z candidates (lh) and once for high-to-low z candidates (hl). Because BroadTripletSeedFilter::filterTripletsMiddleFixed clears its candidate collector after each call, maxSeedsPerSpM was applied independently per z-direction. With the default maxSeedsPerSpM = 0 (= 1 seed allowed), this permitted up to 2 seeds per middle SP, while the Orthogonal seeder accumulates candidates from both directions in a single filter pass allowing only 1 seed total.

   Fix: combine bottom_lh_v + bottom_hl_v and top_lh_v + top_hl_v into single subsets and call createSeedsFromGroup once per middle SP. This is physically valid since all bottom candidates (lh + hl) have r < rM and all top candidates have r > rM; the doublet finder's physics cuts (cotTheta, deltaR, impact parameter) naturally reject invalid combinations. This matches the Orthogonal seeder behavior and eliminates the ~20% excess seeds.

What is the urgency of this PR?

• High (please describe reason below)
• Medium
• Low

What kind of change does this PR introduce?

• Bug fix (Seeding2 path produced ~20% more seeds than Orthogonal due to per-direction seed counting)
• New feature (complete switch to Seeding2 for Acts ≥ 45.3)
• 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 PR changes default behavior. Seeding2 becomes the default for Acts ≥ 45.3 (was Acts ≥ 47).
• AI was used in preparing this PR. Root cause identified and fix implemented with GitHub Copilot.

[Copilot]

Pull request overview

This PR adjusts the compile-time default seeding-method selection in TrackSeedingConfig so that newer Acts versions default to the modern Seeding2-based implementation rather than the legacy Orthogonal seeding.

Changes:

• Updated the Acts-version preprocessor cutoff controlling the default SeedingMethod choice (Orthogonal vs Seeding2).

Suppressed comments (1)

src/algorithms/tracking/TrackSeedingConfig.h:51

• The default seedingMethod selection is still based only on Acts_VERSION_MAJOR, but TrackSeeding’s availability logic distinguishes Seeding2 support at Acts 45.3+ (and Seeding at 47+). Keeping the default check in sync with the actual availability thresholds avoids future drift and enables Seeding2 by default for Acts 45.3+ where it is already supported.

#if Acts_VERSION_MAJOR <= 45
  SeedingMethod seedingMethod = SeedingMethod::Orthogonal;
#else
  SeedingMethod seedingMethod = SeedingMethod::Seeding2;
#endif

---

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

[wdconinc]

FYI @ShujieL @bschmookler Please study the new seeding algorithm on a deterministic timeframe so we can move towards it by default. Parameters can always be tweaked later.

[github-actions]

Capybara summary for PR 2910

• rec_dis_10x100_minQ2=0_craterlake_10x100_MT (Δ: 1003)
• rec_dis_10x100_minQ2=100_craterlake_tracking_only (Δ: 613)
• rec_dis_10x130_minQ2=1_craterlake_10x130 (Δ: 1222)
• rec_dis_10x250_minQ2=1_craterlake_10x250 (Δ: 1040)
• rec_dis_18x275_minQ2=0_craterlake_18x275 (Δ: 959)
• rec_dis_18x275_minQ2=0_craterlake_18x275_MT (Δ: 1047)
• rec_dis_18x275_minQ2=0_craterlake_18x275_ST_MT (Δ: 402)
• rec_dis_18x275_minQ2=0_craterlake_18x275_Seeding_Seeding2 (Δ: 959)
• rec_dis_18x275_minQ2=100_craterlake_18x275 (Δ: 1705)
• rec_dis_18x275_minQ2=100_craterlake_18x275_MT (Δ: 1778)
• rec_dis_18x275_minQ2=100_craterlake_18x275_ST_MT (Δ: 507)
• rec_dis_18x275_minQ2=100_craterlake_18x275_Seeding_Seeding2 (Δ: 1705)
• rec_dis_5x41_minQ2=0_craterlake_5x41 (Δ: 1058)
• rec_e_1GeV_20GeV_craterlake (Δ: 519)
• rec_pi_1GeV_20GeV_craterlake (Δ: 254)
• two_stage_raw_e_1GeV_20GeV_craterlake (Δ: 0)
• two_stage_rec_e_1GeV_20GeV_craterlake (Δ: 0)

Last updated 2026-09-05T00:09-04:00 ad59e2e

[Copilot]

🔵 Needs a closer look

It modifies core tracking seeding behavior in a way that can impact physics outputs and should be validated by a human reviewer with domain/context checks.

Review details

Suppressed comments (1)

src/algorithms/tracking/TrackSeeding.cc:476

• Same as for bottom candidates: reserve the final size before inserting into combinedTopIndices to minimize per-event allocations in the tight seeding loop.

      combinedTopIndices.clear();
      combinedTopIndices.insert(combinedTopIndices.end(), candidates.top_lh_v.begin(),
                                candidates.top_lh_v.end());
      combinedTopIndices.insert(combinedTopIndices.end(), candidates.top_hl_v.begin(),
                                candidates.top_hl_v.end());

• Files reviewed: 2/2 changed files
• Comments generated: 2
• Review effort level: Lite

[Copilot]

🔵 Needs a closer look

The behavioral bug fix should be backed by a focused regression test (and there’s also a small hotspot allocation issue) to reduce the risk of future regressions in core tracking seeding.

Review details

Suppressed comments (2)

src/algorithms/tracking/TrackSeeding.cc:463

• The change to apply maxSeedsPerSpM across both z-directions is behaviorally significant, but the existing TrackSeeding unit tests only cover the "three hits -> one seed" and "accepted-spacepoint-empty" paths. Adding a regression test that constructs hits producing both lh and hl candidates for the same middle space point and asserts that maxSeedsPerSpM=0 yields at most one seed would help prevent reintroducing the double-counting bug.

      // Combine low-high and high-low z-direction candidates into a single
      // createSeedsFromGroup call per middle SP. This mirrors the Orthogonal
      // seeder which processes both z-directions in a single filter pass and
      // applies maxSeedsPerSpM across all directions combined. Calling
      // createSeedsFromGroup twice (once per direction) would apply the limit
      // independently, allowing up to twice as many seeds per middle SP.
      // All bottom candidates (lh + hl) have r < rM, and all top candidates

src/algorithms/tracking/TrackSeeding.cc:477

• combinedTopIndices is rebuilt for every middle space point but does not reserve capacity before insertions, which can trigger repeated reallocations in a hot loop (combinedBottomIndices already reserves). Reserving upfront avoids unnecessary allocations/copies when candidate counts fluctuate.

      combinedTopIndices.clear();
      combinedTopIndices.insert(combinedTopIndices.end(), candidates.top_lh_v.begin(),
                                candidates.top_lh_v.end());
      combinedTopIndices.insert(combinedTopIndices.end(), candidates.top_hl_v.begin(),
                                candidates.top_hl_v.end());

• Files reviewed: 2/2 changed files
• Comments generated: 0 new
• Review effort level: Lite