Repository navigation
Expose Math.MaxNumber and Math.MinNumber functions that don't propagate NaN #29279
Description
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/maximumNumberare largely compatible with the existingminNum/maxNumfunctions. But also clarifies that+0is greater than-0for the purpose of this function and clarifies the behavior of signalling NaN.minimum/maximumare new and would propagate theNaNas expected here. They likewise clarify that+0is greater than-0for 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.
Activity
tannergooding commentedon Apr 16, 2019
cc. @jkotas, @danmosemsft.
Thoughts?
jkotas commentedon Apr 16, 2019
Reverting
Math.Max/Minto 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 commentedon Apr 16, 2019
It's not that they are changing their opinion as the original functionality for
minNumandmaxNumremains the same, with an additional clarification that+0should be greater than-0. Instead, they are now exposing additional functionsminimumandmaximumthat don't propagate NaN based on common implementations and user feedback.jkotas commentedon Apr 16, 2019
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 commentedon Apr 16, 2019
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:
Math.Max/Math.Minso that they propagate the NaN (as previous); but continue comparing+0as greater than-0.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 commentedon Apr 16, 2019
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 commentedon Apr 16, 2019
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.
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 commentedon Apr 16, 2019
I'll move this to
Futurefor 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 commentedon Apr 16, 2019
I am happy with pushing out this new API and documenting the omission. Thank you @tannergooding
jkotas commentedon Apr 16, 2019
Reverting the behavior of Math.Max/Min should stay in 3.0.
tannergooding commentedon Apr 16, 2019
I'll open a separate issue to track reverting the behavior to propagate the NaN.
[-]Behavior change for Math.Min and Math.Max[/-][+]Expose Math.MaxNumber and Math.MinNumber functions that don't propagate NaN[/+]tannergooding commentedon Apr 16, 2019
Opened: https://github.com/dotnet/corefx/issues/36931
GrabYourPitchforks commentedon Jan 7, 2020
Feedback: Should we have a wider discussion about putting future APIs on the primitive types (
double,half, etc.) instead of theMath/MathF/MathHclasses? Otherwise there's likely to be an explosion ofMath*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
Numbername 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 commentedon Jan 21, 2021
Closing in favor of #20137