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

Enable CMake testing #28

Merged
merged 8 commits into from
Oct 10, 2023
Merged

Enable CMake testing #28

merged 8 commits into from
Oct 10, 2023

Conversation

wo80
Copy link
Contributor

@wo80 wo80 commented Oct 9, 2023

This pull request enables testing features:

  • All examples now return an exit code of EXIT_SUCCESS or EXIT_FAILURE depending on whether the number of converged eigenvalues is less than the number of requested eigenvalues.
  • The CMake option ENABLE_EXAMPLES was renamed to ENABLE_TESTS and tests are configured in examples/CMakeLists.txt.
  • A couple of problems with SuperLU have been fixed to make the tests pass.
  • A Github workflow with tests passing on Ubuntu and Mac OS has been added.

I compared the Github and Travis logs for the Mac OS build. Both use

  • macOS 12
  • AppleClang 14.0
  • Xcode 14.2
  • MacOSX13.1.sdk/System/Library/Frameworks/Accelerate.framework

I haven't enabled ctest on Travis, but using the default BLAS/LAPACK provider (Accelerate.framework) some tests on the Github workflow failed (see https://github.com/wo80/arpackpp/actions/runs/6453199699/job/17516350173). I had to explicitly specify BLA_VENDOR=OpenBLAS and CMAKE_PREFIX_PATH="/usr/local/opt/lapack;/usr/local/opt/openblas".

So in case you don't want to ditch the Travis job completely, it needs to be updated accordingly. Not sure what's the cause of the problem, though. Since I don't have a Mac, I cannot debug it.

wo80 added 6 commits October 9, 2023 07:41
This is done by simply returning EXIT_SUCCESS or EXIT_FAILURE depending on whether the number of converged eigenvalues is less than the number requested eigenvalues.
CMake can unpack zip archives by default.
The constructor for rectangular matrices did not allocate the SuperMatrix A, though it sets defined=true. This led to a memory access violation in ClearMem().

Additionally, the DataOk() method did not work correctly for non-square matrices.
@wo80
Copy link
Contributor Author

wo80 commented Oct 9, 2023

We might also want to get rid of file(GLOB ...) in the test setup. This would allow more control setting up the tests that rely on input test data (specifically the tests in the harwell folder).

@m-reuter
Copy link
Owner

m-reuter commented Oct 9, 2023

Nice! Happy to get rid of Travis. Not sure what you mean about how getting rid of glob helps?

@wo80
Copy link
Contributor Author

wo80 commented Oct 10, 2023

CMake doesn't recommend using GLOB, see CMake docs, but since everything is working fine, I guess we can ignore that.

The problem with the Harwell examples (now that they are run as tests) is, that they require input data. Doing, for example,

file(GLOB harwell_complex harwell/complex/*.cc)

we get both hcompstd.cc and hcompgen.cc, but those two files require different command line arguments, so we cannot use GLOB here. I fixed that in the latest commits.

This should be ready to merge. I haven't removed the .travis.yml file. Please check if anything has to be done on the Travis side, first.

@m-reuter m-reuter merged commit c9d901a into m-reuter:master Oct 10, 2023
@wo80 wo80 deleted the feature/ctest branch October 10, 2023 09:48
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.

2 participants