Re: RFR: 8214761: Bug in parallel Kahan summation implementation [v2]

2021-08-30 Thread Joe Darcy
On Wed, 21 Jul 2021 20:19:31 GMT, Ian Graves  wrote:

>> 8214761: Bug in parallel Kahan summation implementation
>
> Ian Graves has updated the pull request incrementally with three additional 
> commits since the last revision:
> 
>  - Updates with more test coverage
>  - stashing
>  - Stashing

The code changes look fine, but IMO the comments should be re-worded a bit.

Rather text like

// Negating this value because low-order bits are in negated form

I suggest something like

// Subtract compensation bits

The main compensation loop also subtracts the compensation bits. A comment like 
"subtract compensation bits" makes the corrected handling of them seem less 
anomalous.

-

Marked as reviewed by darcy (Reviewer).

PR: https://git.openjdk.java.net/jdk/pull/4674


Re: RFR: 8214761: Bug in parallel Kahan summation implementation [v2]

2021-07-22 Thread Ian Graves
On Wed, 21 Jul 2021 20:19:31 GMT, Ian Graves  wrote:

>> 8214761: Bug in parallel Kahan summation implementation
>
> Ian Graves has updated the pull request incrementally with three additional 
> commits since the last revision:
> 
>  - Updates with more test coverage
>  - stashing
>  - Stashing

Circling back on this. I've worked in the test from Ivan Gerasimov's email back 
when. It includes some additional comparisons to prior approaches to assert 
improvements in error.

-

PR: https://git.openjdk.java.net/jdk/pull/4674


Re: RFR: 8214761: Bug in parallel Kahan summation implementation [v2]

2021-07-21 Thread Ian Graves
> 8214761: Bug in parallel Kahan summation implementation

Ian Graves has updated the pull request incrementally with three additional 
commits since the last revision:

 - Updates with more test coverage
 - stashing
 - Stashing

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/4674/files
  - new: https://git.openjdk.java.net/jdk/pull/4674/files/d7fc3948..10b8dcda

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk=4674=01
 - incr: https://webrevs.openjdk.java.net/?repo=jdk=4674=00-01

  Stats: 216 lines in 4 files changed: 213 ins; 0 del; 3 mod
  Patch: https://git.openjdk.java.net/jdk/pull/4674.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/4674/head:pull/4674

PR: https://git.openjdk.java.net/jdk/pull/4674