New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Intel compiled test failures with geodtest #826

Closed
mwtoews opened this Issue Feb 27, 2018 · 18 comments

Comments

Projects
None yet
3 participants
@mwtoews
Contributor

mwtoews commented Feb 27, 2018

Building proj-5.0.0RC6 using autotools with Intel compilers, I see some test failures. (These are non-critical, and can be addressed after PROJ v 5 is out.)

Here is the workflow

$ CC=icc ./configure
...
$ make
...
$ make check
Making check in src
make[1]: Entering directory `/tmp/proj-5.0.0/src'
make  geodtest
make[2]: Entering directory `/tmp/proj-5.0.0/src'
make[2]: Leaving directory `/tmp/proj-5.0.0/src'
make  check-TESTS
make[2]: Entering directory `/tmp/proj-5.0.0/src'
make[3]: Entering directory `/tmp/proj-5.0.0/src'
FAIL: geodtest
============================================================================
Testsuite summary for PROJ.4 Projections 5.0.0
============================================================================
# TOTAL: 1
# PASS:  0
# SKIP:  0
# XFAIL: 0
# FAIL:  1
# XPASS: 0
# ERROR: 0
============================================================================
See src/test-suite.log
Please report to warmerdam@pobox.com
============================================================================
$ cat src/test-suite.log
==================================================
   PROJ.4 Projections 5.0.0: src/test-suite.log
==================================================

# TOTAL: 1
# PASS:  0
# SKIP:  0
# XFAIL: 0
# FAIL:  1
# XPASS: 0
# ERROR: 0

.. contents:: :depth: 2

FAIL: geodtest
==============

assertEquals fails: 0 != 3.5e-14 +/- 1.5e-14
assertEquals fails: 180 != 180 +/- 1.5e-14
GeodSolve59 fail: 2
assertEquals fails: 180 != -180 +/- 5e-06
GeodSolve67 fail: 1
FAIL geodtest (exit status: 2)

I've started an open discussion with @cffk regarding some of these with GeographicLib, where they may perhaps be resolved.

@cffk

This comment has been minimized.

Contributor

cffk commented Feb 27, 2018

Regarding the geodtest failures... This isn't a case where relaxing the tests is the right solution. It's possible that icc is optimizing too aggressively. Is it possible to compile the code without optimization (-O0)? I'll also see if I can lay my hands on a system with icc installed.

@mwtoews

This comment has been minimized.

Contributor

mwtoews commented Feb 27, 2018

@cffk your guess is correct; make clean && CC=icc CFLAGS=-O0 ./configure && make && make check is clean.
I'm not sure how to get around Intel's aggressive optimizations here...

@mwtoews

This comment has been minimized.

Contributor

mwtoews commented Feb 27, 2018

Testing further, the minimum optimization threshold that triggers this error for icc is -O1

@cffk

This comment has been minimized.

Contributor

cffk commented Feb 27, 2018

@mwtoews Can you try the following patch...?

diff --git a/src/geodesic.c b/src/geodesic.c
index 23557b5c..b5be3507 100644
--- a/src/geodesic.c
+++ b/src/geodesic.c
@@ -764,7 +764,14 @@ static real geod_geninverse_int(const struct geod_geodesic* g,
   lonsign = lon12 >= 0 ? 1 : -1;
   /* If very close to being on the same half-meridian, then make it so. */
   lon12 = lonsign * AngRound(lon12);
+#if defined(__INTEL_COMPILER)
+  {
+    volatile real temp = 180 - lon12;
+    lon12s = AngRound(temp - lonsign * lon12s);
+  }
+#else
   lon12s = AngRound((180 - lon12) - lonsign * lon12s);
+#endif
   lam12 = lon12 * degree;
   if (lon12 > 90) {
 sincosdx(lon12s, &slam12, &clam12);
@cffk

This comment has been minimized.

Contributor

cffk commented Feb 27, 2018

Alternatively, add the

-fp-model precise

flag to the compiler. This is probably the preferred fix since it forces the compiler to pay attention to the parens in an expression (and the standard mandates that this be so).

@kbevers

This comment has been minimized.

Member

kbevers commented Feb 27, 2018

Alternatively, add the
-fp-model precise

How about we add that to the CFLAGS we set up with CMake and automake? Obviously only for the affected compiler which I believe is possible.

@cffk

This comment has been minimized.

Contributor

cffk commented Feb 27, 2018

Quite so. The more precise flag to set is

-fprotect-parens

and, with cmake, this can be made conditional on the compiler with

if (CMAKE_C_COMPILER_ID STREQUAL "Intel")
  set (CMAKE_C_FLAGS "${CMAKE_C_FLAGS} -fprotect-parens")
endif ()

Still need to figure what the appropriate autoconf magic is, though. I don't feel the need to rush this fix. The geodesic routines would have had this issue with the Intel compiler since version 4.9.3. Also, it's not that a wrong result is returned. It's just not as accurate as it should be.

@kbevers

This comment has been minimized.

Member

kbevers commented Feb 27, 2018

Still need to figure what the appropriate autoconf magic is, though.

There are some hints here although not a definitive answer.

I don't feel the need to rush this fix.

That wasn't my intention either. I'll put it in for 5.1.0.

@kbevers kbevers added this to the 5.1.0 milestone Feb 27, 2018

@mwtoews

This comment has been minimized.

Contributor

mwtoews commented Feb 27, 2018

@cffk yes, that flag fixes this issue (regardless of optimization level). And the CMake snippet put in cmake/Proj4SystemInfo.cmake was successful too.
As for autotools/conf, I'd suspect this would be implemented in m4 somehow...

@cffk

This comment has been minimized.

Contributor

cffk commented Feb 28, 2018

There are two issues with the Intel compiler: the parentheses are ignored in expressions like

(a + b) + c

and the expression

0.0 + x

is simplified to x (which is wrong if x = -0.0). The flag

-fp-model precise

is needed to fix both of these.

@kbevers kbevers modified the milestones: 5.1.0, 5.0.1 Mar 1, 2018

cffk added a commit to cffk/proj.4 that referenced this issue Mar 17, 2018

Patch 1.49.3 for geodesic package.
Set flags for Intel compiler to prevent incorrect optimization of
arithmetic expressions OSGeo#826.  Guard against nans in sincosdx OSGeo#834.
Issue OSGeo#831 is not addressed here (need more information...).

@cffk cffk closed this in a2c0a41 Mar 19, 2018

kbevers added a commit that referenced this issue Mar 19, 2018

Merge pull request #868 from cffk/geod-1.49.3
Patch 1.49.3 for geodesic package.

Closes #826, partially closes #843.
@mwtoews

This comment has been minimized.

Contributor

mwtoews commented Mar 20, 2018

I've had a chance to look at this with #868 but unfortunately with CC=icc, both cmake and autoconf builds complain:

icc: command line warning #10006: ignoring unknown option '-fsigned-zeros'

The -fsigned-zeros option seems to be a GCC-only compile flag (where the default is true).

Also with CMakeLists.txt, why are the Intel flags inconsistent? I.e. /fp:precise vs -fprotect-parens. Of these two compiler flags, I'd choose -fprotect-parens//Qprotect-parens, since -fp-model precise//fp:precise may slow performance.

With modified compile flags to just use -fprotect-parens, here are the failures from both build methods.

First from autotools, the main and new issue is GeodSolve73 fail: 1:

$ $ make check
Making check in src
make[1]: Entering directory `/tmp/proj.4/src'
make  geodtest
make[2]: Entering directory `/tmp/proj.4/src'
make[2]: Leaving directory `/tmp/proj.4/src'
make  check-TESTS
make[2]: Entering directory `/tmp/proj.4/src'
GeodSolve73 fail: 1
FAIL: geodtest
=======================================================
1 of 1 test failed
Please report to https://github.com/OSGeo/proj.4/issues
=======================================================
make[2]: *** [check-TESTS] Error 1
make[2]: Leaving directory `/tmp/proj.4/src'
make[1]: *** [check-am] Error 2
make[1]: Leaving directory `/tmp/proj.4/src'
make: *** [check-recursive] Error 1

and from the cmake build:

$ make test
Running tests...
Test project /tmp/proj.4/BUILD
      Start  1: test27
 1/29 Test  #1: test27 ...........................   Passed    1.56 sec
      Start  2: test83
 2/29 Test  #2: test83 ...........................   Passed    1.31 sec
      Start  3: testvarious
 3/29 Test  #3: testvarious ......................   Passed    0.52 sec
      Start  4: geodesic-test
 4/29 Test  #4: geodesic-test ....................***Failed    0.01 sec
      Start  5: Builtins
 5/29 Test  #5: Builtins .........................   Passed    0.06 sec
      Start  6: Builtins2
 6/29 Test  #6: Builtins2 ........................   Passed    0.06 sec
      Start  7: Axisswap
 7/29 Test  #7: Axisswap .........................   Passed    0.01 sec
      Start  8: Deformation
 8/29 Test  #8: Deformation ......................   Passed    0.01 sec
      Start  9: Ellipsoid
 9/29 Test  #9: Ellipsoid ........................   Passed    0.01 sec
      Start 10: GDA
10/29 Test #10: GDA ..............................   Passed    0.01 sec
      Start 11: 4D-API-cs2cs-style
11/29 Test #11: 4D-API-cs2cs-style ...............   Passed    0.03 sec
      Start 12: DHDN_ETRS89
12/29 Test #12: DHDN_ETRS89 ......................***Failed    0.01 sec
      Start 13: GIGS-5101.1-jhs
13/29 Test #13: GIGS-5101.1-jhs ..................   Passed    0.13 sec
      Start 14: GIGS-5101.2-jhs
14/29 Test #14: GIGS-5101.2-jhs ..................   Passed    0.06 sec
      Start 15: GIGS-5101.3-jhs
15/29 Test #15: GIGS-5101.3-jhs ..................   Passed    0.06 sec
      Start 16: GIGS-5101.4-jhs-etmerc
16/29 Test #16: GIGS-5101.4-jhs-etmerc ...........   Passed    0.06 sec
      Start 17: GIGS-5102.1
17/29 Test #17: GIGS-5102.1 ......................   Passed    0.12 sec
      Start 18: GIGS-5103.1
18/29 Test #18: GIGS-5103.1 ......................   Passed    0.06 sec
      Start 19: GIGS-5103.2
19/29 Test #19: GIGS-5103.2 ......................   Passed    0.04 sec
      Start 20: GIGS-5103.3
20/29 Test #20: GIGS-5103.3 ......................   Passed    0.04 sec
      Start 21: GIGS-5105.2
21/29 Test #21: GIGS-5105.2 ......................   Passed    0.08 sec
      Start 22: GIGS-5106
22/29 Test #22: GIGS-5106 ........................   Passed    0.07 sec
      Start 23: GIGS-5107
23/29 Test #23: GIGS-5107 ........................   Passed    0.03 sec
      Start 24: GIGS-5109
24/29 Test #24: GIGS-5109 ........................   Passed    0.03 sec
      Start 25: GIGS-5111.1
25/29 Test #25: GIGS-5111.1 ......................   Passed    0.07 sec
      Start 26: GIGS-5112
26/29 Test #26: GIGS-5112 ........................   Passed    0.02 sec
      Start 27: GIGS-5113
27/29 Test #27: GIGS-5113 ........................   Passed    0.01 sec
      Start 28: GIGS-5201
28/29 Test #28: GIGS-5201 ........................***Failed    0.03 sec
      Start 29: GIGS-5208
29/29 Test #29: GIGS-5208 ........................   Passed    0.06 sec

90% tests passed, 3 tests failed out of 29

Total Test time (real) =   4.59 sec

The following tests FAILED:
          4 - geodesic-test (Failed)
         12 - DHDN_ETRS89 (Failed)
         28 - GIGS-5201 (Failed)
Errors while running CTest
$ ./bin/geodtest
GeodSolve73 fail: 1
@cffk

This comment has been minimized.

Contributor

cffk commented Mar 20, 2018

There are two bugs with the Intel compiler and standard optimization:

(a) (a+b)+c can be evaluated as (a+c)+b
(b) 0.0+x is simplified to x

geodtest now has an added test in GeodSolve73 to catch (b).

(a) is fixed by -fprotect-parens
(b) is fixed by -fsigned-zeros (on the platform I was using)

If -fsigned-zeros isn't generally available, then the only alternative
is to use use -fp-model precise (which is what I used for Windows) --
unless, of course, there's a more narrowly defined flag that fixes (b);
is there?

@cffk

This comment has been minimized.

Contributor

cffk commented Mar 20, 2018

#874 resolves the issue of tests failing with the Intel compiler. Conceivably there's a more-tailored set of flags to the compiler which will do the job; however at least now we should have a working build with this compiler.

@cffk cffk reopened this Mar 20, 2018

@mwtoews

This comment has been minimized.

Contributor

mwtoews commented Mar 20, 2018

Thanks for #874, it was the appropriate fix! Now moving into the check/test outputs beyond geodtest...

From autotools

$ make check
Making check in src
make[1]: Entering directory `/home/mtoews/src/proj.4/src'
make  geodtest
make[2]: Entering directory `/home/mtoews/src/proj.4/src'
make[2]: Leaving directory `/home/mtoews/src/proj.4/src'
make  check-TESTS
make[2]: Entering directory `/home/mtoews/src/proj.4/src'
PASS: geodtest
=============
1 test passed
=============
make[2]: Leaving directory `/home/mtoews/src/proj.4/src'
make[1]: Leaving directory `/home/mtoews/src/proj.4/src'
Making check in man
make[1]: Entering directory `/home/mtoews/src/proj.4/man'
Making check in man1
make[2]: Entering directory `/home/mtoews/src/proj.4/man/man1'
make[2]: Nothing to be done for `check'.
make[2]: Leaving directory `/home/mtoews/src/proj.4/man/man1'
Making check in man3
make[2]: Entering directory `/home/mtoews/src/proj.4/man/man3'
make[2]: Nothing to be done for `check'.
make[2]: Leaving directory `/home/mtoews/src/proj.4/man/man3'
make[2]: Entering directory `/home/mtoews/src/proj.4/man'
make[2]: Nothing to be done for `check-am'.
make[2]: Leaving directory `/home/mtoews/src/proj.4/man'
make[1]: Leaving directory `/home/mtoews/src/proj.4/man'
Making check in nad
make[1]: Entering directory `/home/mtoews/src/proj.4/nad'
make  check-local
make[2]: Entering directory `/home/mtoews/src/proj.4/nad'
../nad/test27 ../src/proj
============================================
Running ../nad/test27 using ../src/proj:
============================================
doing tests into file proj_out27, please wait
diff proj_out27 with pj_out27.dist
TEST OK
test file proj_out27 removed

