Skip to content

Fix rank-3 softmax conversion to NeuralNetwork SoftmaxLayer - #2856

Open
cpruijsen wants to merge 1 commit into
apple:mainfrom
cpruijsen:fix/issue-1714
Open

cpruijsen wants to merge 1 commit into
apple:mainfrom
cpruijsen:fix/issue-1714

Conversation

@cpruijsen

Copy link
Copy Markdown

Summary

Rank-3 MIL softmax(axis=-3) now lowers to NeuralNetwork SoftmaxND instead of SoftmaxLayer. The runtime applies SoftmaxLayer along axis -1 when rank is 3, so the converted model was numerically wrong (reporter's (2, 1, 1) example predicted all-ones instead of a 2-class softmax). Rank ≥ 4 is unchanged.

Proto comment for SoftmaxLayerParams now describes the rank-3 runtime exception.

Fixes #1714.

Decision

Emit SoftmaxND when rank < 4; keep SoftmaxLayer for rank ≥ 4 when the axis is -3 / N-3. Document the rank-3 runtime in NeuralNetwork.proto.

Alternative: always emit SoftmaxND (never SoftmaxLayer). The same rank >= 4 gate is already used by concat in op_mapping.py. Rank ≥ 4 SoftmaxLayer is still the layer type on-device-update / CCE expects (#1705). Rank-3 SoftmaxLayer was numerically wrong, so it is not a usable updatable workaround.

Can switch to always-SoftmaxND, or drop the proto comment and keep the converter-only workaround.

Test plan

  • Reporter's (2, 1, 1) / axis=-3 program with convert_to="neuralnetwork": predict matches scipy.special.softmax; spec uses softmaxND with axis == -3.
  • (1, 1, 2) confirms pre-fix SoftmaxLayer was axis -1; after the fix, axis=-3 is a size-1 softmax (all-ones) as expected.
  • Rank-4 axis=-3 still emits SoftmaxLayer and matches scipy.
  • TestSoftmax including the new cases (neuralnetwork + mlprogram). Tests fail on neuralnetwork if the mapping change is reverted.
  • mlprogram on the reporter's graph: already matched scipy on current macOS before this change; not a converter bug on this tree.

The NeuralNetwork SoftmaxLayer applies axis=-1 for rank-3 inputs, so
MIL softmax(axis=-3) must lower to SoftmaxND. Rank >= 4 is unchanged.

Fixes apple#1714
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.

Softmax with rank 3 has wrong semantics and wrong MIL->NNv1 conversion.

1 participant