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

Expose the DegreesToRadians and RadiansToDegrees APIs #88866

Merged
merged 4 commits into from
Jul 14, 2023
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions src/libraries/System.Private.CoreLib/src/System/Double.cs
Original file line number Diff line number Diff line change
Expand Up @@ -1874,12 +1874,18 @@ public static double CosPi(double x)
/// <inheritdoc cref="ITrigonometricFunctions{TSelf}.DegreesToRadians(TSelf)" />
public static double DegreesToRadians(double degrees)
{
// NOTE: Don't change the algorithm without consulting the DIM
// which elaborates on why this implementation was chosen

return (degrees * Pi) / 180.0;
}

/// <inheritdoc cref="ITrigonometricFunctions{TSelf}.RadiansToDegrees(TSelf)" />
public static double RadiansToDegrees(double radians)
{
// NOTE: Don't change the algorithm without consulting the DIM
// which elaborates on why this implementation was chosen

return (radians * 180.0) / Pi;
}

Expand Down
16 changes: 14 additions & 2 deletions src/libraries/System.Private.CoreLib/src/System/Half.cs
Original file line number Diff line number Diff line change
Expand Up @@ -2147,10 +2147,22 @@ private static bool TryConvertTo<TOther>(Half value, [MaybeNullWhen(false)] out
public static Half CosPi(Half x) => (Half)float.CosPi((float)x);

/// <inheritdoc cref="ITrigonometricFunctions{TSelf}.DegreesToRadians(TSelf)" />
public static Half DegreesToRadians(Half degrees) => (Half)float.DegreesToRadians((float)degrees);
public static Half DegreesToRadians(Half degrees)
{
// NOTE: Don't change the algorithm without consulting the DIM
// which elaborates on why this implementation was chosen

return (Half)float.DegreesToRadians((float)degrees);
}

/// <inheritdoc cref="ITrigonometricFunctions{TSelf}.RadiansToDegrees(TSelf)" />
public static Half RadiansToDegrees(Half radians) => (Half)float.RadiansToDegrees((float)radians);
public static Half RadiansToDegrees(Half radians)
{
// NOTE: Don't change the algorithm without consulting the DIM
// which elaborates on why this implementation was chosen

return (Half)float.RadiansToDegrees((float)radians);
}

/// <inheritdoc cref="ITrigonometricFunctions{TSelf}.Sin(TSelf)" />
public static Half Sin(Half x) => (Half)MathF.Sin((float)x);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,54 @@ public interface ITrigonometricFunctions<TSelf>
/// <returns>The value of <paramref name="degrees" /> converted to radians.</returns>
static virtual TSelf DegreesToRadians(TSelf degrees)
{
// We don't want to simplify this to: degrees * (Pi / 180)
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added a lengthy comment to this API here and pointed the other implementations to it to help avoid people trying to change the algorithm without considering the edge cases.

//
// Doing so will change the result and in many cases will
// cause a loss of precision. The main exception to this
// is when the initial multiplication causes overflow, but
// if we decide that should be handled in the future it needs
// to be more explicit around how its done.
//
// Floating-point operations are natural imprecise due to
tannergooding marked this conversation as resolved.
Show resolved Hide resolved
// rounding required to fit the "infinitely-precise result"
// into the limits of the underlying representation. Because
// of this, every operation can introduce some amount of rounding
// error.
//
// For integers, the IEEE 754 binary floating-point types can
// exactly represent them up to the 2^n, where n is the number
// of bits in the significand. This is 10 for Half, 23 for Single,
// and 52 for Double. As you approach this limit, the number of
// digits available to represent the fractional portion decreases.
//
// For Half, you get around 3.311 total decimal digits of precision.
// For Single, this is around 7.225 and around 15.955 for Double.
//
// The actual number of digits can be slightly more or less, depending.
//
// This means that values such as `Pi` are not exactly `Pi`, instead:
// * Half: 3.14 0625
// * Single: 3.14 1592 741012573 2421875
// * Double: 3.14 1592 653589793 115997963468544185161590576171875
// * Actual: 3.14 1592 653589793 2384626433832795028841971693993751058209749445923...
//
// If we were to simplify this to simply multiply by (Pi / 180), we get:
// * Half: 0.01745 6054 6875
// * Single: 0.01745 3292 384743690 49072265625
// * Double: 0.01745 3292 519943295 4743716805978692718781530857086181640625
// * Actual: 0.01745 3292 519943295 7692369076848861271344287188854172545609719144...
//
// Neither of these end up "perfect". There will be some cases where they will trade
// in terms of closeness to the "infinitely precise result". Over the entire domain
// however, doing the separate multiplications tends to produce overall more accurate
// results. It helps ensure the implementation can be trivial for the DIM case, and
// covers the vast majority of typical inputs more efficiently largely only pessimizing
// the case where the first multiplication results in overflow.
//
// This is particularly true for `RadiansToDegrees` where 180 is exactly representable
// and so allows an exactly representable intermediate value to be computed when overflow
// doesn't occur.

return (degrees * TSelf.Pi) / TSelf.CreateChecked(180);
}

Expand All @@ -70,6 +118,9 @@ static virtual TSelf DegreesToRadians(TSelf degrees)
/// <returns>The value of <paramref name="radians" /> converted to degrees.</returns>
static virtual TSelf RadiansToDegrees(TSelf radians)
{
// We don't want to simplify this to: radians * (180 / Pi)
// See DegreesToRadians for a longer explanation as to why

return (radians * TSelf.CreateChecked(180)) / TSelf.Pi;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1852,10 +1852,22 @@ private static bool TryConvertTo<TOther>(NFloat value, [MaybeNullWhen(false)] ou
public static NFloat CosPi(NFloat x) => new NFloat(NativeType.CosPi(x._value));

/// <inheritdoc cref="ITrigonometricFunctions{TSelf}.DegreesToRadians(TSelf)" />
public static NFloat DegreesToRadians(NFloat degrees) => new NFloat(NativeType.DegreesToRadians(degrees._value));
public static NFloat DegreesToRadians(NFloat degrees)
{
// NOTE: Don't change the algorithm without consulting the DIM
// which elaborates on why this implementation was chosen

return new NFloat(NativeType.DegreesToRadians(degrees._value));
}

/// <inheritdoc cref="ITrigonometricFunctions{TSelf}.RadiansToDegrees(TSelf)" />
public static NFloat RadiansToDegrees(NFloat radians) => new NFloat(NativeType.RadiansToDegrees(radians._value));
public static NFloat RadiansToDegrees(NFloat radians)
{
// NOTE: Don't change the algorithm without consulting the DIM
// which elaborates on why this implementation was chosen

return new NFloat(NativeType.RadiansToDegrees(radians._value));
}

/// <inheritdoc cref="ITrigonometricFunctions{TSelf}.Sin(TSelf)" />
public static NFloat Sin(NFloat x) => new NFloat(NativeType.Sin(x._value));
Expand Down
6 changes: 6 additions & 0 deletions src/libraries/System.Private.CoreLib/src/System/Single.cs
Original file line number Diff line number Diff line change
Expand Up @@ -1755,12 +1755,18 @@ public static float CosPi(float x)
/// <inheritdoc cref="ITrigonometricFunctions{TSelf}.DegreesToRadians(TSelf)" />
public static float DegreesToRadians(float degrees)
{
// NOTE: Don't change the algorithm without consulting the DIM
// which elaborates on why this implementation was chosen

return (degrees * Pi) / 180.0f;
}

/// <inheritdoc cref="ITrigonometricFunctions{TSelf}.RadiansToDegrees(TSelf)" />
public static float RadiansToDegrees(float radians)
{
// NOTE: Don't change the algorithm without consulting the DIM
// which elaborates on why this implementation was chosen

return (radians * 180.0f) / Pi;
}

Expand Down