Skip to content

AODBcRewriter: regroup tracks by collision so the -1 group stays contiguous - #2370

Merged
ktf merged 1 commit into
masterfrom
pr2370
Jun 4, 2026
Merged

AODBcRewriter: regroup tracks by collision so the -1 group stays contiguous#2370
ktf merged 1 commit into
masterfrom
pr2370

Conversation

@ktf

@ktf ktf commented Jun 2, 2026

Copy link
Copy Markdown
Member

No description provided.

@ktf
ktf requested review from jackal1-66 and sawenzel as code owners June 2, 2026 13:26
@github-actions

github-actions Bot commented Jun 2, 2026

Copy link
Copy Markdown

REQUEST FOR PRODUCTION RELEASES:
To request your PR to be included in production software, please add the corresponding labels called "async-" to your PR. Add the labels directly (if you have the permissions) or add a comment of the form (note that labels are separated by a ",")

+async-label <label1>, <label2>, !<label3> ...

This will add <label1> and <label2> and removes <label3>.

The following labels are available
async-2023-pbpb-apass4
async-2023-pp-apass4
async-2024-pp-apass1
async-2022-pp-apass7
async-2024-pp-cpass0
async-2024-PbPb-apass1
async-2024-ppRef-apass1
async-2024-PbPb-apass2
async-2023-PbPb-apass5

@ktf

ktf commented Jun 2, 2026

Copy link
Copy Markdown
Member Author

@jgrosseo @jackal1-66 @sawenzel would this make sense? I assume this is the same as sort + uniq if the two collections of BCs are one a subset of the other.

@ktf

ktf commented Jun 2, 2026

Copy link
Copy Markdown
Member Author

I am actually not convinced this changes anything, actually...

@ktf ktf changed the title Do not resort the BC table, just remove the duplicates AODBcRewriter: regroup tracks by collision so the -1 group stays contiguous Jun 3, 2026
@ktf

ktf commented Jun 3, 2026

Copy link
Copy Markdown
Member Author

this one seems to produce a valid file...

@ktf
ktf merged commit 28b44ef into master Jun 4, 2026
17 checks passed
@ktf
ktf deleted the pr2370 branch June 4, 2026 15:12
smaff92 pushed a commit to smaff92/O2DPG that referenced this pull request Jul 14, 2026
sawenzel added a commit that referenced this pull request Jul 28, 2026
…098)

Stage 1b (added in #2370) reorders O2track_iu/O2mfttrack/O2fwdtrack to keep
the "-1" ambiguous group contiguous, but wrote each table as soon as it was
planned. O2fwdtrack references O2mfttrack (reordered later in the same loop)
and itself (fIndexFwdTracks_MatchMCHTrack); neither permutation existed yet
at write time, so both columns kept pre-reorder row numbers -- in range, so
AODBcRewriterValidate passed, while every global muon got a foreign MFT/MCH
leg (Maurice Coquet, sameParticle=0 on an anchored pO sample; patch proposal
#2418). This fires whenever Stage 1b's regroup runs, independent of any
BC/collision reorder, so every merged MC AO2D since 4 Jun is affected.

Fix:
* kIndexRefs: one registry of "which fIndex* column points at which table",
  used by the rewriter, the validator, and a new drift guard (an
  unregistered fIndex* column now fails the test instead of silently
  mis-linking data).
* buildRemaps() derives a table's full remap set from that registry; the old
  "primary index + optional extras" split in rewriteTable is gone, so a
  stage can no longer forget a column the way Stage 1b forgot O2fwdtrack's.
* Planning and writing are separate phases: every stage publishes its row
  permutation before anything is written, so forward/self references
  resolve correctly.

New AODBcRewriterCheckLinks(in, out): fingerprints each row's non-index
payload and checks, per table, that (row, referenced row) pairs survive the
rewrite -- the only check that can see this bug class, since the corrupted
indices were structurally valid. Needs only the input file, so it also runs
on real production AO2Ds.

MC/utils/tests/ adds the executable test this tool never had: a synthetic
AO2D covering every pathology (duplicate/non-monotonic BCs, duplicate MC
collisions, split -1 groups, fwd<->MFT and fwd->fwd references, ...), driven
by run_aodbcrewriter_tests.sh -- ROOT only, seconds to run, nothing binary
committed.

Verified against the pre-fix rewriter and a real anchored pp production
(run 553185, LHC24al apass1): sameParticle collapses to 0 with the shipped
code and is fully restored with this fix, in every scenario tested,
including one engineered to contain genuine duplicate BCs.

Diagnosis and original patch: Maurice Coquet (#2418).

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant