Was VectorN.Clamp breaking change intended?

[koszeggy]

Starting with .NET 5.0 the behavior of Vector2/3/4.Clamp changed. Consider the following code:

var v = new Vector4(Single.NaN);
Console.WriteLine(Vector4.Clamp(v, Vector4.Zero, Vector4.One));
Console.WriteLine(Vector4.Min(Vector4.Max(v, Vector4.Zero), Vector4.One));

Between .NET Framework 4.6 and .NET Core 3.1 this code prints the following:

<NaN, NaN, NaN, NaN>
<0, 0, 0, 0>

But starting with .NET 5 both rows are zeros.

In the sources I can see that up to .NET Core 3.1 it worked like Math.Min/Max, preserving NaN values, and above it turned into an intrinsic method where the fallback implementation calls Vector.Min/Max (which btw are intrinsics as well).

In fact I prefer the new behavior, even the documentation feels more correct when saying:

Restricts a vector between a minimum and a maximum value.

However, I don't see anything about this change in the docs and I'm not sure whether I can build on the current behavior. Is it something that depends on the actual intrinsics injected for the current CPU if possible, or is it guaranteed that now Clamp always returns values between the specified range?

[tannergooding]

Vector*.Clamp has actually always been "non-deterministic" on x86/x64 because Min and Max are presently non-deterministic.

Min compiles down to minps which does not correctly account for -0.0 or NaN, much as Max compiles down to maxps and has the same consideration. In particular, these instructions always return y (the second operand) if both values are zero or if either is a NaN.

Thus, on x86/x64, Min(NaN, 1) returns 1. While Min(1, NaN) returns NaN. Likewise, Min(-0.0, +0.0) returns +0.0 while Min(+0.0, -0.0) returns -0.0.

The difference here is one that moves closer towards determinsm by not allowing the operation to be considered commutative.

---

A "correct" implementation would be one where we provide:

• Min, Max, Clamp - These propagate NaN and treat -0.0 as less than +0.0
• MinNumber, MaxNumber, ClampNumber - These propagate a number and treat -0.0 as less than +0.0
• MinUnsafe, MaxUnsafe, ClampUnsafe - These are non-deterministic for NaN and -0.0, they do whatever is the "fast" behavior

This would represent a non-trivial perf impact to Min/Max/Clamp by default.

[tannergooding]

We probably should update the documentation to call this consideration out.

We should likely also take a "proper" break here and do the correct thing to mirror System.Math and System.MathF

[koszeggy]

We should likely also take a "proper" break here and do the correct thing to mirror System.Math and System.MathF

If so, I definitely support the idea of MinNumber/MaxNumber/ClampNumber

Is there a vectorized performant way of implementing ClampNumber today or should I go for the vanilla Single.IsNaN checks?