../nad/test83 ../src/proj
============================================
Running ../nad/test83 using ../src/proj:
============================================
doing tests into file proj_out83, please wait
diff proj_out83 with pj_out83.dist
TEST OK
test file proj_out83 removed

PROJ_LIB=. ../nad/testvarious ../src/cs2cs
Using locale with comma as decimal separator
============================================
Running ../nad/testvarious using ../src/cs2cs:
============================================
doing tests into file tv_out, please wait
Rel. 5.0.0, March 1st, 2018
<lt-cs2cs>: while processing file: <stdin>, line 1
pj_transform(): invalid x or y
Rel. 5.0.0, March 1st, 2018
<lt-cs2cs>: while processing file: <stdin>, line 2
pj_transform(): acos/asin: |arg| >1.+1e-14
diff tv_out with tv_out.dist
TEST OK
test file tv_out removed

============================================
Running ../nad/testdatumfile using ../src/cs2cs:
============================================
doing tests into file td_out, please wait
Rel. 5.0.0, March 1st, 2018
<lt-cs2cs>: while processing file: <stdin>, line 1
pj_transform(): failed to load datum shift file
Rel. 5.0.0, March 1st, 2018
<lt-cs2cs>: while processing file: <stdin>, line 2
pj_transform(): failed to load datum shift file
Rel. 5.0.0, March 1st, 2018
<lt-cs2cs>: while processing file: <stdin>, line 1
pj_transform(): failed to load datum shift file
Rel. 5.0.0, March 1st, 2018
<lt-cs2cs>: while processing file: <stdin>, line 2
pj_transform(): failed to load datum shift file
Rel. 5.0.0, March 1st, 2018
<lt-cs2cs>: while processing file: <stdin>, line 3
pj_transform(): failed to load datum shift file
Rel. 5.0.0, March 1st, 2018
<lt-cs2cs>: while processing file: <stdin>, line 4
pj_transform(): failed to load datum shift file
Rel. 5.0.0, March 1st, 2018
<lt-cs2cs>: while processing file: <stdin>, line 1
pj_transform(): failed to load datum shift file
Rel. 5.0.0, March 1st, 2018
<lt-cs2cs>: while processing file: <stdin>, line 2
pj_transform(): failed to load datum shift file
Rel. 5.0.0, March 1st, 2018
<lt-cs2cs>: while processing file: <stdin>, line 3
pj_transform(): failed to load datum shift file
Rel. 5.0.0, March 1st, 2018
<lt-cs2cs>: while processing file: <stdin>, line 4
pj_transform(): failed to load datum shift file
diff td_out with td_out.dist
3,4c3,4
< 111d00'00.000"W 44d00'00.000"N 0.0    *       * 0.000
< 111d00'00.000"W 39d00'00.000"N 0.0    *       * 0.000
---
> 111d00'00.000"W 44d00'00.000"N 0.0    111d0'3.085"W   43d59'59.756"N 0.000
> 111d00'00.000"W 39d00'00.000"N 0.0    111d0'2.604"W   38d59'59.912"N 0.000
18,25c18,25
< -5.5 52.0     *       * 0.000000000000
< -5.5000000000001 52.0000000000001     *       * 0.000000000000
< -5.4999 51.9999       *       * 0.000000000000
< -5.5001 52.0  *       * 0.000000000000
< -5.5 52.0     *       * 0.000000000000
< -5.5000000000001 52.0000000000001     *       * 0.000000000000
< -5.4999 51.9999       *       * 0.000000000000
< -5.5001 52.0  *       * 0.000000000000
---
> -5.5 52.0     -5.501106465528 51.999890470284 0.000000000000
> -5.5000000000001 52.0000000000001     -5.501106465529 51.999890470284 0.000000000000
> -5.4999 51.9999       -5.501006458305 51.999790470257 0.000000000000
> -5.5001 52.0  -5.500100000000 52.000000000000 0.000000000000
> -5.5 52.0     -5.498893534472 52.000109529716 0.000000000000
> -5.5000000000001 52.0000000000001     -5.498893534472 52.000109529717 0.000000000000
> -5.4999 51.9999       -5.498793541695 52.000009529743 0.000000000000
> -5.5001 52.0  -5.500100000000 52.000000000000 0.000000000000

