AtChem / AtChem2

Atmospheric chemistry box-model for the MCM
MIT License
58 stars 23 forks source link

Escape colon character in LAPACK_LIBS string #459

Closed kilicomu closed 3 years ago

kilicomu commented 3 years ago

The colon character is getting passed through to a make target unescaped, which causes the test for LAPACK libraries to fail.

kilicomu commented 3 years ago

Initially, I'm not sure how this change will affect the CI builds, so I'm opening in draft to see what happens when the LAPACK libs are visible to SUNDIALS.

codecov[bot] commented 3 years ago

Codecov Report

Merging #459 (9e7f516) into master (dd25104) will not change coverage. The diff coverage is n/a.

:exclamation: Current head 9e7f516 differs from pull request most recent head 85af443. Consider uploading reports for the commit 85af443 to get more accurate results Impacted file tree graph

@@           Coverage Diff           @@
##           master     #459   +/-   ##
=======================================
  Coverage   65.51%   65.51%           
=======================================
  Files          17       17           
  Lines        2047     2047           
=======================================
  Hits         1341     1341           
  Misses        706      706           
Flag Coverage Δ
build 52.23% <ø> (ø)
tests 82.81% <ø> (ø)
unittests 31.61% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.


Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update dd25104...85af443. Read the comment docs.

kilicomu commented 3 years ago

Doesn't look like escaping the colon has led to functional LAPACK. In my head this means that the build couldn't find the LAPACK libraries at the specified location, so I need to poke around in the build.

kilicomu commented 3 years ago

LAPACK is now functional in the CVODE build, but we're in the same position that I am on my system - undefined symbols in the CVODE libraries which propagate through to the AtChem2 build:

/usr/bin/ld: cvode/lib/libsundials_cvode.so: undefined reference to `dgbtrs_'
/usr/bin/ld: cvode/lib/libsundials_cvode.so: undefined reference to `dcopy_'
/usr/bin/ld: cvode/lib/libsundials_cvode.so: undefined reference to `dscal_'
/usr/bin/ld: cvode/lib/libsundials_cvode.so: undefined reference to `dgetrf_'
/usr/bin/ld: cvode/lib/libsundials_cvode.so: undefined reference to `dgbtrf_'
/usr/bin/ld: cvode/lib/libsundials_cvode.so: undefined reference to `dgetrs_'

(https://github.com/AtChem/AtChem2/pull/459/checks?check_run_id=3349186758#step:12:40)

I guess this means that CVODE isn't correctly linking to OpenBLAS, as these symbols are found in the OpenBLAS library (at least on my system - worth checking in the CI build...).

kilicomu commented 3 years ago

The symbols are definitely present in OpenBLAS lib: https://github.com/AtChem/AtChem2/pull/459/checks?check_run_id=3349428841#step:7:1

So, need to look at the SUNDIALS build to see what's going wrong.