jdk17u-dev
jdk17u-dev copied to clipboard
openjdk
8315024: Vector API FP reduction tests should not test for exact equality
Backport of JDK-8315024
- This PR has two commits
commit 1: the git apply result, which iscleancommit 2: manually merge the following two.rejfilestest/jdk/jdk/incubator/vector/DoubleMaxVectorTests.java.rejtest/jdk/jdk/incubator/vector/FloatMaxVectorTests.java.rej
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) {
int i = 0;
try {
- Assert.assertEquals(rc, fa.apply(a, mask));
+ Assert.assertEquals(rc, fa.apply(a, mask), Math.abs(rc * relativeError));
for (; i < a.length; i += SPECIES.length()) {
- Assert.assertEquals(r[i], f.apply(a, i, mask));
+ Assert.assertEquals(r[i], f.apply(a, i, mask), Math.abs(r[i] *
+relativeError));
}
} catch (AssertionError e) {
- Assert.assertEquals(rc, fa.apply(a, mask), "Final result is incorrect!");
- Assert.assertEquals(r[i], f.apply(a, i, mask), "at index #" + i);
+ Assert.assertEquals(rc, fa.apply(a, mask), Math.abs(rc * relativeError), "Final result is incorrect!");
+ Assert.assertEquals(r[i], f.apply(a, i, mask), Math.abs(r[i] * relativeError), "at index #" + i);
}
}
Content of test/jdk/jdk/incubator/vector/FloatMaxVectorTests.java.rej
@@ -668,15 +670,21 @@
static void assertReductionArraysEquals(float[] r, float rc, float[] a,
FReductionOp f, FReductionAllOp fa) {
+ assertReductionArraysEquals(r, rc, a, f, fa, (float)0.0);
+ }
+
+ static void assertReductionArraysEquals(float[] r, float rc, float[] a,
+ FReductionOp f, FReductionAllOp fa,
+ float 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(float[] r, float rc, float[] a, boolean[] mask,
FReductionMaskedOp f, FReductionAllMaskedOp fa) {
+ assertReductionArraysEqualsMasked(r, rc, a, mask, f, fa, (float)0.0);
+ }
+
+ static void assertReductionArraysEqualsMasked(float[] r, float rc, float[] a, boolean[] mask,
+ FReductionMaskedOp f, FReductionAllMaskedOp fa,
+ float relativeError) {
int i = 0;
try {
- Assert.assertEquals(rc, fa.apply(a, mask));
+ Assert.assertEquals(rc, fa.apply(a, mask), Math.abs(rc * relativeError));
for (; i < a.length; i += SPECIES.length()) {
- Assert.assertEquals(r[i], f.apply(a, i, mask));
+ Assert.assertEquals(r[i], f.apply(a, i, mask), Math.abs(r[i] *
+relativeError));
}
} catch (AssertionError e) {
- Assert.assertEquals(rc, fa.apply(a, mask), "Final result is incorrect!");
- Assert.assertEquals(r[i], f.apply(a, i, mask), "at index #" + i);
+ Assert.assertEquals(rc, fa.apply(a, mask), Math.abs(rc * relativeError), "Final result is incorrect!");
+ Assert.assertEquals(r[i], f.apply(a, i, mask), Math.abs(r[i] * relativeError), "at index #" + i);
}
}
Testing
- Local: Test passed on
MacOS 14.5withApple M1 MaxDouble128VectorTests.java: Test results: passed: 1Double256VectorTests.java: Test results: passed: 1Double512VectorTests.java: Test results: passed: 1Double64VectorTests.java: Test results: passed: 1DoubleMaxVectorTests.java: Test results: passed: 1Float128VectorTests.java: Test results: passed: 1Float256VectorTests.java: Test results: passed: 1Float512VectorTests.java: Test results: passed: 1Float64VectorTests.java: Test results: passed: 1FloatMaxVectorTests.java: Test results: passed: 1
- Pipeline:
- Testing Machine:
Progress
- [ ] Change must be properly reviewed (1 review required, with at least 1 Reviewer)
- [x] Change must not contain extraneous whitespace
- [x] Commit message must refer to an issue
- [ ] JDK-8315024 needs maintainer approval
Issue
- JDK-8315024: Vector API FP reduction tests should not test for exact equality (Bug - P5)
Reviewing
Using git
Checkout this PR locally:
$ git fetch https://git.openjdk.org/jdk17u-dev.git pull/2768/head:pull/2768
$ git checkout pull/2768
Update a local copy of the PR:
$ git checkout pull/2768
$ git pull https://git.openjdk.org/jdk17u-dev.git pull/2768/head
Using Skara CLI tools
Checkout this PR locally:
$ git pr checkout 2768
View PR using the GUI difftool:
$ git pr show -t 2768
Using diff file
Download this PR as a diff file:
https://git.openjdk.org/jdk17u-dev/pull/2768.diff
Webrev
:wave: Welcome back ashi! A progress list of the required criteria for merging this PR into master will be added to the body of your pull request. There are additional pull request commands available for use with this pull request.
![bridgekeeper[bot] avatar](https://avatars.githubusercontent.com/u/41768318?v=4 "Published by a pub.dev verified publisher"){kind=small}
@amosshi This change now passes all automated pre-integration checks.
ℹ️ This project also has non-automated pre-integration requirements. Please see the file CONTRIBUTING.md for details.
After integration, the commit message for the final commit will be:
8315024: Vector API FP reduction tests should not test for exact equality
Reviewed-by: mdoerr
You can use pull request commands such as /summary, /contributor and /issue to adjust it as needed.
At the time when this comment was updated there had been 245 new commits pushed to the master branch:
- be4f913efab46b601f7cc472d654b4da883df152: 8312049: runtime/logging/ClassLoadUnloadTest can be improved
- 8dbd6772f9b65ab1ae6ef683c7300092e4745951: 8315576: compiler/codecache/CodeCacheFullCountTest.java fails after JDK-8314837
- c10340918cfe6d316c1d4d57e9f586c94c6d3098: 8308891: TestCDSVMCrash.java needs @requires vm.cds
- d12077dc7d92eac99da9ab40c19a01b48f5dc121: 8284316: Support accessibility ManualTestFrame.java for non SwingSet tests
- 6cbedd928985e9ddf8a370470c8ee9f4d5f0826b: 8273216: JCMD does not work across container boundaries with Podman
- d5d26be717f5540de73e9d411d324f870d3d224b: 8315936: Parallelize gc/stress/TestStressG1Humongous.java test
- 2e3ca3149709f40996eac5fe96021cac259b29c2: 8303891: Speed up Zip64SizeTest using a small ZIP64 file
- 27c92b6d50cf30bb08fe7829d4a3bce3f5b3f390: 8299813: java/nio/channels/DatagramChannel/Disconnect.java fails with jtreg test timeout due to lost datagram
- 61e0a11132d4d85986b9f5d91439cc6f28ca03c7: 8315684: Parallelize sun/security/util/math/TestIntegerModuloP.java
- 698f0e37746bffe61fc59cd553d3582d0e004097: 8313674: (fc) java/nio/channels/FileChannel/BlockDeviceSize.java should test for more block devices
- ... and 235 more: https://git.openjdk.org/jdk17u-dev/compare/d953de6b2cb0572ea24b9e02952aa5aedee6cc64...master
As there are no conflicts, your changes will automatically be rebased on top of these commits when integrating. If you prefer to avoid this automatic rebasing, please check the documentation for the /integrate command for further details.
➡️ To integrate this PR with the above commit message to the master branch, type /integrate in a new comment.
Webrevs
- 05: Full - Incremental (2b9e3b34)
- 04: Full - Incremental (a7ff7735)
- 03: Full - Incremental (1b6851e9)
- 02: Full - Incremental (7ffeb081)
- 01: Full - Incremental (23f5de96)
- 00: Full (678979a6)
In the original commit I see in test/jdk/jdk/incubator/vector/DoubleMaxVectorTests.java
https://github.com/openjdk/jdk/commit/e6f23a90d4a53339a3c9c2b76fc5d317940e4472 private static final double RELATIVE_ROUNDING_ERROR = (double)0.000001;
Is it intentional that it is not in the backport ?
Hi @MBaesken,
Somehow the git patch command has put these two lines to the test/jdk/jdk/incubator/vector/FloatMaxVectorTests.java file at line 625-626
+ 625 // for floating point reduction ops that may introduce rounding errors
+ 626 private static final double RELATIVE_ROUNDING_ERROR = (double)0.000001;
I will move them to earlier lines in respect to the original commit.
Regards Amos A Happy Developer
⚠️ @amosshi This change is now ready for you to apply for maintainer approval. This can be done directly in each associated issue or by using the /approval command.
/approval request "Backporting for parity with 17.0.13-oracle. No risk, test fix. Un-Clean backport. Test passes and SAP nightlies pass on 2024-08-17”
Going to push as commit 03cbfaa733bc6a59da6ba5c0d0918245e534752a.
Since your change was applied there have been 250 commits pushed to the master branch:
- 803c2c27d88ee0948611f11f1d099d7fe5c7d593: 8302800: Augment NaN handling tests of FDLIBM methods
- cd0bb4d31ee53947234c717782f9d9bf9b488986: 8310683: Refactor StandardCharset/standard.java to use JUnit
- 1887a80260abc7d5ef4fb70a7a7a740751fdb398: 8332248: (fc) java/nio/channels/FileChannel/BlockDeviceSize.java failed with RuntimeException
- 64ab2faaddb0e7a81dd0139dbb178ef31b1aa40e: 8303920: Avoid calling out to python in DataDescriptorSignatureMissing test
- 97e4a9374922bd8fd84e1315f89b189669a7b6d0: 8316193: jdk/jfr/event/oldobject/TestListenerLeak.java java.lang.Exception: Could not find leak
- be4f913efab46b601f7cc472d654b4da883df152: 8312049: runtime/logging/ClassLoadUnloadTest can be improved
- 8dbd6772f9b65ab1ae6ef683c7300092e4745951: 8315576: compiler/codecache/CodeCacheFullCountTest.java fails after JDK-8314837
- c10340918cfe6d316c1d4d57e9f586c94c6d3098: 8308891: TestCDSVMCrash.java needs @requires vm.cds
- d12077dc7d92eac99da9ab40c19a01b48f5dc121: 8284316: Support accessibility ManualTestFrame.java for non SwingSet tests
- 6cbedd928985e9ddf8a370470c8ee9f4d5f0826b: 8273216: JCMD does not work across container boundaries with Podman
- ... and 240 more: https://git.openjdk.org/jdk17u-dev/compare/d953de6b2cb0572ea24b9e02952aa5aedee6cc64...master
Your commit was automatically rebased without conflicts.
@amosshi Pushed as commit 03cbfaa733bc6a59da6ba5c0d0918245e534752a.
:bulb: You may see a message that your pull request was closed with unmerged commits. This can be safely ignored.