Continue with RG-L Fix in the HitReader and AHDCEngine #1207
Open
mathieuouillon wants to merge 6 commits intodevelopmentfrom
Open
Continue with RG-L Fix in the HitReader and AHDCEngine #1207mathieuouillon wants to merge 6 commits intodevelopmentfrom
mathieuouillon wants to merge 6 commits intodevelopmentfrom
Conversation
mathieuouillon
requested review from
N-Plx,
baltzell,
efuchey,
ftouchte,
mpaolone and
whit2333
as code owners
mathieuouillon
removed request for
N-Plx,
efuchey,
ftouchte,
mpaolone and
whit2333
In simulation mode fetch_AHDCHits was still reading the per-wire rawHitCuts table even though the pass/fail result was discarded, which is wasted work and can fail when the sim CCDB run has no cut entries. The cut lookups and pass check are now nested inside the existing !sim branch alongside the ToT and ADC-gain corrections, so sim events go straight from time calibration to DOCA. While in the file: - Drop the rawHitCutsTable/timeOffsetsTable/... instance fields; the IndexedTables are passed through to fetch_AHDCHits and T2Dfunction as parameters, matching how they are used. - Make T2Dfunction / fetch_AHDCHits / fetch_TrueAHDCHits private; nothing outside HitReader calls them. - Collapse the DOCA branch to a single ternary. - Add Javadoc on the class, constructor, calibration pipeline, T2D function, and the hit/true-hit accessors.
…p unused MaterialMap - Add AHDC::interclusters and AHDC::docaclusters to registerOutputBank so framework bank management (clearing, schema lookup) sees them. - Use a per-event effectiveMode local instead of overwriting the modeTrackFinding instance field when an event exceeds MAX_HITS_FOR_AI; previously a single noisy event forced CV_Distance for the rest of the run. - Remove the unused materialMap field and its MaterialMap/Material imports; the Kalman filter no longer consumes it from AHDCEngine.
mathieuouillon
force-pushed
the
rgl-hit-fix
branch
from
155cb41 to
0a376e9
Compare
Rename all IndexedTable fields, parameters, and javadoc references in the ALERT engine suite to carry a "Table" suffix, making calibration-table variables easy to spot at a glance. Touches AHDCEngine, ATOFEngine, ALERTEngine, HitReader, HitFinder, ATOFHit, and BarHit.
Collaborator
|
Thanks! |
baltzell
previously approved these changes
Apr 13, 2026
Collaborator
Author
|
New issue with the last version on ALERTEngine, my previous fix only move the problem. Now I have the following issue: I will work on that. |
The Kalman preprocessing loop read tracks back via AHDC_tracks.get(row), which breaks as soon as the empty-hit guard skips a row and desynchronises row from the list index. Build each Track through a local reference, initialise position/momentum/trackid, then append — so skipped rows never poison later iterations. Also log a warning on the skip branch so the upstream "AHDC::track row with no matching AHDC::hits" case is visible.
Collaborator
Author
|
It should fix the issue but I need to understand why a row in AHDC::track can ever have zero matching hits in AHDC::hits |
The AI candidate generator routinely emits overlapping TrackPredictions that share PreCluster (and therefore Hit) references. Accepting all predictions above threshold let later tracks silently steal earlier tracks' hits via in-place set_trackId() mutation, leaving orphan rows in AHDC::track with no matching rows in AHDC::hits — which in turn crashed the ALERTEngine Kalman loop with IndexOutOfBoundsException inside Track(ArrayList<Hit>). Sort predictions by score descending, greedily accept each one only if none of its PreClusters has already been claimed, enforcing one-hit one-track.
Collaborator
Author
|
Ok, it is enough for one pull request, and I can cook 35k events without any warning or error. Can be merged now. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Skip per-wire raw-hit cut lookups in simulation mode in
HitReader.fetch_AHDCHits; they were being read and then discarded, and could fail when the sim CCDB run has no cut entries. The cut lookups now live inside the existing!simbranch alongside the ToT and ADC-gain corrections.Fix a sticky-mode bug in
AHDCEngine: once a single event exceededMAX_HITS_FOR_AI, the instancemodeTrackFindingfield was overwritten toCV_Distancefor the rest of the run. It now uses a per-eventeffectiveModelocal.Register
AHDC::interclustersandAHDC::docaclusterswithregisterOutputBank— both were being appended to the event without being declared.Minor
HitReadercleanup: drop unused instance fields for the calibration tables (they are passed as parameters), makeT2Dfunction/fetch_AHDCHits/fetch_TrueAHDCHitsprivate, collapse the DOCA branch to a ternary, and add Javadoc.Drop the unused
materialMapfield and itsMaterialMap/Materialimports fromAHDCEngine.Rename all IndexedTable fields, parameters, and javadoc references in the ALERT engine suite to carry a "Table" suffix, making calibration-table variables easy to spot at a glance.