Opened 4 years ago
Closed 4 years ago
#5501 closed bug (fixed)
randomR overflow
Reported by: | daniel.is.fischer | Owned by: | rrnewton |
---|---|---|---|
Priority: | normal | Milestone: | |
Component: | libraries/random | Version: | 7.2.1 |
Keywords: | Cc: | ||
Operating System: | Unknown/Multiple | Architecture: | Unknown/Multiple |
Type of failure: | Incorrect result at runtime | Test Case: | |
Blocked By: | Blocking: | ||
Related Tickets: | Differential Rev(s): | ||
Wiki Page: |
Description
When given a large range, randomR overflows at Double etc.
randomIvalDouble has two problems: first, the calculation of the center, (l+h)/2 overflows if the range is located near ±Infinity; second, and that concerns also randomRFloating, the scaling factor (h-l) overflows if the range is large enough.
Both problems can be fixed "well enough" by multiplying the bounds by 0.5 before the calculations and scaling up at the end,
0.5*l + 0.5*h instead of (l+h)/2 (0.5*h - 0.5*l)/(0.5*realToFrac int32Count) in randomIvalDouble 2.0*(0.5*l + coef*(0.5*h - 0.5*l)) in randomRFloating
These transformations can introduce a small error when a subnormal number is involved, but I think we can ignore that (no sane person would have a [nonzero] subnormal number as a bound, and a correct-for-all-cases transformation would be somewhat convoluted).
Change History (2)
comment:1 Changed 4 years ago by igloo
- Owner set to rrnewton
comment:2 Changed 4 years ago by rrnewton
- Resolution set to fixed
- Status changed from new to closed
Thanks! Fixes applied. Changes in random version 1.0.1.1.
It looks like there may have been some performance regression; more testing is needed. I don't think it's due to these changes, however. (Though they do add a few extra multiplies.)