Re: RFR: 8284493: Fix rounding error in computeNextExponential [v2]
On Thu, 2 Jun 2022 03:00:32 GMT, Chris Hennick wrote: >> Chris Hennick has refreshed the contents of this pull request, and previous >> commits have been removed. The incremental views will show differences >> compared to the previous content of the PR. The pull request contains one >> new commit since the last revision: >> >> Fix rounding error in computeNextExponential; use FMA >> >> Repeatedly adding DoubleZigguratTables.exponentialX0 to extra causes a >> rounding error to accumulate at the tail of the distribution; this fixes >> that by tracking the multiple of exponentialX0 as a long. >> >> Add computeWinsorizedNextExponential for testability > > In addition to the changes discussed heretofore, I've also changed line 1382 > to eliminate unneeded tail exploration; this should make `nextGaussian` > faster at high percentiles (probably measurable at 99%ile; should definitely > be measurable at at 99.99%ile). @Pr0methean Please do not rebase or force-push to an active PR as it invalidates existing review comments. All changes will be squashed into a single commit automatically when integrating. See [OpenJDK Developers’ Guide](https://openjdk.java.net/guide/#working-with-pull-requests) for more information. - PR: https://git.openjdk.java.net/jdk/pull/8131
Re: RFR: 8284493: Fix rounding error in computeNextExponential [v2]
On Wed, 1 Jun 2022 22:49:01 GMT, Chris Hennick wrote: >> @Pr0methean As @turbanoff observes, I think there's a closing parentheses >> too much on L.1411 and one on L.1421. >> Which compiler are you using ;-) ? > > Should be fixed as of fa340fb47b1169805f21fb71cce0da3fc175d427. I haven't yet > found a way for IntelliJ IDEA to use the project as its own JDK, so I've > sanity-checked the syntax using the Rainbow Brackets plugin. Update: successfully built `images` and `run-test-tier1` on my AL2 machine. - PR: https://git.openjdk.java.net/jdk/pull/8131
Re: RFR: 8284493: Fix rounding error in computeNextExponential [v2]
> Repeatedly adding DoubleZigguratTables.exponentialX0 to extra causes a > rounding error to accumulate at the tail of the distribution (probably > starting around 2*exponentialX0 == 0x1.e46eff20739afp3 ~ 15.1); this fixes > that by tracking the multiple of exponentialX0 as a long. (This changes the > maximum possible output to `1.0p63 * DoubleZigguratTables.exponentialX0 == > 0x1.e46eff20739afp65`; previously it would have been `0x1.0p56` because once > `extra` reaches that amount, `x + extra == extra` due to the rounding error. > This lowers the probability of reaching the maximum with an ideal PRNG from > about `1.3877787807814488E-17` to about `1.4323726067488646E-20` (calculated > using the identity `ln(x) == Math.log10(x)/Math.log10(Math.exp(1))`). Chris Hennick has refreshed the contents of this pull request, and previous commits have been removed. The incremental views will show differences compared to the previous content of the PR. The pull request contains one new commit since the last revision: Fix rounding error in computeNextExponential; use FMA Repeatedly adding DoubleZigguratTables.exponentialX0 to extra causes a rounding error to accumulate at the tail of the distribution; this fixes that by tracking the multiple of exponentialX0 as a long. Add computeWinsorizedNextExponential for testability - Changes: - all: https://git.openjdk.java.net/jdk/pull/8131/files - new: https://git.openjdk.java.net/jdk/pull/8131/files/1fd959cc..fa340fb4 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=8131&range=01 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=8131&range=00-01 Stats: 5 lines in 1 file changed: 2 ins; 0 del; 3 mod Patch: https://git.openjdk.java.net/jdk/pull/8131.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/8131/head:pull/8131 PR: https://git.openjdk.java.net/jdk/pull/8131
Re: RFR: 8284493: Fix rounding error in computeNextExponential [v2]
On Mon, 23 May 2022 14:21:55 GMT, Raffaello Giulietti wrote: >> src/java.base/share/classes/jdk/internal/util/random/RandomSupport.java line >> 1411: >> >>> 1409: long U2 = (rng.nextLong() >>> 1); >>> 1410: // Compute the actual x-coordinate of the randomly >>> chosen point. >>> 1411: x = Math.fma(X[j-1] - X[j], (double)U1, X[j] * >>> 0x1.0p63)); >> >> Code is not compilable > > @Pr0methean As @turbanoff observes, I think there's a closing parentheses too > much on L.1411 and one on L.1421. > Which compiler are you using ;-) ? Should be fixed as of fa340fb47b1169805f21fb71cce0da3fc175d427. I haven't yet found a way for IntelliJ IDEA to use the project as its own JDK, so I've sanity-checked the syntax using the Rainbow Brackets plugin. - PR: https://git.openjdk.java.net/jdk/pull/8131
