Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Align ReduceMin / ReduceMax etc. handling of empty tensors with spec #341

Open
robertknight opened this issue Sep 1, 2024 · 2 comments
Open
Labels
Spec compliance Issues with RTen behavior not matching the ONNX specifications

Comments

@robertknight
Copy link
Owner

The rten implementation of ReduceMin and ReduceMax returns an error if the tensor is empty. However the spec for eg. ReduceMax says:

Reduction over an empty set of values yields minus infinity (if supported by the datatype) or the minimum value of the data type otherwise.

@robertknight robertknight added the Spec compliance Issues with RTen behavior not matching the ONNX specifications label Sep 1, 2024
@robertknight
Copy link
Owner Author

This came up in the context of the DynamicQuantizeLinear operator, which uses ReduceMin / ReduceMax in its definition. I noticed that there was no explicit handling of the case where the input is empty.

@robertknight
Copy link
Owner Author

robertknight commented Oct 18, 2024

The spec-mandated behavior here of returning the minimum or maximum value for the type does have an issue that it can mask errors, less so with floats where the min/max values are +inf/-inf.

For ReduceMean the issue is worse because the handling of empty tenors is specified as undefined:

Reduction over an empty set of values yields undefined.

For RTen I think some general principles are needed for handling of situations that are allowed according to the spec, but highly likely to be errors, and behavior which is undefined according to the spec.

Some possible approaches for undefined behavior include:

  • Making the behavior deterministic, as eg. Rust does with float -> int casts using as. This may have a performance cost though.
  • Triggering an error (eg. as ReduceMean currently does with empty inputs)
  • Allowing the behavior to vary by platform, but specifying that it must have one of a fixed set outcomes. This is how WebAssembly relaxed SIMD handles cross-platform variation

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Spec compliance Issues with RTen behavior not matching the ONNX specifications
Projects
None yet
Development

No branches or pull requests

1 participant