[jdk17u-dev] RFR: 8315024: Vector API FP reduction tests should not test for exact equality [v4]

[Martin Doerr]

On Fri, 16 Aug 2024 06:36:22 GMT, Amos Shi <ashi at openjdk.org> wrote:

>> Backport of [JDK-8315024](https://bugs.openjdk.org/browse/JDK-8315024)
>> - This PR has two commits
>>   - `commit 1`: the git apply result, which is `clean`
>>   - `commit 2`: manually merge the following two `.rej` files
>>     - `test/jdk/jdk/incubator/vector/DoubleMaxVectorTests.java.rej`
>>     - `test/jdk/jdk/incubator/vector/FloatMaxVectorTests.java.rej`
>>   - `commit 3,4`: Fix code postion
>>   - `commit 5`: Add file `Unit-header.template` 
>> 
>> Content of `test/jdk/jdk/incubator/vector/DoubleMaxVectorTests.java.rej`
>> 
>> 
>> @@ -668,15 +670,21 @@
>>  
>>      static void assertReductionArraysEquals(double[] r, double rc, double[] a,
>>                                              FReductionOp f, FReductionAllOp fa) {
>> +        assertReductionArraysEquals(r, rc, a, f, fa, (double)0.0);
>> +    }
>> +
>> +    static void assertReductionArraysEquals(double[] r, double rc, double[] a,
>> +                                            FReductionOp f, FReductionAllOp fa,
>> +                                            double relativeError) {
>>          int i = 0;
>>          try {
>> -            Assert.assertEquals(rc, fa.apply(a));
>> +            Assert.assertEquals(rc, fa.apply(a), Math.abs(rc * relativeError));
>>              for (; i < a.length; i += SPECIES.length()) {
>> -                Assert.assertEquals(r[i], f.apply(a, i));
>> +                Assert.assertEquals(r[i], f.apply(a, i), Math.abs(r[i] * relativeError));
>>              }
>>          } catch (AssertionError e) {
>> -            Assert.assertEquals(rc, fa.apply(a), "Final result is incorrect!");
>> -            Assert.assertEquals(r[i], f.apply(a, i), "at index #" + i);
>> +            Assert.assertEquals(rc, fa.apply(a), Math.abs(rc * relativeError), "Final result is incorrect!");
>> +            Assert.assertEquals(r[i], f.apply(a, i), Math.abs(r[i] * relativeError), "at index #" + i);
>>          }
>>      }
>>  
>> @@ -690,15 +698,22 @@
>>  
>>      static void assertReductionArraysEqualsMasked(double[] r, double rc, double[] a, boolean[] mask,
>>                                              FReductionMaskedOp f, FReductionAllMaskedOp fa) {
>> +        assertReductionArraysEqualsMasked(r, rc, a, mask, f, fa, (double)0.0);
>> +    }
>> +
>> +    static void assertReductionArraysEqualsMasked(double[] r, double rc, double[] a, boolean[] mask,
>> +                                            FReductionMaskedOp f, FReductionAllMaskedOp fa,
>> +                                            double relativeError) {
>>       ...
>
> Amos Shi has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Add file Unit-header.template

Looks basically good. I only found a couple of formatting differences which should get removed.

test/jdk/jdk/incubator/vector/DoubleMaxVectorTests.java line 612:

> 610:     }
> 611: 
> 612: 

This empty line should not get removed.

test/jdk/jdk/incubator/vector/FloatMaxVectorTests.java line 612:

> 610:     }
> 611: 
> 612: 

This empty line should not get removed.

test/jdk/jdk/incubator/vector/templates/Unit-header.template line 184:

> 182:             Assert.assertEquals(r[i], f.apply(a, i), Math.abs(r[i] * relativeError), "at index #" + i);
> 183:         }
> 184:      }

1 space too much.

test/jdk/jdk/incubator/vector/templates/Unit-header.template line 228:

> 226:             Assert.assertEquals(r[i], f.apply(a, i, mask), Math.abs(r[i] * relativeError), "at index #" + i);
> 227:         }
> 228:      }

1 space too much.

-------------

PR Review: https://git.openjdk.org/jdk17u-dev/pull/2768#pullrequestreview-2242314518
PR Review Comment: https://git.openjdk.org/jdk17u-dev/pull/2768#discussion_r1719596433
PR Review Comment: https://git.openjdk.org/jdk17u-dev/pull/2768#discussion_r1719598000
PR Review Comment: https://git.openjdk.org/jdk17u-dev/pull/2768#discussion_r1719607476
PR Review Comment: https://git.openjdk.org/jdk17u-dev/pull/2768#discussion_r1719607685