build and test fails with OpenJDK11

[ostueker]

I noticed the build of Euclid fails when using OpenJDK11:

[ERROR] Failed to execute goal org.apache.maven.plugins:maven-compiler-plugin:3.3:compile (default-compile) on project euclid: Compilation failure
[ERROR] /home/travis/build/BlueObelisk/euclid/src/main/java/blogspot/software_and_algorithms/stern_library/data_structure/ThriftyList.java:[1043,30] reference to toArray is ambiguous
[ERROR]   both method <T>toArray(java.util.function.IntFunction<T[]>) in java.util.Collection and method <U>toArray(U[]) in blogspot.software_and_algorithms.stern_library.data_structure.ThriftyList match

I've managed to get the build working with this:

diff --git a/src/main/java/blogspot/software_and_algorithms/stern_library/data_structure/ThriftyList.java b/src/main/java/blogspot/software_and_algorithms/stern_library/data_structure/ThriftyList.java
index 0b5a091..6dc166d 100644
--- a/src/main/java/blogspot/software_and_algorithms/stern_library/data_structure/ThriftyList.java
+++ b/src/main/java/blogspot/software_and_algorithms/stern_library/data_structure/ThriftyList.java
@@ -1040,7 +1040,7 @@ public class ThriftyList<T> extends AbstractList<T> implements List<T>,
         */
        @Override
        public T[] toArray() {
-               return (T[]) toArray(null);
+               return (T[]) toArray((Object[]) null);
        }

        /**

However I really can't tell whether this would be the right thing to do.

---

There are also some tests that are failing with OpenJDK11:

target/surefire-reports/org.xmlcml.euclid.test.PolarTest.txt

-------------------------------------------------------------------------------
Test set: org.xmlcml.euclid.test.PolarTest
-------------------------------------------------------------------------------
Tests run: 17, Failures: 1, Errors: 0, Skipped: 0, Time elapsed: 0.006 sec <<< FAILURE! - in org.xmlcml.euclid.test.PolarTest
testPlus(org.xmlcml.euclid.test.PolarTest)  Time elapsed: 0.001 sec  <<< FAILURE!
java.lang.AssertionError: polar expected:<60.0> but was:<60.000000000000014>
    at org.junit.Assert.fail(Assert.java:91)
    at org.junit.Assert.failNotEquals(Assert.java:645)
    at org.junit.Assert.assertEquals(Assert.java:441)
    at org.xmlcml.euclid.test.PolarTest.testPlus(PolarTest.java:138)

Here the precision is just out of the bounds of EPS.

target/surefire-reports/org.xmlcml.euclid.test.IntArrayTest.txt

-------------------------------------------------------------------------------
Test set: org.xmlcml.euclid.test.IntArrayTest
-------------------------------------------------------------------------------
Tests run: 58, Failures: 2, Errors: 0, Skipped: 0, Time elapsed: 0.034 sec <<< FAILURE! - in org.xmlcml.euclid.test.IntArrayTest
testElementAt(org.xmlcml.euclid.test.IntArrayTest)  Time elapsed: 0.001 sec  <<< FAILURE!
org.junit.ComparisonFailure: ArrayIndexOutOfBoundsException expected:<[5]> but was:<[Index 5 out of bounds for length 4]>
    at org.junit.Assert.assertEquals(Assert.java:123)
    at org.xmlcml.euclid.test.IntArrayTest.testElementAt(IntArrayTest.java:229)

testGetSubArray(org.xmlcml.euclid.test.IntArrayTest)  Time elapsed: 0 sec  <<< FAILURE!
org.junit.ComparisonFailure: subArray ArrayIndexOutOfBoundsException expected:<...OutOfBoundsException[]> but was:<...OutOfBoundsException[: arraycopy: last source index 6 out of bounds for int[4]]>
    at org.junit.Assert.assertEquals(Assert.java:123)
    at org.xmlcml.euclid.test.IntArrayTest.testGetSubArray(IntArrayTest.java:375)

target/surefire-reports/org.xmlcml.euclid.test.RealArrayTest.txt

-------------------------------------------------------------------------------
Test set: org.xmlcml.euclid.test.RealArrayTest
-------------------------------------------------------------------------------
Tests run: 88, Failures: 2, Errors: 0, Skipped: 0, Time elapsed: 0.14 sec <<< FAILURE! - in org.xmlcml.euclid.test.RealArrayTest
testElementAt(org.xmlcml.euclid.test.RealArrayTest)  Time elapsed: 0.001 sec  <<< FAILURE!
org.junit.ComparisonFailure: ArrayIndexOutOfBoundsException expected:<[5]> but was:<[Index 5 out of bounds for length 4]>
    at org.junit.Assert.assertEquals(Assert.java:123)
    at org.xmlcml.euclid.test.RealArrayTest.testElementAt(RealArrayTest.java:537)

testGetSubArray(org.xmlcml.euclid.test.RealArrayTest)  Time elapsed: 0.001 sec  <<< FAILURE!
org.junit.ComparisonFailure: subArray ArrayIndexOutOfBoundsException expected:<...OutOfBoundsException[]> but was:<...OutOfBoundsException[: arraycopy: last source index 6 out of bounds for double[4]]>
    at org.junit.Assert.assertEquals(Assert.java:123)
    at org.xmlcml.euclid.test.RealArrayTest.testGetSubArray(RealArrayTest.java:714)

The other ones seem to choke on some more comprehensive error messages.

[mjw99]

On the ThriftyList issue; it also exists in https://github.com/KevinStern/software-and-algorithms . I would file a bug there with a suggestion/PR of your fix.

[mjw99]

On the PolarTest issue; maybe revisit the value of org.xmlcml.euclid.EC.EPS or work out if something is actually wrong?

[ostueker]

On the PolarTest issue; maybe revisit the value of org.xmlcml.euclid.EC.EPS or work out if something is actually wrong?

The difference is 0.000000000000014, which is just outside the tolerance (EPS) of 0.00000000000001. Increasing EPS slightly should probably be okay.

[mjw99]

OK, sounds reasonable within this context; but I would modify the delta on the assert method of that specific test with a scaling prefactor, but not change the value of EPS.

Of course, I did not write this code, hence I don't know how important this is, but sometimes it can be very important.

[ostueker]

As suggested I've fixed the PolarTest issue by tweaking the tolerance within the test itself. (96a1cc1)

I've also created a Work-In-Progress PR #7 .

---

Until a new (and compatible) version of the stern_library is released, I'll also apply that fix internally. (According to KevinStern/software-and-algorithms#11 there haven't been new releases in a while.)

[mjw99]

of the stern_library is released,

Sounds reasonable; I also noticed that that repo seems to have been dead for ~3 years.

[petermr]

The main point of tests is major regression fails. It's probably a good idea to set EPS considerably larger. There are several reasons for EPS.

a/ direct FP equality where we expect the values to be identical but this is to make sure b/ physical tolerance - e.g. is a line horizontal c/ (related) precision (ndecimal) for formatting FP for printing - mainly to save space.

Note that complex geometrical operations can give quite large deviations on different systems.

There is generally no consistency over different packages. Probably the main thing is to try to avoid hardcoding and at least have per-package CONSTANTs. An industrial strength implementation would probably have special toolkits for precision.

On Thu, Jan 2, 2020 at 1:52 PM Mark J. Williamson notifications@github.com wrote:

of the stern_library is released,

Sounds reasonable; I also noticed that that repo seems to have been dead for ~3 years.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/BlueObelisk/euclid/issues/5?email_source=notifications&email_token=AAFTCS5DHH3Q5JVBARKYV3TQ3XWRVA5CNFSM4KB4H35KYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEH6MGLA#issuecomment-570213164, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAFTCS66YTZORC7QXA6IG3LQ3XWRVANCNFSM4KB4H35A .

-- "I always retain copyright in my papers, and nothing in any contract I sign with any publisher will override that fact. You should do the same".

Peter Murray-Rust Reader Emeritus in Molecular Informatics Unilever Centre, Dept. Of Chemistry University of Cambridge CB2 1EW, UK +44-1223-763069

[ostueker]

I've merged some fixes that make the unit tests pass under both JDK8 and JDK11.

For the PolarTest, I took the cautious approach by using 2.0 * EPS in the single assert statement (commit 96a1cc1). Then in commit 5e05a81 I've used some more sophisticated hamcrest.CoreMatchers to be able to deal with the different exception messages in JDK8 and 11, which required a newer JUnit.

Shall we create a new issue for discussing how to increase EPS in general?

[petermr]

On Thu, Jan 9, 2020 at 12:08 AM Oliver Stueker notifications@github.com wrote:

I've merged some fixes that make the unit tests pass under both JDK8 and JDK11.

For the PolarTest, I took the cautious approach by using 2.0 * EPS in the single assert statement (commit 96a1cc1 https://github.com/BlueObelisk/euclid/commit/96a1cc15add557e5c8e9aedcba2bb95b232d4675 ). Then in commit 5e05a81 https://github.com/BlueObelisk/euclid/commit/5e05a81d7872c6822a14d3e274c1425f70539451 I've used some more sophisticated hamcrest.CoreMatchers to be able to deal with the different exception messages in JDK8 and 11, which required a newer JUnit.

Thanks, I remember Hamcrest from years ago but haven't used it recently

Shall we create a new issue for discussing how to increase EPS in general?

Yes. It's a harder problem than it looks. In some cases direct equality is often good enough - e.g. when numbers have been copied or formatted to - say - 3 places. But even simple maths operations can build up errors. And things like Angle can have quite large variations Then there's "experimental" error - extracting coordinates from bitmaps, for example, which certainly seem to be time variable - i.e when there's a new OS they need mending. Shouldn't happen but it could be order of computation , etc.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/BlueObelisk/euclid/issues/5?email_source=notifications&email_token=AAFTCS224UI35XFOOKY6X2LQ4ZTJRA5CNFSM4KB4H35KYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEIOOGYI#issuecomment-572318561, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAFTCS72IPZKRNA5EJYESALQ4ZTJRANCNFSM4KB4H35A .

-- Peter Murray-Rust Founder ContentMine.org and Reader Emeritus in Molecular Informatics Dept. Of Chemistry, University of Cambridge, CB2 1EW, UK

[ostueker]

I've created a new issue #10 for refactoring the EPS in the future.

Closing this one.