PROBLEMS HAVE OCCURRED
test file td_out saved

make[2]: *** [check-local] Error 100
make[2]: Leaving directory `/home/mtoews/src/proj.4/nad'
make[1]: *** [check-am] Error 2
make[1]: Leaving directory `/home/mtoews/src/proj.4/nad'
make: *** [check-recursive] Error 1

And from CMake

$ ctest
Test project /home/mtoews/src/proj.4/BUILD
      Start  1: test27
 1/29 Test  #1: test27 ...........................   Passed    2.26 sec
      Start  2: test83
 2/29 Test  #2: test83 ...........................   Passed    1.93 sec
      Start  3: testvarious
 3/29 Test  #3: testvarious ......................   Passed    0.93 sec
      Start  4: geodesic-test
 4/29 Test  #4: geodesic-test ....................   Passed    0.01 sec
      Start  5: Builtins
 5/29 Test  #5: Builtins .........................   Passed    0.07 sec
      Start  6: Builtins2
 6/29 Test  #6: Builtins2 ........................   Passed    0.06 sec
      Start  7: Axisswap
 7/29 Test  #7: Axisswap .........................   Passed    0.01 sec
      Start  8: Deformation
 8/29 Test  #8: Deformation ......................   Passed    0.01 sec
      Start  9: Ellipsoid
 9/29 Test  #9: Ellipsoid ........................   Passed    0.01 sec
      Start 10: GDA
