Skip to content

Conversation

@naomiOvad
Copy link

@naomiOvad naomiOvad commented Oct 29, 2025

Description

This PR fixes the behavior of the reduction operators so it's aligned with the ONNX specification.
See ONNX ReduceSumSquare Specification
for the definition of noop_with_empty_axes and expected behavior when axes=[].

Main changes:

  • Added a new helper function ApplyNoopEmptyAxesElementwise() to handle the case axes=[] and noop_with_empty_axes=1 for all reduction operators (ReduceSumSquare, ReduceL1, ReduceL2, ReduceLogSum, ReduceLogSumExp, etc.).
    This function performs elementwise operations according to the aggregator type:

If the aggregator defines Pre/Post operations (e.g., abs, square, sqrt, log), they are applied elementwise on each element of the input without reduction.

Otherwise, a direct memory copy (memcpy) is performed to efficiently produce an identical output.

  • Introduced a compile-time trait system ReduceAggTraits to detect whether each aggregator defines a PreOp and/or PostOp.
    This allows compile-time specialization and avoids redundant runtime checks.

  • Updated the generic reduction paths (CommonReduce1Loop and CommonReduce2Loops)
    to invoke ApplyNoopEmptyAxesElementwise() when axes=[] and noop_with_empty_axes=1.
    These paths are used by all reduction operators, ensuring consistent handling across the entire operators.

  • Fixed the conditional inside CommonFastReduceCopy: replaced the previous unconditional memcpy and return true with a safe return false, since empty-axes cases are now fully handled upstream by ApplyNoopEmptyAxesElementwise().
    This keeps the control flow explicit and prevents unintended fallback copies.

  • Added unit tests for all affected reduction operators (ReduceSumSquare, ReduceL1, ReduceL2, ReduceLogSum, ReduceLogSumExp),and corrected one test that previously expected identity output but now correctly applies Pre/Post operations per the ONNX specification.

These updates ensure spec-compliant behavior for all Reduce ops and slightly improve performance for identity cases.

Motivation and Context

Before this fix, some Reduce operators (e.g., ReduceSumSquare, ReduceL1, ReduceL2, ReduceLogSum, ReduceLogSumExp) did not follow the ONNX spec when axes=[] and noop_with_empty_axes=1.

Per the ONNX specification:

No reduction should occur if axes is empty and noop_with_empty_axes=1.

Operators with Pre/Post (e.g., abs, square, sqrt, log) must apply them elementwise.

Others should return the input unchanged.

Fixes #26288

…h for all reduction operators)

Fixes microsoft#26288

- Handle all Reduce* operators consistently when axes are empty and noop_with_empty_axes=1
- Apply both Pre-op and Post-op elementwise without performing reduction
- Added comprehensive unit tests for ReduceSumSquare, ReduceL1, ReduceL2, ReduceLogSum, and ReduceLogSumExp covering empty-axes scenarios
@naomiOvad
Copy link
Author

@naomiOvad please read the following Contributor License Agreement(CLA). If you agree with the CLA, please reply with the following information.

@microsoft-github-policy-service agree [company="{your company}"]

Options:

  • (default - no company specified) I have sole ownership of intellectual property rights to my Submissions and I am not making Submissions in the course of work for my employer.
@microsoft-github-policy-service agree
  • (when company given) I am making Submissions in the course of work for my employer (or my employer has intellectual property rights in my Submissions by contract or applicable law). I have permission from my employer to make Submissions and enter into this Agreement on behalf of my employer. By signing below, the defined term “You” includes me and my employer.
@microsoft-github-policy-service agree company="Microsoft"

Contributor License Agreement

@naomiOvad please read the following Contributor License Agreement(CLA). If you agree with the CLA, please reply with the following information.

@microsoft-github-policy-service agree [company="{your company}"]

Options:

  • (default - no company specified) I have sole ownership of intellectual property rights to my Submissions and I am not making Submissions in the course of work for my employer.
@microsoft-github-policy-service agree
  • (when company given) I am making Submissions in the course of work for my employer (or my employer has intellectual property rights in my Submissions by contract or applicable law). I have permission from my employer to make Submissions and enter into this Agreement on behalf of my employer. By signing below, the defined term “You” includes me and my employer.
@microsoft-github-policy-service agree company="Microsoft"

Contributor License Agreement

@naomiOvad naomiOvad closed this Oct 29, 2025
@naomiOvad naomiOvad reopened this Oct 29, 2025
@naomiOvad
Copy link
Author

@naomiOvad please read the following Contributor License Agreement(CLA). If you agree with the CLA, please reply with the following information.

@microsoft-github-policy-service agree [company="{your company}"]

Options:

  • (default - no company specified) I have sole ownership of intellectual property rights to my Submissions and I am not making Submissions in the course of work for my employer.
@microsoft-github-policy-service agree
  • (when company given) I am making Submissions in the course of work for my employer (or my employer has intellectual property rights in my Submissions by contract or applicable law). I have permission from my employer to make Submissions and enter into this Agreement on behalf of my employer. By signing below, the defined term “You” includes me and my employer.
@microsoft-github-policy-service agree company="Microsoft"

Contributor License Agreement

@microsoft-github-policy-service agree

@naomiOvad
Copy link
Author

@microsoft-github-policy-service agree

@naomiOvad naomiOvad marked this pull request as ready for review October 30, 2025 07:03
@naomiOvad
Copy link
Author

@microsoft-github-policy-service agree

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.

Spec changed for reduction ops when noop_with_empty_axes is True

1 participant