Repository navigation
Return the exact floor from 3D and 4D Hypot for 32-bit and narrower integers [patch] - #538
Open
matt-edmondson wants to merge 1 commit into
Open
matt-edmondson wants to merge 1 commit into
matt-edmondson wants to merge 1 commit into
Conversation
…ntegers [patch] The byte, sbyte, short, ushort, int and uint storage types nested two double.Hypot calls and truncated the result. Each call rounds, so an exact length could come back one ulp low (58.99999999999999), and truncation then answered one too low on a perfect square: Position3D<int>(6, 41, 42).Length() returned 58 instead of 59, and (3, 3, 3, 3) gave 5 instead of 6. Those types now sum their squares in Int128, which holds four squares of any 32-bit component, and take the exact integer floor, as the wide integers already do. Every generated Length, Magnitude, Distance and DistanceTo on 2D, 3D and 4D vector forms goes through this path. Fixes #537 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017vfJW4YCCXpkCDCmjUcRT6
|
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 free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.



Fixes #537
What was wrong
For
byte,sbyte,short,ushort,intanduintstorage, the 3- and 4-componentStorageMath.Hypotoverloads nested twodouble.Hypotcalls and then truncated the result withCreateChecked. Each call rounds, so an exact length could come back one ulp low (58.99999999999999), and truncation then made it one too low on a perfect square:Position3D<int>.FromMeter(6, 41, 42).Length()returned 58, not 59new Acceleration4D<int> { X = 3, Y = 3, Z = 3, W = 3 }.Length()returned 5, not 6This was a regression from the #327 overflow fix.
The fix
A new
IsNarrowInteger<T>covers the 32-bit-and-narrower integer primitives (plusnint/nuinton a 32-bit process).HypotOfNarrowIntegerwidens the components toInt128, which holds the sum of four squares of any 32-bit value (at most 2^66), and takes the exact floor with the existingFloorSqrtOfWideInteger. This is the same approach the wide integers already use since #520. The 2-, 3- and 4-component overloads all use it, so the generatedLength,Magnitude,DistanceandDistanceTomethods are exact for these types. A length the type can't hold still throwsOverflowException, for exampleHypot(int.MinValue, 0, 0).float,doubleandHalfstill take thedouble.Hypotroute. The issue notes thatdoublehas the same one-ulp error on (6, 41, 42). That isn't changed here, because a correctly rounded floating point result is a separate decision. The wrap-around in generatedDistance()(QuantitiesGenerator.cs:1935) is also left for its own issue.Tests
NarrowIntegerHypotenuseIsTheExactFloor: the perfect-square cases from the issue forint,short,uintandbyte, plus the extremes and the overflows.NarrowIntegerHypotenuseMatchesTheIntegerRootEverywhere: the issue's brute force, run against an integer square root over every 3D vector with components 0..60 and every 4D vector with components 0..20.IntegerVectorLengthsOfPerfectSquaresAreExact:Length,MagnitudeandDistanceTochecked through the generated vector types.TheRootsArePublicAndTheirWorkingsAreNotnow also checks that the two new helpers stay private.With the fix reverted, all three new tests fail. With the fix, the full
dotnet testrun on net10.0 passes: 2043 passed, 0 failed, 14 skipped as Windows-only. The container has no older .NET runtime, so one test host could not launch.Generated by Claude Code