Skip to content
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

Add Mac CI tests #97

Closed
wants to merge 10 commits into from
Closed

Add Mac CI tests #97

wants to merge 10 commits into from

Conversation

jfowkes
Copy link
Contributor

@jfowkes jfowkes commented Nov 10, 2022

We should also automatically test SPRAL on Mac.

@amontoison
Copy link
Member

@jfowkes, I found this today, it could be relevant for SPRAL (and GALAHAD): https://gist.github.com/scivision/d94bb10a01fa3b8ed0c9a93ee16318ba

@jfowkes
Copy link
Contributor Author

jfowkes commented Nov 11, 2022

@amontoison great will add an Intel test script!

@jfowkes
Copy link
Contributor Author

jfowkes commented Nov 11, 2022

@amontoison also what is your FC when compiling with Clang on Mac?

@amontoison
Copy link
Member

It's gfortran in the cross-compiler.

@jfowkes
Copy link
Contributor Author

jfowkes commented Nov 14, 2022

@amontoison ok will switch the workflow to gfortran, flang is still experimental...

@jfowkes
Copy link
Contributor Author

jfowkes commented Nov 14, 2022

Now we're getting a legitimate C++ compiler failure:

./src/ssids/cpu/kernels/assemble.hxx:230:35: error: no matching constructor for initialization of 'std::vector<int, PoolAllocInt>' (aka 'vector<int, BuddyAllocator<int, std::allocator<double>>>')
   std::vector<int, PoolAllocInt> map(n+1, PoolAllocInt(pool_alloc));
                                  ^   ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

@nimgould do you also run into this compiler failure with Clang?

@nimgould
Copy link
Contributor

nimgould commented Nov 14, 2022 via email

@jfowkes
Copy link
Contributor Author

jfowkes commented Nov 14, 2022

Looking at the error further, it appears that Clang can't find the C++ standard library vector implementation, which suggests an issue with finding the C++ standard library headers.

@nimgould
Copy link
Contributor

nimgould commented Nov 14, 2022 via email

@amontoison
Copy link
Member

amontoison commented Nov 14, 2022

@jfowkes
LLVM projects use -lc++ and not -lstdc++.
Maybe you just need to update these lines: https://github.com/ralna/spral/blob/master/configure.ac#L70-L78

@jfowkes
Copy link
Contributor Author

jfowkes commented Nov 14, 2022

@amontoison ahh yes you are right, that should do the trick!

@jfowkes
Copy link
Contributor Author

jfowkes commented Nov 14, 2022

@amontoison sadly that didn't seem to work, unfortunately I don't understand autotools enough to know if I'm doing it correctly or even if CXXLIB is being picked up anywhere.

@amontoison
Copy link
Member

@jfowkes
Copy link
Contributor Author

jfowkes commented Nov 14, 2022

I have tried with $(brew --prefix llvm@14)/bin/clang as suggested by the docs here:
https://github.com/actions/runner-images/blob/releases/macOS-11/20221028/images/macos/macos-11-Readme.md
However then configure is unable to find Metis for some reason.

@amontoison
Copy link
Member

amontoison commented Nov 14, 2022

Ok, let's try again with the flag -stdlib=libc++ instead of -lc++.

@jfowkes
Copy link
Contributor Author

jfowkes commented Nov 14, 2022

@amontoison no joy, -stdlib=libc++ is the default anyway and should not be required.

@amontoison
Copy link
Member

amontoison commented Nov 14, 2022

Hum, I don't know how to solve this issue.
We should drop clang / clang++ for now and just compile with gcc / g++ on all platforms.

@dpo
Copy link

dpo commented Nov 15, 2022

I don't think the problem is that std::vector is not available. The problem is that no std::vector constructor is found for the arguments given (Int and PoolAllocInt).

@dpo
Copy link

dpo commented Nov 15, 2022

You're missing -std=c++14 from the C++ flags. The allocator construct used here is rather new.

After that, there are other compilation issues. After fiddling for a while, this incantation worked for me:

./configure CC=clang CXX=clang++ F77=gfortran-12 FC=gfortran-12 CFLAGS="-g -O2 -Wall -I$(brew --prefix openblas)/include" CXXFLAGS="-g -O2 -Wall -std=c++14 -I$(brew --prefix openblas)/include" FCFLAGS="-g -O2 -Wall -pedantic" LIBS="-lc++ -L$(brew --prefix openblas)/lib -lopenblas" --with-metis="-L$(brew --prefix metis)/lib -lmetis"

