Skip to content

feat: ALERT pre- and post-KF AI PID inference - #1368

Open
skuditha wants to merge 4 commits into
JeffersonLab:developmentfrom
skuditha:alert-ai-pid
Open

feat: ALERT pre- and post-KF AI PID inference#1368
skuditha wants to merge 4 commits into
JeffersonLab:developmentfrom
skuditha:alert-ai-pid

Conversation

@skuditha

Copy link
Copy Markdown
Contributor
  • Replaces the existing ALERT pre-PID implementation with the new AHDC- and ATOF-based TorchScript models.
  • Adds post-KF PID inference for every valid ALERT::ai:projections pair.
  • Writes predicted particle IDs and class probabilities to the existing ALERT AI PID banks.

@skuditha
skuditha marked this pull request as ready for review July 30, 2026 13:15

@mathieuouillon mathieuouillon left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The old translator did logits.softmax(1) before toFloatArray(). Both new translators (ModelPrePID.java:290, ModelPostPID.java:84) read the raw output tensor. argmax is unaffected, so pid/prepid are fine either way, but if the exported TorchScript modules end in a Linear rather than a Softmax, then p2212/prob_2212/... are logits, and downstream users of a bank field literally named prob_* will be misled. Can you confirm the export includes the softmax?

"AHDC::mc", "AHDC::kftrack",
"ALERT::projections", "ALERT::ai:projections", "ALERT::prePID");
"ALERT::projections", "ALERT::ai:projections",
"ALERT::ai:prePID", "ALERT::ai:postPID");

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wrong banks name: look into etc/bankdefs/hipo4/alert.json
"name": "ALERT::ai:prepid", and name": "ALERT::ai:pid"

}
}
}
rbc.appendPIDBank(event, pidResults);

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

appendPIDBank is outside the isFinite(startTime) guard, so a zero-row ALERT::ai:pid is appended to every qualifying event. Consider if (!pidResults.isEmpty()).

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(1) Confirmed. I inspected the TorchScript graphs for both pre-PID models and the post-KF model. All three exported modules apply softmax(..., -1) inside their forward() method before returning the output tensor. Therefore, the values read by the new translators are already normalized class probabilities, not logits. Applying another softmax in Java would double-transform the probabilities, so the current translators are correct as written.
(2) Thank you! I will fix it. But it does not affect the output for some reason.
(3) Will do. Again, a zero row ALERT::ai:pid did not appear in my tests. So, there seems to be no effect.

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.

2 participants