10/29 Test #10: GDA ..............................   Passed    0.01 sec
      Start 11: 4D-API-cs2cs-style
11/29 Test #11: 4D-API-cs2cs-style ...............   Passed    0.03 sec
      Start 12: DHDN_ETRS89
12/29 Test #12: DHDN_ETRS89 ......................***Failed    0.01 sec
      Start 13: GIGS-5101.1-jhs
13/29 Test #13: GIGS-5101.1-jhs ..................   Passed    0.14 sec
      Start 14: GIGS-5101.2-jhs
14/29 Test #14: GIGS-5101.2-jhs ..................   Passed    0.07 sec
      Start 15: GIGS-5101.3-jhs
15/29 Test #15: GIGS-5101.3-jhs ..................   Passed    0.07 sec
      Start 16: GIGS-5101.4-jhs-etmerc
16/29 Test #16: GIGS-5101.4-jhs-etmerc ...........   Passed    0.06 sec
      Start 17: GIGS-5102.1
17/29 Test #17: GIGS-5102.1 ......................   Passed    0.13 sec
      Start 18: GIGS-5103.1
18/29 Test #18: GIGS-5103.1 ......................   Passed    0.06 sec
      Start 19: GIGS-5103.2
19/29 Test #19: GIGS-5103.2 ......................   Passed    0.04 sec
      Start 20: GIGS-5103.3