I decided to brew install openblas to work around an error where cblas.h was not found. Even though it is present in some obscure system header location, I could never get the compiler to find it.

After all that, make check works, but there's a failing ssids test:

PASS: lsmr_test
PASS: rutherford_boeing_test
PASS: scaling_test
PASS: random_test
PASS: random_matrix_test
./test-driver: line 112: 15120 Abort trap: 6           "$@" >> "$log_file" 2>&1
FAIL: ssids_test
PASS: ssids_kernel_test
PASS: ssmfe_test
PASS: ssmfe_ciface_test
============================================================================
Testsuite summary for spral 2021.09.01-dev
============================================================================
# TOTAL: 9
# PASS:  8
# SKIP:  0
# XFAIL: 0
# FAIL:  1
# XPASS: 0
# ERROR: 0

@dpo
Copy link

dpo commented Nov 15, 2022

All is well if I replace clang and clang++ with gcc-12 and g++-12. All tests pass. That points to a bug in either the compiler, SSIDS or the SSIDS test.

@dpo
Copy link

dpo commented Nov 15, 2022

This almost works with the Intel compilers:

./configure CC=icc CXX=icc F77=ifort FC=ifort CFLAGS="-g -O2 -Wall" CXXFLAGS="-g -O2 -Wall -std=c++14" FCFLAGS="-g -O2 -Wall -pedantic" LIBS=-lc++ HWLOC_CFLAGS="-I$(brew --prefix hwloc)/include" HWLOC_LIBS="-L$(brew --prefix hwloc)/lib -lhwloc" --with-blas="-L$MKLROOT/lib -lmkl_intel_lp64 -lmkl_sequential -lmkl_core -lpthread -lm" --with-lapack="-L$MKLROOT/lib -lmkl_intel_lp64 -lmkl_sequential -lmkl_core -lpthread -lm" --with-metis="-L$(brew --prefix metis)/lib -lmetis"

However, the -Wall flag isn't right, and during make check, the compiler can't find cblas.h. There doesn't seem to be one in the MKL, so I'm not sure what the solution is. Perhaps an additional define that would #include "mkl.h" (or whatever) instead of cblas.h if configuring --with-mkl for instance.

There are couple additional hoops to jump through related to Homebrew when building on Apple Silicon. (It's not the case of the GitHub VMs.) I can explain or write instructions at some point if you're interested.

It would be good if the workflows tested a variety of compilers, as those also serve as documentation and can help users get the incantation right.

@jfowkes jfowkes changed the title Add Mac and Windows CI tests Add Mac CI tests Nov 16, 2022
@jfowkes
Copy link
Contributor Author

jfowkes commented Nov 16, 2022

@dpo thank you very much, that fixes the compilation issues!! Yeah finding C/C++ headers on modern macs is a real pain...

I will have a go later at creating macOS Intel CI tests, need to figure out the best way to install the Intel compilers (no apt on mac but probably pip is the easiest way). I assume that for your local tests you're sourcing /opt/intel/oneapi/setvars.sh to set up the Intel environment variables (including MKL). Btw it's -warn all for ifort (I presume for historical reasons).

I agree entirely that we should have proper CI tests for a variety of platforms and compilers. This has already shown us that SPRAL will only run correctly with gcc/gfortran as there are bugs when using all the other compilers.

@nimgould
Copy link
Contributor

(To be honest, I don't see issues with spral under ifort/icc and ifx/icx on Linux). Maybe its my lucky day

@nimgould
Copy link
Contributor

But, yes, CI on as many common platforms and compilers as possible

@jfowkes
Copy link
Contributor Author

jfowkes commented Nov 16, 2022

(To be honest, I don't see issues with spral under ifort/icc and ifx/icx on Linux). Maybe its my lucky day

@nimgould yes the issue with Intel on Linux is a sefgault with SSMFE (see #98)

@dpo
Copy link

dpo commented Nov 16, 2022

need to figure out the best way to install the Intel compilers (no apt on mac but probably pip is the easiest way).

See my SIFDecode/CUTEst/GALAHAD workflow where I install the Intel compilers using the "official" way.

@jfowkes jfowkes force-pushed the add-mac-windows-tests branch 2 times, most recently from 22eb098 to f83f9ff Compare November 16, 2022 16:40
@jfowkes
Copy link
Contributor Author

jfowkes commented Jul 17, 2023

Superseded by #113

@jfowkes jfowkes closed this Jul 17, 2023
@jfowkes jfowkes deleted the add-mac-windows-tests branch September 25, 2023 14:08
@jfowkes jfowkes mentioned this pull request Sep 27, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants