positive_f64: saturate arithmetic at MIN_POSITIVE, define inf/inf as 1 - #1038
Conversation
|
In 695c2f8: I'm glad that you tested the behavior of
I think we should just document that the result is wrong if you have multiple infinities. We never hit this case in our own usage, and I think the 'correct' way to implement it is to add a bunch of extra logic where we do an additional pass to count all the infinities, then yield |
| if f < f64::MIN_POSITIVE { | ||
| f64::MIN_POSITIVE | ||
| } else { | ||
| f | ||
| } |
There was a problem hiding this comment.
This looks like an f64::max operation, and it upholds the invariants here
| if f < f64::MIN_POSITIVE { | |
| f64::MIN_POSITIVE | |
| } else { | |
| f | |
| } | |
| f.max(f64::MIN_POSITIVE) |
There was a problem hiding this comment.
Agreed, though we need to keep the comment saying that f being NaN is impossible here. (Though if we did get a NaN, f64::max would turn it into f64::MIN_POSITIVE which I guess is an improvement.)
PositiveF64 guarantees that it holds a positive, non-NaN f64, but its arithmetic could escape this invariant at the extremes: inf / inf produced NaN (silently breaking the Eq contract), a finite value divided by inf produced 0.0, and repeated division (e.g. through deeply-nested-or policies) could underflow a probability to 0.0. Harden the arithmetic as described in issue rust-bitcoin#1033: - Multiplication and division results that would underflow to a subnormal number or to 0.0 now saturate at f64::MIN_POSITIVE, exposed as PositiveF64::MIN_POSITIVE. Once this value is reached, further division (or multiplication by values less than one) is a no-op. - inf / inf is defined to be 1.0, making NaN unreachable and the Eq implementation sound. - A finite value divided by inf underflows to 0.0 and therefore saturates at MIN_POSITIVE like any other underflow. - NormalizedIterator uses the same hardened division, closing the compiler-side underflow path. Document the invariant and saturation semantics on the type (replacing the stale "Ordered f64 for comparison" doc line left over from OrdF64), document that NormalizedIterator yields mathematically incorrect results when its input contains multiple infinities, and add targeted unit tests for the extreme-value behavior, including the reversed-iteration path the compiler uses. Fixes rust-bitcoin#1033
695c2f8 to
d0beebf
Compare
3 participants
PositiveF64 guarantees that it holds a positive, non-NaN f64, but its
arithmetic could escape this invariant at the extremes: inf / inf
produced NaN (silently breaking the Eq contract), a finite value
divided by inf produced 0.0, and repeated division (e.g. through
deeply-nested-or policies) could underflow a probability to 0.0.
Harden the arithmetic as described in issue #1033:
subnormal number or to 0.0 now saturate at f64::MIN_POSITIVE, exposed
as PositiveF64::MIN_POSITIVE. Once this value is reached, further
division (or multiplication by values less than one) is a no-op.
implementation sound.
saturates at MIN_POSITIVE like any other underflow.
compiler-side underflow path.
Document the invariant and saturation semantics on the type (replacing
the stale "Ordered f64 for comparison" doc line left over from OrdF64)
and add targeted unit tests for the extreme-value behavior, including
the reversed-iteration path the compiler uses.
Fixes #1033