Partially fix Decimal.ulp to return a valid representation. #3014
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.
Unfortunately,
Decimal.ulp
has never actually worked correctly. To make it short, users currently get an invalid representation of an incorrect result—The incorrect result: The implementation has never actually returned the "unit in the last place" as an IEEE floating-point type would. For instance,
print((0.1 as Decimal).ulp)
gives0.1
, but it's obvious that the type has enough precision to be able to represent values between0.1
and0.2
. This is somewhat forgivable, though, becauseFoundation.Decimal
isn't an IEEE decimal type.The invalid representation: The implementation constructs a
Decimal
value explicitly with_length
set to8
but using a mantissa of only length1
. This breaksFoundation.Decimal
invariants and is not normalized byNSDecimalNormalize
. Consequently, all sorts of weirdness happens when one tries to compare this invalid representation to valid ones. For instance,(0.1 as Decimal).ulp == 0.1
evaluates tofalse
, which is untenable because, as we just noted above,print((0.1 as Decimal).ulp)
gives0.1
.This PR defers addressing (1), since it should be done in concert with addressing the implementation of
nextUp
andnextDown
, and we'd need to consider whether that's a straightforward bugfix or whether it would break too much existing code that relies on the current behavior of these properties. What this PR does do is address (2) by changing the explicitly set_length
to1
; corresponding tests are added and adjusted as appropriate.