Uh oh!
There was an error while loading. Please reload this page.
[release/11.0] Make DegreesToRadians and RadiansToDegrees correctly rounded - #132578
Merged
Merged
Conversation
`x * (pi / 180)` rounds twice -- once forming the constant, once forming the product -- leaving up to ~1 ulp of error. Over all `2^31` finite positive `float` inputs the current shape misrounds 37.9% of them, and 0.042% are off by 2 ulp. Carry the factor as a multi-limb value and fold the limbs back with a single rounding. Two limbs suffice for all but a rare input, and whether they did is itself decidable, so the third limb is only paid for when needed; a scaled path covers the subnormals. Covers `float`, `double`, `Half`, `BFloat16`, `Vector64/128/256/512`, `Vector<T>`, and `Decimal32/64/128`. Fixes#131930 ---------- Validated against exact rational arithmetic: exhaustively for `Half`, `BFloat16`, `float` (all `2^31`) and `Decimal32` (60M); for `double` and the larger decimals against an enumerated near-tie set (372k vectors) plus replay of the whole subnormal band. 0 misrounded in every case. ---------- Cost in ns per operation, over a 4096 element `double` array storing to a destination, so a `Vector256` entry is one call handling four elements. Ad hoc harness on a 7950X, and both directions measure the same within noise. | | before | after | |---|---|---| | scalar | 0.90 | 1.02 | | `Vector128` | 0.88 | 1.81 | | `Vector256` | 0.85 | 1.92 | | `Vector512` | 1.65 | 3.00 | The old code was bound by `vdivsd` / `vdivpd` throughput rather than by the multiply, which is why it cost about the same at every width. The new code has no divide and is FP issue bound instead, so it scales with the work rather than with the divider. `Vector512` is double pumped on this part, hence roughly twice `Vector256` in both columns. Inputs below the point where the two limb form stops working take a scaled path, at roughly 13 ns. ---------- - The vector paths use `Vector*.FusedMultiplyAdd` where the hardware has it and a Veltkamp split otherwise, since recovering the roundoff of a product requires a real fused multiply-add. - Decimal zero now carries the IEEE preferred exponent: `Decimal32.DegreesToRadians(0E0)` is `0E-17`, was `0E-8`. - The six new `Decimal*` members mirror the existing `double` / `float` `ITrigonometricFunctions` surface. > [!NOTE] > Portions of this pull request were drafted with Copilot. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
|
Azure Pipelines: Successfully started running 3 pipeline(s). 13 pipeline(s) were filtered out due to trigger conditions. There may be pipelines that require an authorized user to comment /azp run to run. |
Contributor
Tagging subscribers to this area: @dotnet/area-system-numerics |
jeffhandley
approved these changes
Aug 20, 2026
jeffhandley
left a comment
Member
There was a problem hiding this comment.
This has my support for .NET 11 RC2
artl93
approved these changes
Aug 20, 2026
artl93
left a comment
Member
There was a problem hiding this comment.
We Should Do Math Right. Approved.
tannergooding
commented
Aug 21, 2026
Member
/ba-g httplistener timeouts |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Backport of #132096 to release/11.0
/cc @tannergooding
Customer Impact
Customers can receive infinity when converting a finite
floatordoublevalue if the intermediate multiplication overflows, even though the correctly rounded result is finite. For non-overflowing inputs, the existing implementations are also frequently 1 ULP inaccurate and can be 2 ULP inaccurate. This was reported in #131930.Regression
This corrects the original implementation of these APIs rather than a regression introduced in the current release.
Testing
The implementation was validated against exact rational arithmetic: exhaustively for
Half,BFloat16,float(all2^31finite positive inputs), andDecimal32(60 million inputs). The more complexdoubleimplementation was validated using mathematically derived rounding boundaries, including 372,000 enumerated near-tie vectors and the complete subnormal band.Decimal64andDecimal128were validated against the same near-tie and subnormal coverage. All cases were correctly rounded.Unit tests cover scalar floating-point and decimal conversions, including special values, overflow boundaries, subnormals, and known near-tie cases. The vector implementations use the same validated algorithm.
Risk
Low. Every possible input was exhaustively validated for the smaller binary formats, and
Decimal32was exhaustively validated across 60 million inputs. The more complexdoublepath was validated against exact rational arithmetic using mathematically derived boundary and near-tie cases plus the complete subnormal range. The decimal implementations use exact integer arithmetic, and the vector paths reuse the corresponding validated scalar algorithm.Note
This PR description was drafted by GitHub Copilot on behalf of the author.