Skip to content

[tmva][sofie] Compute LogSoftmax without taking log of softmax - #23554

Merged
guitargeek merged 1 commit into
root-project:masterfrom
Maher-star-eng:sofie-logsoftmax
Sep 30, 2026
Merged

guitargeek merged 1 commit into
root-project:masterfrom
Maher-star-eng:sofie-logsoftmax

Conversation

@Maher-star-eng

Copy link
Copy Markdown
Contributor

This Pull request:

Changes or fixes:

LogSoftmax was computed as the log of the normalized softmax. Where exp(x - max) underflows to 0 in float (inputs about 100 or more below the maximum), this gave -inf instead of a large negative value, and values close to the underflow lost precision (e.g. -99.98 instead of -100). Both code paths of ROperator_Softmax (last axis and generic axis) now compute (x - max) - log(sum), which never takes the log of an underflowed value. Softmax itself is unchanged.

Adds the first LogSoftmax tests, LogSoftmaxLargeRange (last axis) and LogSoftmaxLargeRangeAxis0 (generic path), each with one row of far-apart and one row of ordinary values. Their expected values come from a new _logsoftmax_reference in generate_input_models.py, because onnx.reference also takes the log of the softmax and returns -inf here (onnxruntime gives the correct values). Both tests fail without the fix and pass with it.

Checklist:

  • tested changes locally (ctest -R sofie: all 8 pass)
  • updated the docs (if necessary) — not needed

This PR fixes #23547

AI disclosure: I found this bug with an AI-assisted differential-testing harness (SOFIE vs onnxruntime).

LogSoftmax took the log of the normalized softmax. Where exp(x - max)
underflows to 0 in float, this gave -inf instead of a large negative
value, and it lost precision for values close to underflow. Compute
(x - max) - log(sum) instead, in both the last-axis and the generic
code paths.

Add regression tests LogSoftmaxLargeRange and
LogSoftmaxLargeRangeAxis0, the first LogSoftmax tests. Their expected
values come from a NumPy reference, since onnx.reference also takes the
log of the softmax.
Copilot AI balanced review requested due to automatic review settings September 30, 2026 14:30

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@guitargeek guitargeek left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM!

@guitargeek
guitargeek merged commit 2ec309b into root-project:master Sep 30, 2026
31 of 37 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[tmva][sofie] LogSoftmax returns -inf when the inputs are far apart

4 participants