Support float16 and bfloat16 in CPU ScatterND reductions - #32065
Open
Alexander Novikov (novikov-alexander) wants to merge 2 commits into
Open
Support float16 and bfloat16 in CPU ScatterND reductions#32065Alexander Novikov (novikov-alexander) wants to merge 2 commits into
Alexander Novikov (novikov-alexander) wants to merge 2 commits into
Conversation
ScatterND threw ORT_NOT_IMPLEMENTED on the CPU EP for MLFloat16 and BFloat16 under every reduction: 'add', 'mul', 'min' and 'max'. docs/OperatorKernels.md already lists tensor(float16) and tensor(bfloat16) for this kernel, because the table is generated from the registrations and both types are in element_type_lists::All, so the op claimed support it did not have. 'add' and 'mul' are now evaluated in float and rounded back to half on each update, matching what ScatterElements does after microsoft#32025 and what the CUDA and WebGPU kernels do. The stubs existed because the generic functors use compound assignment, which neither half type defines. Func_Min_ND and Func_Max_ND needed no implementation and are removed instead: both half types have the comparison operators the generic functors use, so they now fall through to the generic path. Nothing forced those four stubs. Note this is a wider gap than ScatterElements had, which never threw for MLFloat16 min/max. No registration or type constraint changes, so docs/OperatorKernels.md does not need regenerating. The remaining ORT_NOT_IMPLEMENTED cases are bool and string, which are genuine. Tests cover all eight previously-throwing combinations, with an index repeated so the reduction is applied more than once per output slice, and values chosen to be exact in both half formats. All eight fail against the previous kernel. A ninth pins the accumulation precision the same way microsoft#32025 does; since ONNX does not specify intermediate precision, that one runs on CPU only. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
Azure Pipelines: There may be pipelines that require an authorized user to comment /azp run to run. |
They would fail on the CUDA plugin EP for a reason unrelated to this change: that kernel selects its compute type by element size, so it treats bfloat16 as float16 and reduces misread bits (microsoft#32061). Scoped conservatively, including min and max, since I have no CUDA hardware to confirm which of them survive the misinterpretation. The float16 tests are unaffected and still run everywhere. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
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
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.
Description
ScatterNDthrowsORT_NOT_IMPLEMENTEDon the CPU EP forMLFloat16andBFloat16under every reduction —add,mul,minandmax. This implements them.This is the same gap #32025 closes for
ScatterElements, andScatterNDis a step further behind:ScatterElementsnever threw forMLFloat16min/max, whereasScatterNDdoes.addandmulare now evaluated in float and rounded back to half on each update:Those stubs existed for a mechanical reason: the generic functors use compound assignment (
(*a++) += (*b++)), and neither half type definesoperator+=.Func_Min_NDandFunc_Max_NDare removed rather than implemented. Nothing forced those four stubs — both half types have the comparison operators the generic functors already use, so they now fall through to the generic path, exactly asMLFloat16has always done inScatterElements.That takes the file from 13
ORT_NOT_IMPLEMENTEDcases down to 5. The remaining five areboolandstd::string, which are genuine.Motivation and Context
docs/OperatorKernels.mdalready liststensor(float16)andtensor(bfloat16)for CPUScatterND, since the table is generated from the registrations and both types are inelement_type_lists::All. So the op advertises support it does not have, and a half-precision model using any reduction fails at inference time rather than falling back to anything.No kernel registration or type-constraint changes, so
docs/OperatorKernels.mddoes not need regenerating.On accumulation precision: rounding to half after every update rather than accumulating in float and rounding once. That matches what the other backends implementing these reductions do (CUDA's
atomicAdd(half), WebGPU's f16 add), it is what #32025 does forScatterElements, and it is the natural fit for a functor that walks one element at a time.Tests
Eight tests cover every previously-throwing combination —
add,mul,min,max×MLFloat16,BFloat16. Each repeats an index so the reduction is genuinely applied more than once to the same output slice, and uses small powers of two that are exact in both half formats, so the expected values do not depend on rounding. All eight fail against the unmodified kernel — verified by reverting onlyscatter_nd.cc, rebuilding and re-running.A ninth test pins the accumulation precision the way #32025 does: eight
0.25updates onto1024.0, where one ULP is1.0in binary16, so per-update rounding leaves1024while a float accumulator rounded once would give1026. ONNX does not specify the intermediate precision, so that is a property of this kernel rather than of the operator, and it runs on CPU only rather than asserting anything about other providers.Full
onnxruntime_provider_testsweep: 5645 tests, 5457 passed, 0 failures, unchanged from baseline.This is independent of #32025 — different file, no overlap — so the two can land in either order.