Expose Math.MaxNumber and Math.MinNumber functions that don't propagate NaN

[tannergooding]

Rationale

dotnet/coreclr#20912 updated the existing Math.Min/Math.Max functions to be both IEEE 754:2008 compliant and to be inline with the C/C++ language standard (Annex F - IEC 60559 floating-point arithmetic). This also brought it inline with MSVC, GCC, and Clang for their implementations under 'precise' floating-point mode.

However, it looks like the draft for IEEE 754:2019 (a summary can be found here http://754r.ucbtest.org/background/, but I also have the latest draft locally) is changing things up a bit. Namely, they are removing minNum/maxNum from the list of "required" operations and replacing them with new minimum/maximum and minimumNumber/maximumNumber operations which are recommended by not required and which more clearly specify various behaviors.

• minimumNumber/maximumNumber are largely compatible with the existing minNum/maxNum functions. But also clarifies that +0 is greater than -0 for the purpose of this function and clarifies the behavior of signalling NaN.
• minimum/maximum are new and would propagate the NaN as expected here. They likewise clarify that +0 is greater than -0 for the purpose of this function.

Proposal

Given this change of behavior in the new spec, it might be desirable to revert Math.Min/Math.Max to continue propagating the NaN result. We could then expose new functions which do not propagate the NaN instead.

This would be, overall, a more compatible change and would still allow us to be IEEE compliant.

Such new APIs would be:

public static partial class Math
{
    public static double MaxNumber(double x, double y);
    public static double MaxMagnitudeNumber(double x, double y);
    public static double MinNumber(double x, double y);
    public static double MinMagnitudeNumber(double x, double y);
}

public static partial class MathF
{
    public static float MaxNumber(float x, float y);
    public static float MaxMagnitudeNumber(float x, float y);
    public static float MinNumber(float x, float y);
    public static float MinMagnitudeNumber(float x, float y);
}

Notes

WPF hit a bug due to the change in behavior: dotnet/wpf#521

We have detected a perf regression due to the new behavior being more complex: https://github.com/dotnet/coreclr/issues/22951. We still need to keep the change that +0 is greater than -0, but it will lessen the regression overall and make a fix easier to attain.

IEEE 754:2008 refer to these as minNum, maxNum, minNumMag, and maxNumMag. However, IEEE 754:2019 (draft) refers to them as minimumNumber, maximumNumber, minimumMagnitudeNumber, and maxmimumMagnitudeNumber. I opted with the latter postfix (MagnitudeNumber, rather than NumberMagnitude), as I believe it is more readable.

[tannergooding]

cc. @jkotas, @danmosemsft.

Thoughts?

[jkotas]

Reverting Math.Max/Min to have the original behavior sounds fine to me.

I do not think that the IEEE compliance for these operations is important, especially given that the IEEE is changing their opinion about them as well.

[tannergooding]

I do not think that the IEEE compliance for these operations is important, especially given that the IEEE is changing their opinion about them as well.

It's not that they are changing their opinion as the original functionality for minNum and maxNum remains the same, with an additional clarification that +0 should be greater than -0. Instead, they are now exposing additional functions minimum and maximum that don't propagate NaN based on common implementations and user feedback.

[jkotas]

api-ready-for-review

I do not think we should be adding new APIs in 3.0. Just revert the Math.Max/Min for 3.0, and leave the rest for future.

[tannergooding]

I do not think we should be adding new APIs in 3.0

That would mean we are no longer implementing all IEEE 754:2008 required general operations for 3.0.

I think the best scenario is to:

• Partially revert Math.Max/Math.Min so that they propagate the NaN (as previous); but continue comparing +0 as greater than -0.
• Take the new non-propagating functionality we have already exposed and move it into the new methods I gave above.

This ensures we remain IEEE 754:2008 compliant, ensures that we are not breaking WPF for a scenario they are already depending on, keeps the functionality deterministic (regardless of the ordering of the inputs), and moves us to be inline with IEEE 754:2019.

[jkotas]

That would mean we are no longer implementing all IEEE 754:2008 required general operations for 3.0.

I do not see a problem with it. This is corner case functionality that it fine to mention in the fine print.

We need to get into the shipping mindset for 3.0. Work through the bugs and stop faulting in more nice-to-haves.

[tannergooding]

I do not see a problem with it. This is corner case functionality that it fine to mention in the fine print.

Being able to say that we are IEEE compliant for required operations is something that can be quite important for various numerics libraries, for porting code from native, and for determinism across platforms.

We need to get into the shipping mindset for 3.0. Work through the bugs and stop faulting in more nice-to-haves.

I agree, but if we can fix a "bug" by moving the new functionality to a neighboring function and the code has already been in/tested for many months now; then I think that is better than pulling it out completely. Especially when it allows us to tick a compliance checkbox that has been unchecked for years and include that as part of the 3.0 milestones we reached.

The alternative is that this is pulled out in 3.0; and then reviewed/approved and checked back in for vNext (likely 3.1) which seems to be a worse option considering how trivial the code is.

[tannergooding]

I'll move this to Future for now, but I think that we should consider this for 3.0 as part of the bug fix as it would improve the release story for 3.0 at a trivial cost.

[danmoseley]

I am happy with pushing out this new API and documenting the omission. Thank you @tannergooding

[jkotas]

move this to Future for now

Reverting the behavior of Math.Max/Min should stay in 3.0.

[tannergooding]

Reverting the behavior of Math.Max/Min should stay in 3.0.

I'll open a separate issue to track reverting the behavior to propagate the NaN.

[tannergooding]

Opened: https://github.com/dotnet/corefx/issues/36931

[GrabYourPitchforks]

Feedback: Should we have a wider discussion about putting future APIs on the primitive types (double, half, etc.) instead of the Math / MathF / MathH classes? Otherwise there's likely to be an explosion of Math* types and APIs on those types.

If we decide that putting these APIs on the primitive types isn't worthwhile then the proposal as stated here seems viable. There was some discussion on whether we should keep the Number name and whether those APIs would be discoverable enough, but: (a) these names match what IEEE uses; and (b) it's ok if users keep using the old APIs instead of the new APIs for the majority of use cases.

[tannergooding]

Closing in favor of #20137