Re: RFR: 8326227: Rounding error that may distort computeNextGaussian results [v3]

[Chris Hennick]

On Wed, 12 Jun 2024 09:35:12 GMT, Jaikiran Pai  wrote:

>> Chris Hennick has updated the pull request incrementally with one additional 
>> commit since the last revision:
>> 
>>   Bug fix: add-exports was for wrong package
>
> test/jdk/jdk/internal/util/random/RandomSupportTest.java line 32:
> 
>> 30: for (double max = 1.0; max < 10.0; max++) {
>> 31: WorstCaseRandomGenerator rng = new 
>> WorstCaseRandomGenerator();
>> 32: 
>> assertTrue(RandomSupport.computeNextExponentialSoftCapped(rng, max) >= max);
> 
> Would you mind changing this to something like:
> 
> double val = RandomSupport.computeNextExponentialSoftCapped(rng, max);
> System.out.println("got " + val + " for max " + max);
> assertTrue(val >= max, val + " isn't >= " + max);
> 
> 
> That way if this test fails for any reason, then we get the necessary details 
> on what the computed value is.

Done.

-

PR Review Comment: https://git.openjdk.org/jdk/pull/17703#discussion_r1712323293

Re: RFR: 8326227: Rounding error that may distort computeNextGaussian results [v3]

[Chris Hennick]

On Wed, 12 Jun 2024 08:16:25 GMT, Jaikiran Pai  wrote:

>> Chris Hennick has updated the pull request incrementally with one additional 
>> commit since the last revision:
>> 
>>   Bug fix: add-exports was for wrong package
>
> src/java.base/share/classes/jdk/internal/util/random/RandomSupport.java line 
> 1214:
> 
>> 1212: // We didn't use the upper part of U1 after all.  We'll 
>> probably be able to use it later.
>> 1213: if (maxValue <= DoubleZigguratTables.exponentialX0) {
>> 1214: return DoubleZigguratTables.exponentialX0;
> 
> Chris, could you explain why this change to this if block is necessary? Guy 
> notes that (I've paraphrased):
> 
>> I can see that (without this proposed change here), if `maxValue` is not 
>> greater than `DoubleZigguratTables.exponentialX0`, then the subsequent 
>> computation for `maxExtraMinus1`, a few lines below using `Math.round()`, 
>> will compute 1 or 2 for the value of `maxExtraMinus1`. But with this 
>> proposed change, it just returns `DoubleZigguratTables.exponentialX0`, which 
>> by the contract of `computeNextExponentialSoftCapped` is okay (the doc says 
>> "{@code maxValue} is a "soft" cap in that a value larger than {@code 
>> maxValue} may be returned in order to save a calculation" and I remember 
>> that that is true). So that is also okay. The motivation for doing so would 
>> be that it saves computation time overall. The part I don't quite understand 
>> yet is the judgment that it will in fact save computation time overall. If 
>> you take advantage of the "soft cap" then it could cause additional 
>> iteration of the loop in `computeNextGaussian` where this 
>> `computeNextExponentialSoftCapped` method gets called. I'm 
 uncertain whether the change to this if block is guaranteed or likely to save 
computation time.

Updated to return `maxValue` instead. Now it's unambiguously a performance 
optimization.

> src/java.base/share/classes/jdk/internal/util/random/RandomSupport.java line 
> 1222:
> 
>> 1220: // Math.round rounds toward infinity in conversion to 
>> long; adding 1.0 corrects for any
>> 1221: // downward rounding error in the division
>> 1222: maxExtraMinus1 = Math.round(1.0 + maxValue / 
>> DoubleZigguratTables.exponentialX0);
> 
> We had asked Guy Steele for his inputs on these proposed changes. Guy 
> reviewed these changes and for this specific if/else block change, he notes 
> that (I've paraphrased it):
> 
>> the (proposed new) computation of `maxExtraMinus1` looks okay to me: it’s 
>> always okay for maxExtraMinus1 to be a bit larger than you might expect; the 
>> only downside is that the (for) loop starting (on the next line) might take 
>> extra iterations.

Is Guy saying it's impossible for `maxExtraMinus1` to be too small even without 
this change?

-

PR Review Comment: https://git.openjdk.org/jdk/pull/17703#discussion_r1712306048
PR Review Comment: https://git.openjdk.org/jdk/pull/17703#discussion_r1712308075

Re: RFR: 8326227: Rounding error that may distort computeNextGaussian results [v3]

[Chris Hennick]

On Wed, 12 Jun 2024 09:23:23 GMT, Jaikiran Pai  wrote:

>> Chris Hennick has updated the pull request incrementally with one additional 
>> commit since the last revision:
>> 
>>   Bug fix: add-exports was for wrong package
>
> test/jdk/jdk/internal/util/random/RandomSupportTest.java line 5:
> 
>> 3:  * @bug 8326227
>> 4:  * @modules java.base/jdk.internal.util.random
>> 5:  * @summary Verify that RandomSupport methods behave as specified
> 
> Nit - can you move this `@summary` line after the `@bug` line? It would then 
> match the recommended tag order https://openjdk.org/jtreg/tag-spec.html#ORDER

Done.

-

PR Review Comment: https://git.openjdk.org/jdk/pull/17703#discussion_r1712018244

Re: RFR: 8326227: Rounding error that may distort computeNextGaussian results [v3]

[Chris Hennick]

On Wed, 12 Jun 2024 10:08:02 GMT, Jaikiran Pai  wrote:

>>> Update: confirmed that the new test fails without the fix.
>> 
>> Thanks for verifying the test checks the fix; I'll let others who have 
>> worked more directly on the random code review the actual fix.
>
>> @jddarcy Added a regression test, and currently working on adjusting it (see 
>> https://github.com/Pr0methean/jdk/actions/runs/798127) to ensure we have 
>> a case that fails without the fix, passes with the fix, and is practical to 
>> run within the usual unit-test timeouts.
> 
> I gave this a try locally. It doesn't fail for me without the source code 
> changes proposed in this PR. I see the following output from the test without 
> the source code changes:
> 
> 
> got 1.0 for max 1.0
> got 2.0 for max 2.0
> got 3.0 for max 3.0
> got 4.0 for max 4.0
> got 5.0 for max 5.0
> got 6.0 for max 6.0
> got 7.0 for max 7.0
> got 11.353912041222094 for max 8.0
> got 11.353912041222094 for max 9.0
> 
> 
> With the proposed changes in this PR, the test continues to pass and I see 
> this output:
> 
> 
> got 7.569274694148063 for max 1.0
> got 7.569274694148063 for max 2.0
> got 7.569274694148063 for max 3.0
> got 7.569274694148063 for max 4.0
> got 7.569274694148063 for max 5.0
> got 7.569274694148063 for max 6.0
> got 7.569274694148063 for max 7.0
> got 11.353912041222094 for max 8.0
> got 11.353912041222094 for max 9.0

@jaikiran Thanks for these results; I'll take another look at the test over the 
weekend.

-

PR Comment: https://git.openjdk.org/jdk/pull/17703#issuecomment-2278574602

Re: RFR: 8326227: Rounding error that may distort computeNextGaussian results [v3]

[Jaikiran Pai]

On Thu, 22 Feb 2024 05:40:10 GMT, Joe Darcy  wrote:

>> Update: confirmed that the new test fails without the fix.
>
>> Update: confirmed that the new test fails without the fix.
> 
> Thanks for verifying the test checks the fix; I'll let others who have worked 
> more directly on the random code review the actual fix.

> @jddarcy Added a regression test, and currently working on adjusting it (see 
> https://github.com/Pr0methean/jdk/actions/runs/798127) to ensure we have 
> a case that fails without the fix, passes with the fix, and is practical to 
> run within the usual unit-test timeouts.

I gave this a try locally. It doesn't fail for me without the source code 
changes proposed in this PR. I see the following output from the test without 
the source code changes:


got 1.0 for max 1.0
got 2.0 for max 2.0
got 3.0 for max 3.0
got 4.0 for max 4.0
got 5.0 for max 5.0
got 6.0 for max 6.0
got 7.0 for max 7.0
got 11.353912041222094 for max 8.0
got 11.353912041222094 for max 9.0


With the proposed changes in this PR, the test continues to pass and I see this 
output:


got 7.569274694148063 for max 1.0
got 7.569274694148063 for max 2.0
got 7.569274694148063 for max 3.0
got 7.569274694148063 for max 4.0
got 7.569274694148063 for max 5.0
got 7.569274694148063 for max 6.0
got 7.569274694148063 for max 7.0
got 11.353912041222094 for max 8.0
got 11.353912041222094 for max 9.0

-

PR Comment: https://git.openjdk.org/jdk/pull/17703#issuecomment-2162623391

Re: RFR: 8326227: Rounding error that may distort computeNextGaussian results [v3]

[Jaikiran Pai]

On Wed, 21 Feb 2024 02:18:08 GMT, Chris Hennick  wrote:

>> This provides a slightly more accurate bounding limit for 
>> `computeNextExponentialSoftCapped` when calling it from 
>> `computeNextGaussian`. This could cause the `while 
>> (computeNextExponentialSoftCapped(rng, limit) < limit)` check in 
>> `computeNextGaussian` on line 1402 to always be true, making the 
>> `nextGaussian` runtime unbounded in the worst case; but more likely, it 
>> would give a result that was truncated too close to zero.
>> 
>> This change is being tested prior to submission to OpenJDK by 
>> https://github.com/openjdk/jdk/pull/17703/commits/b8be051cbf40a6a05fafc6a2c76942e9e0b11fdf.
>
> Chris Hennick has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Bug fix: add-exports was for wrong package

test/jdk/jdk/internal/util/random/RandomSupportTest.java line 32:

> 30: for (double max = 1.0; max < 10.0; max++) {
> 31: WorstCaseRandomGenerator rng = new WorstCaseRandomGenerator();
> 32: 
> assertTrue(RandomSupport.computeNextExponentialSoftCapped(rng, max) >= max);

Would you mind changing this to something like:

double val = RandomSupport.computeNextExponentialSoftCapped(rng, max);
assertTrue(val >= max, val + " isn't >= " + max);


That way if this test fails for any reason, then we get the necessary details 
on what the computed value is.

-

PR Review Comment: https://git.openjdk.org/jdk/pull/17703#discussion_r1636139162

Re: RFR: 8326227: Rounding error that may distort computeNextGaussian results [v3]

[Jaikiran Pai]

On Wed, 21 Feb 2024 02:18:08 GMT, Chris Hennick  wrote:

>> This provides a slightly more accurate bounding limit for 
>> `computeNextExponentialSoftCapped` when calling it from 
>> `computeNextGaussian`. This could cause the `while 
>> (computeNextExponentialSoftCapped(rng, limit) < limit)` check in 
>> `computeNextGaussian` on line 1402 to always be true, making the 
>> `nextGaussian` runtime unbounded in the worst case; but more likely, it 
>> would give a result that was truncated too close to zero.
>> 
>> This change is being tested prior to submission to OpenJDK by 
>> https://github.com/openjdk/jdk/pull/17703/commits/b8be051cbf40a6a05fafc6a2c76942e9e0b11fdf.
>
> Chris Hennick has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Bug fix: add-exports was for wrong package

test/jdk/jdk/internal/util/random/RandomSupportTest.java line 5:

> 3:  * @bug 8326227
> 4:  * @modules java.base/jdk.internal.util.random
> 5:  * @summary Verify that RandomSupport methods behave as specified

Nit - can you move this `@summary` line after the `@bug` line? It would then 
match the recommended tag order https://openjdk.org/jtreg/tag-spec.html#ORDER

-

PR Review Comment: https://git.openjdk.org/jdk/pull/17703#discussion_r1636122002

Re: RFR: 8326227: Rounding error that may distort computeNextGaussian results [v3]

[Jaikiran Pai]

On Wed, 21 Feb 2024 02:18:08 GMT, Chris Hennick  wrote:

>> This provides a slightly more accurate bounding limit for 
>> `computeNextExponentialSoftCapped` when calling it from 
>> `computeNextGaussian`. This could cause the `while 
>> (computeNextExponentialSoftCapped(rng, limit) < limit)` check in 
>> `computeNextGaussian` on line 1402 to always be true, making the 
>> `nextGaussian` runtime unbounded in the worst case; but more likely, it 
>> would give a result that was truncated too close to zero.
>> 
>> This change is being tested prior to submission to OpenJDK by 
>> https://github.com/openjdk/jdk/pull/17703/commits/b8be051cbf40a6a05fafc6a2c76942e9e0b11fdf.
>
> Chris Hennick has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Bug fix: add-exports was for wrong package

src/java.base/share/classes/jdk/internal/util/random/RandomSupport.java line 
1214:

> 1212: // We didn't use the upper part of U1 after all.  We'll 
> probably be able to use it later.
> 1213: if (maxValue <= DoubleZigguratTables.exponentialX0) {
> 1214: return DoubleZigguratTables.exponentialX0;

Chris, could you explain why this change to this if block is necessary? Guy 
notes that (I've paraphrased):

> I can see that (without this proposed change here), if `maxValue` is not 
> greater than `DoubleZigguratTables.exponentialX0`, then the subsequent 
> computation for `maxExtraMinus1`, a few lines below using `Math.round()`, 
> will compute 1 or 2 for the value of `maxExtraMinus1`. But with this proposed 
> change, it just returns `DoubleZigguratTables.exponentialX0`, which by the 
> contract of `computeNextExponentialSoftCapped` is okay (the doc says "{@code 
> maxValue} is a "soft" cap in that a value larger than {@code maxValue} may be 
> returned in order to save a calculation" and I remember that that is true). 
> So that is also okay. The motivation for doing so would be that it saves 
> computation time overall. The part I don't quite understand yet is the 
> judgment that it will in fact save computation time overall. If you take 
> advantage of the "soft cap" then it could cause additional iteration of the 
> loop in `computeNextGaussian` where this `computeNextExponentialSoftCapped` 
> method gets called. I'm u
 ncertain whether the change to this if block is guaranteed or likely to save 
computation time.

-

PR Review Comment: https://git.openjdk.org/jdk/pull/17703#discussion_r1636024966

Re: RFR: 8326227: Rounding error that may distort computeNextGaussian results [v3]

[Jaikiran Pai]

On Wed, 21 Feb 2024 02:18:08 GMT, Chris Hennick  wrote:

>> This provides a slightly more accurate bounding limit for 
>> `computeNextExponentialSoftCapped` when calling it from 
>> `computeNextGaussian`. This could cause the `while 
>> (computeNextExponentialSoftCapped(rng, limit) < limit)` check in 
>> `computeNextGaussian` on line 1402 to always be true, making the 
>> `nextGaussian` runtime unbounded in the worst case; but more likely, it 
>> would give a result that was truncated too close to zero.
>> 
>> This change is being tested prior to submission to OpenJDK by 
>> https://github.com/openjdk/jdk/pull/17703/commits/b8be051cbf40a6a05fafc6a2c76942e9e0b11fdf.
>
> Chris Hennick has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Bug fix: add-exports was for wrong package

src/java.base/share/classes/jdk/internal/util/random/RandomSupport.java line 
1222:

> 1220: // Math.round rounds toward infinity in conversion to long; 
> adding 1.0 corrects for any
> 1221: // downward rounding error in the division
> 1222: maxExtraMinus1 = Math.round(1.0 + maxValue / 
> DoubleZigguratTables.exponentialX0);

We had asked Guy Steele for his inputs on these proposed changes. Guy reviewed 
these changes and for this specific if/else block change, he notes that (I've 
paraphrased it):

> the (proposed new) computation of `maxExtraMinus1` looks okay to me: it’s 
> always okay for maxExtraMinus1 to be a bit larger than you might expect; the 
> only downside is that the (for) loop starting (on the next line) might take 
> extra iterations.

-

PR Review Comment: https://git.openjdk.org/jdk/pull/17703#discussion_r1636013052

Re: RFR: 8326227: Rounding error that may distort computeNextGaussian results [v3]

[Jaikiran Pai]

On Wed, 21 Feb 2024 02:18:08 GMT, Chris Hennick  wrote:

>> This provides a slightly more accurate bounding limit for 
>> `computeNextExponentialSoftCapped` when calling it from 
>> `computeNextGaussian`. This could cause the `while 
>> (computeNextExponentialSoftCapped(rng, limit) < limit)` check in 
>> `computeNextGaussian` on line 1402 to always be true, making the 
>> `nextGaussian` runtime unbounded in the worst case; but more likely, it 
>> would give a result that was truncated too close to zero.
>> 
>> This change is being tested prior to submission to OpenJDK by 
>> https://github.com/openjdk/jdk/pull/17703/commits/b8be051cbf40a6a05fafc6a2c76942e9e0b11fdf.
>
> Chris Hennick has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Bug fix: add-exports was for wrong package

Hello Chris, I just noticed that there have been (unrelated) changes in this 
file in the master. Could you please merge the latest master branch changes 
into this PR?

-

PR Comment: https://git.openjdk.org/jdk/pull/17703#issuecomment-2162345189

Re: RFR: 8326227: Rounding error that may distort computeNextGaussian results [v3]

[Chris Hennick]

On Wed, 21 Feb 2024 02:18:08 GMT, Chris Hennick  wrote:

>> This provides a slightly more accurate bounding limit for 
>> `computeNextExponentialSoftCapped` when calling it from 
>> `computeNextGaussian`. This could cause the `while 
>> (computeNextExponentialSoftCapped(rng, limit) < limit)` check in 
>> `computeNextGaussian` on line 1402 to always be true, making the 
>> `nextGaussian` runtime unbounded in the worst case; but more likely, it 
>> would give a result that was truncated too close to zero.
>> 
>> This change is being tested prior to submission to OpenJDK by 
>> https://github.com/openjdk/jdk/pull/17703/commits/b8be051cbf40a6a05fafc6a2c76942e9e0b11fdf.
>
> Chris Hennick has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Bug fix: add-exports was for wrong package

keep open

-

PR Comment: https://git.openjdk.org/jdk/pull/17703#issuecomment-2153184927

Re: RFR: 8326227: Rounding error that may distort computeNextGaussian results [v3]

[Jaikiran Pai]

On Thu, 9 May 2024 03:55:15 GMT, Chris Hennick  wrote:

>> Chris Hennick has updated the pull request incrementally with one additional 
>> commit since the last revision:
>> 
>>   Bug fix: add-exports was for wrong package
>
> Keep open. 
> 
> @JimLaskey it looks like you're the author of most of the surrounding code; 
> can you please confirm that there's a rounding error in the existing 
> implementation and that this PR will fix it? Or if the `git blame` logs are 
> giving you more credit than you're willing to take, then could you please 
> assign it to one ot the real authors or, if that's not possible, to someone 
> else who's in a relevant role and can capable of reviewing and eventually 
> merging this PR?

Hello Chris @Pr0methean, please keep this open. Some of us are checking if we 
can find someone experienced with this code to provide reviews.

-

PR Comment: https://git.openjdk.org/jdk/pull/17703#issuecomment-2151485139

Re: RFR: 8326227: Rounding error that may distort computeNextGaussian results [v3]

[Chris Hennick]

On Wed, 21 Feb 2024 02:18:08 GMT, Chris Hennick  wrote:

>> This provides a slightly more accurate bounding limit for 
>> `computeNextExponentialSoftCapped` when calling it from 
>> `computeNextGaussian`. This could cause the `while 
>> (computeNextExponentialSoftCapped(rng, limit) < limit)` check in 
>> `computeNextGaussian` on line 1402 to always be true, making the 
>> `nextGaussian` runtime unbounded in the worst case; but more likely, it 
>> would give a result that was truncated too close to zero.
>> 
>> This change is being tested prior to submission to OpenJDK by 
>> https://github.com/openjdk/jdk/pull/17703/commits/b8be051cbf40a6a05fafc6a2c76942e9e0b11fdf.
>
> Chris Hennick has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Bug fix: add-exports was for wrong package

Keep open. @JimLaskey it looks like you're the author of most of the 
surrounding code; can you please confirm that there's a rounding error in the 
existing implementation and that this PR will fix it?

-

PR Comment: https://git.openjdk.org/jdk/pull/17703#issuecomment-2101885931

Re: RFR: 8326227: Rounding error that may distort computeNextGaussian results [v3]

[Chris Hennick]

On Wed, 21 Feb 2024 02:18:08 GMT, Chris Hennick  wrote:

>> This provides a slightly more accurate bounding limit for 
>> `computeNextExponentialSoftCapped` when calling it from 
>> `computeNextGaussian`. This could cause the `while 
>> (computeNextExponentialSoftCapped(rng, limit) < limit)` check in 
>> `computeNextGaussian` on line 1402 to always be true, making the 
>> `nextGaussian` runtime unbounded in the worst case; but more likely, it 
>> would give a result that was truncated too close to zero.
>> 
>> This change is being tested prior to submission to OpenJDK by 
>> https://github.com/openjdk/jdk/pull/17703/commits/b8be051cbf40a6a05fafc6a2c76942e9e0b11fdf.
>
> Chris Hennick has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Bug fix: add-exports was for wrong package

Keep open

On Wed, Apr 10, 2024, 17:40 bridgekeeper[bot] ***@***.***>
wrote:

> @Pr0methean  This pull request has been
> inactive for more than 4 weeks and will be automatically closed if another
> 4 weeks passes without any activity. To avoid this, simply add a new
> comment to the pull request. Feel free to ask for assistance if you need
> help with progressing this pull request towards integration!
>
> —
> Reply to this email directly, view it on GitHub
> , or
> unsubscribe
> 
> .
> You are receiving this because you were mentioned.Message ID:
> ***@***.***>
>

-

PR Comment: https://git.openjdk.org/jdk/pull/17703#issuecomment-2048715593

Re: RFR: 8326227: Rounding error that may distort computeNextGaussian results [v3]

[Raffaello Giulietti]

On Thu, 29 Feb 2024 01:54:54 GMT, Chris Hennick  wrote:

>> Chris Hennick has updated the pull request incrementally with one additional 
>> commit since the last revision:
>> 
>>   Bug fix: add-exports was for wrong package
>
> @turbanoff @rgiulietti This is a follow-up to my previously merged #8131.

@Pr0methean I'm not familiar with this code, nor with the underlying theory, so 
I'm not really able to give a meaningful review.

-

PR Comment: https://git.openjdk.org/jdk/pull/17703#issuecomment-1971114920

Re: RFR: 8326227: Rounding error that may distort computeNextGaussian results [v3]

[Chris Hennick]

On Wed, 21 Feb 2024 02:18:08 GMT, Chris Hennick  wrote:

>> This provides a slightly more accurate bounding limit for 
>> `computeNextExponentialSoftCapped` when the computed bound is greater than 
>> `(1.0p53 - 1.0) * DoubleZigguratTables.exponentialX0`. This could cause the 
>> `while (computeNextExponentialSoftCapped(rng, limit) < limit)` check in 
>> `computeNextGaussian` on line 1402 to always be true, making the 
>> `nextGaussian` runtime unbounded in the worst case.
>> 
>> This change is being tested prior to submission to OpenJDK by 
>> https://github.com/openjdk/jdk/pull/17703/commits/b8be051cbf40a6a05fafc6a2c76942e9e0b11fdf.
>
> Chris Hennick has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Bug fix: add-exports was for wrong package

@turbanoff @rgiulietti This is a follow-up to my previously merged #8131.

-

PR Comment: https://git.openjdk.org/jdk/pull/17703#issuecomment-1970255590

Re: RFR: 8326227: Rounding error that may distort computeNextGaussian results [v3]

[Joe Darcy]

On Thu, 22 Feb 2024 03:01:58 GMT, Chris Hennick  wrote:

> Update: confirmed that the new test fails without the fix.

Thanks for verifying the test checks the fix; I'll let others who have worked 
more directly on the random code review the actual fix.

-

PR Comment: https://git.openjdk.org/jdk/pull/17703#issuecomment-1958740871

Re: RFR: 8326227: Rounding error that may distort computeNextGaussian results [v3]

[Chris Hennick]

On Wed, 21 Feb 2024 02:18:08 GMT, Chris Hennick  wrote:

>> This provides a slightly more accurate bounding limit for 
>> `computeNextExponentialSoftCapped` when the computed bound is greater than 
>> `(1.0p53 - 1.0) * DoubleZigguratTables.exponentialX0`. This could cause the 
>> `while (computeNextExponentialSoftCapped(rng, limit) < limit)` check in 
>> `computeNextGaussian` on line 1402 to always be true, making the 
>> `nextGaussian` runtime unbounded in the worst case.
>> 
>> This change is being tested prior to submission to OpenJDK by 
>> https://github.com/openjdk/jdk/pull/17703/commits/b8be051cbf40a6a05fafc6a2c76942e9e0b11fdf.
>
> Chris Hennick has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Bug fix: add-exports was for wrong package

Update: confirmed that the new test fails without the fix.

-

PR Comment: https://git.openjdk.org/jdk/pull/17703#issuecomment-1958571446

Re: RFR: 8326227: Rounding error that may distort computeNextGaussian results [v3]

[Chris Hennick]

> This provides a slightly more accurate bounding limit for 
> `computeNextExponentialSoftCapped` when the computed bound is greater than 
> `(1.0p53 - 1.0) * DoubleZigguratTables.exponentialX0`. This could cause the 
> `while (computeNextExponentialSoftCapped(rng, limit) < limit)` check in 
> `computeNextGaussian` on line 1402 to always be true, making the 
> `nextGaussian` runtime unbounded in the worst case.

Chris Hennick has updated the pull request incrementally with one additional 
commit since the last revision:

  Bug fix: add-exports was for wrong package

-

Changes:
  - all: https://git.openjdk.org/jdk/pull/17703/files
  - new: https://git.openjdk.org/jdk/pull/17703/files/e68d03eb..b8be051c

Webrevs:
 - full: https://webrevs.openjdk.org/?repo=jdk&pr=17703&range=02
 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=17703&range=01-02

  Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod
  Patch: https://git.openjdk.org/jdk/pull/17703.diff
  Fetch: git fetch https://git.openjdk.org/jdk.git pull/17703/head:pull/17703

PR: https://git.openjdk.org/jdk/pull/17703