Fix nearby_rational max_error coercion #40280
Open
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.
Previously the coercion model makes it so that
self - max_error
is coerced to the common parent (minimum precision) ofself
andmax_error
, which is often undesirable. See the newly-added test innearby_rational
(previously it returns False)
Also implement
add_error
, mirroring the method with same name inRBF
. See also #39846I decide to delete one test (originally added in #6616 ) because: the value
RR(2.1)
is exactly represented internally byand as shown later,
4728779608739021/2251799813685248 - 21/10
is in fact less than10e-17
, so the original result is incorrect, just as an artifact of hownearby_rational
were implemented.This new implementation, however, might make it such that the returned value is not within the interval. However this is not particularly easy to handle (we may use X extra bits of precision in the interval, but we will not know which X is sufficient), and it may not be what the user want anyway (the original input might its contain error of the order of Θ(1) ulp)
📝 Checklist
⌛ Dependencies