20/29 Test #20: GIGS-5103.3 ......................   Passed    0.04 sec
      Start 21: GIGS-5105.2
21/29 Test #21: GIGS-5105.2 ......................   Passed    0.09 sec
      Start 22: GIGS-5106
22/29 Test #22: GIGS-5106 ........................   Passed    0.07 sec
      Start 23: GIGS-5107
23/29 Test #23: GIGS-5107 ........................   Passed    0.03 sec
      Start 24: GIGS-5109
24/29 Test #24: GIGS-5109 ........................   Passed    0.03 sec
      Start 25: GIGS-5111.1
25/29 Test #25: GIGS-5111.1 ......................   Passed    0.07 sec
      Start 26: GIGS-5112
26/29 Test #26: GIGS-5112 ........................   Passed    0.02 sec
      Start 27: GIGS-5113
27/29 Test #27: GIGS-5113 ........................   Passed    0.02 sec
      Start 28: GIGS-5201
28/29 Test #28: GIGS-5201 ........................***Failed    0.03 sec
      Start 29: GIGS-5208
29/29 Test #29: GIGS-5208 ........................   Passed    0.06 sec

93% tests passed, 2 tests failed out of 29

Total Test time (real) =   6.46 sec

The following tests FAILED:
         12 - DHDN_ETRS89 (Failed)
         28 - GIGS-5201 (Failed)
Errors while running CTest

I'm rather new to ctest, so I'm not sure how to gather more information on the 2 failed tests.

@kbevers

This comment has been minimized.

Member

kbevers commented Mar 20, 2018

Try: ctest -VV -R DHDN_ETRS89 and ctest -VV -R GIGS-5201.

Your errors are likely due to missing grid. Have you pointed PROJ_LIB towards a dir with the grids?

@kbevers

This comment has been minimized.

Member

kbevers commented Mar 20, 2018

Closing since the original issue is fixed.

@kbevers kbevers closed this Mar 20, 2018

@mwtoews

This comment has been minimized.

Contributor

mwtoews commented Mar 21, 2018

@kbevers thanks for the ctest tips! This was probably some PROJ_LIB issue, as I get the same errors from CC=gcc.

@kbevers

This comment has been minimized.

Member

kbevers commented Mar 21, 2018

That's what I figured

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment