Skip to content

AODBcRewriter: remap every index column an output table carries (O2-7098) - #2420

Merged
sawenzel merged 2 commits into
AliceO2Group:masterfrom
sawenzel:swenzel/O2-7098-aodbcrewriter-index-remap
Jul 28, 2026
Merged

AODBcRewriter: remap every index column an output table carries (O2-7098)#2420
sawenzel merged 2 commits into
AliceO2Group:masterfrom
sawenzel:swenzel/O2-7098-aodbcrewriter-index-remap

Conversation

@sawenzel

Copy link
Copy Markdown
Contributor

Summary

Fixes O2-7098: O2fwdtrack.fIndexMFTTracks / fIndexFwdTracks_MatchMCHTrack point at
the wrong rows in merged/anchored MC AO2Ds since #2370 (4 Jun 2026), so the MFT and
MCH legs of a global muon end up belonging to different MC particles
(sameParticle=0, reported by @mcoquet642 on an anchored pO production; patch
proposal in #2418).

Root cause: Stage 1b 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;
neither permutation existed yet at write time, so both columns kept pre-reorder row
numbers — in range, so the existing validator passed, while every global muon got a
foreign MFT/MCH leg. 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, generalising #2418's diagnosis rather than patching O2fwdtrack alone (this
is the fourth bug of this shape in the tool):

  • 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 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.
  • A second commit fixes the same family for tables stored sorted by a reference
    into a reordered table (O2fwdtrkcl, O2trackqa_003, O2v0_002, O2cascade_001,
    ...) — their values were remapped but rows left in place, destroying the ordering.
    Confirmed on real files, not just theoretical.

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 runs on real production AO2Ds too.

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 via
o2-aod-merger --max-size to contain genuine duplicate BCs.

Diagnosis and original patch: @mcoquet642 (#2418) — thank you, this generalises your
fix rather than replacing it. Suggest closing #2418 as superseded once this lands, or
happy to have it rebased on top if preferred.

Test plan

  • MC/utils/tests/run_aodbcrewriter_tests.sh fails on pre-fix master, passes
    with this branch
  • Verified on a real anchored pp production AO2D (naturally-split multi-DF case
    and a forced single-DF merge with genuine duplicate BCs)
  • AODBcRewriterValidate and AODBcRewriterCheckLinks both pass on all tested
    files

sawenzel and others added 2 commits July 28, 2026 23:19
…098)

Stage 1b (added in AliceO2Group#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
AliceO2Group#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 (AliceO2Group#2418).

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Second member of the O2-7098 family. Stage 1b reorders O2track_iu/
O2mfttrack/O2fwdtrack; several other tables (O2fwdtrkcl, O2ambiguoustrack,
O2trackqa_003, O2v0_002, O2cascade_001, O2decay3body) are stored SORTED BY a
reference into one of those (or into O2collision). Their index values were
remapped correctly, but their rows stayed put, so a sorted column came out
unsorted -- the same defect as the split "-1" group that once caused

  [FATAL] Table Tracks_IU index fIndexCollisions has a group with index -1
          that is split by 776

Confirmed on real files: O2fwdtrkcl and O2trackqa_003 came out unsorted in
every DF of two of three anchored-MC samples tested.

Fix: findGroupingColumn() + resortByGroupingColumn(), derived from the data
rather than another hardcoded table list -- "if T.B is non-decreasing on
input and B's referent moved, re-sort T by the remapped B". Leaves
O2mfttrackcov alone (not sorted on input). Iterated to a fixed point, since
O2cascade_001 is sorted by fIndexV0s and O2v0_002 may itself have just
moved.

AODBcRewriterCheckLinks() now also asserts sortedness is preserved. Fixture
extended with O2fwdtrkcl and a second cascade to exercise the
O2v0->O2cascade dependency; against the previous commit the suite fails 3
assertions and 6 link checks, and passes with this one.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@sawenzel
sawenzel requested a review from jackal1-66 as a code owner July 28, 2026 21:24
@github-actions

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

@sawenzel
sawenzel merged commit 367e25b into AliceO2Group:master Jul 28, 2026
6 checks passed
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