Skip to content

[PWGJE] derivedDataProducer: Add guard to collision size - #18064

Closed
mhwang285 wants to merge 1 commit into
AliceO2Group:masterfrom
mhwang285:test-segfault
Closed

mhwang285 wants to merge 1 commit into
AliceO2Group:masterfrom
mhwang285:test-segfault

Conversation

@mhwang285

@mhwang285 mhwang285 commented Sep 26, 2026 •

Copy link
Copy Markdown
Contributor

Occasionally, a track in an AO2D can point to a collision index that does not exist. When one such track is mapped to this "misindexed" collision via track.collision_as() (which occur in L505 and L533), and any property of the collision is queried, it causes a segfault.

Usually this segfault crashes the processing of a single job in a train, but can cause larger issues if a problematic file happens to be a wagon test file. At least two of the wagon test files for the recent 100% production of LHC26a7 on Hyperloop have this issue: /alice/sim/2026/LHC26a7/0/560089/AOD/070/AO2D.root and /alice/sim/2026/LHC26a7/0/560089/AOD/069/AO2D.root. 070 is the second test file, so it is likely to affect many wagon tests (for example, this one). In 070, there are four problematic DFs:

  1. DF_2405564011777600
  2. DF_2405564011953984
  3. DF_2405564011954240
  4. DF_2405564012012864

The first DF contains 398 collisions in O2collision_001 (i.e. collision indices 0 through 397) but the fIndexCollisions branch shows that some tracks in O2track_iu point to collision index 398. The other three DFs have zero collisions (i.e. no entries in O2collision_001), but have tracks in O2track_iu that all point collision index 0. Note that track.collision_as().has_collision() still returns true and track.collsionId() is greater than or equal to zero for all of these tracks, so the usual checks don't seem to avoid this particular problem.

Both of these cases can be avoided if the track's collision ID is checked to be strictly less than the number of collisions (which I believe should be true normally if the files weren't corrupted). For the derived data producer, this issue affects processTrackSelectionForWeightedMC and processTracks. For now I've implemented a configurable that turns on the extra check (default is off), where the check basically skips the problematic tracks. Eventually it would be important to know from the experts whether such tracks should be considered as collision-less (equivalent to tracks with track.collisionId() < 0 and track.has_collision() = false) or thrown out entirely, or maybe this file should be considered as corrupted as a whole and deleted, but for now, I've chosen the more conservative option to throw out the tracks.

@github-actions github-actions Bot added the pwgje label Sep 26, 2026
@github-actions

Copy link
Copy Markdown

O2 linter results: ❌ 0 errors, ⚠️ 43 warnings, 🔕 0 disabled

@github-actions github-actions Bot changed the title derivedDataProducer: Add guard to collision size [PWGJE] derivedDataProducer: Add guard to collision size Sep 26, 2026
@mhwang285
mhwang285 marked this pull request as ready for review September 28, 2026 06:15
@nzardosh

Copy link
Copy Markdown
Collaborator

Hi Tucker, sorry if I am misunderstanding something but in both places where you have put this fix we already check that either track.has_collision() or track.collisionId() >= 0 (both are doing the same thing). These should guard against these tracks (known as orphan tracks) which don't have an associated collision and are common in many datasets.

@mhwang285

mhwang285 commented Sep 28, 2026 •

Copy link
Copy Markdown
Contributor Author

Hi @nzardosh, the problem with these tracks is not that they have no collision; in fact, when you check has_collision() or collisionID(), they return true and a value greater than or equal to zero. The problem is that, although has_collision() and collisionID() tell you it has a collision, this collision doesn't actually exist in the file. This is because the collision at the collision index that collisionID() points to does not actually exist in the file. The DFs I wrote about in the top are the two examples I found of such corruptions: one where the collision table contains only 398 collisions (so collision indices 0 to 397), but the collisionID points to collision index 398 (so one beyond the maximum index), and three where the collision table is actually empty, but there are still tracks that point to collision index 0 (which cannot exist since there are no collisions in this DF). In both these cases has_collision() still returns true and and collisionID() is >= 0 so these checks do not actually protect against this particular issue; from the code's perspective, the track does have a corresponding collision, since fIndexCollisions has a value >= 0, but it doesn't seem to cross-check this with the collision table to see if a collision with that index actually exists there as well.

This is why the fix also checks that the collision ID for the track is a valid one, not just that it is >= 0, by checking it against the maximum possible index. This captures both cases I've seen (since 398 >= 398 and 0 >= 0) but it's still possible there are others. I've cross-checked this fix against two of the problematic files I found (the two mentioned at the top), and both bypass the problematic tracks as expected.

@nzardosh

Copy link
Copy Markdown
Collaborator

@mhwang285 if you agree we can close this PR to keep the functionality of spotting the buggy reconstruction

@mhwang285 mhwang285 closed this Sep 29, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Development

Successfully merging this pull request may close these issues.

2 participants