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 CTest test cases from existing tests (run_tests.sh) #20

Merged
merged 25 commits into from
Sep 20, 2024

Conversation

siggmo
Copy link
Collaborator

@siggmo siggmo commented Sep 8, 2024

This PR should not be merged before PR #19!

Adressing #9

CMake comes with a testing tool called CTest. This PR adds the existing test cases from test/run_tests.sh as CTest test cases. Instead of maintaining a dedicated test shell script, CTest allows to define the tests in the existing CMake build system and offers useful functions for testing like measuring execution time, collecting outputs, checking for assertions etc.

You can run the tests by calling ctest in the build directory. To see the full output, add the -VV flag.

Features

  • Add the four test cases LinearThermalIsotropic, LinearElasticIsotropic, PseudoPlasticLinearHardening and VonMisesPlasticLinearIsotropicHardening.
  • Use maximum possible number of MPI processes (more than 8 caused this error: "[ FANS3D_Grid ] ERROR: Please decrease the number of processes or increase the grid size to ensure that each process has at least 4 boxels in the x direction.").
  • HDF5 output files go into <build-dir>/test/, such that running the tests only creates/alters files in the build tree

Open points

  • No result checking! Should we add that?
  • Do we stay with max. number of MPI processes favoring execution time, or rather focus on reproducability and fix the number?

@siggmo siggmo marked this pull request as draft September 8, 2024 15:11
@siggmo
Copy link
Collaborator Author

siggmo commented Sep 8, 2024

Apparently MPI isn't able to obtain the required "slots" when running in the GitHub Action. Locally on my machine it worked... I have to investigate that further.

Here's the error:

Test project /FANS/build
    Start 1: LinearThermalIsotropic
1/4 Test #1: LinearThermalIsotropic ....................***Failed    0.04 sec
--------------------------------------------------------------------------
There are not enough slots available in the system to satisfy the 4
slots that were requested by the application:

  /FANS/build/FANS

Either request fewer slots for your application, or make more slots
available for use.

A "slot" is the Open MPI term for an allocatable unit where we can
launch a process.  The number of slots available are defined by the
environment in which Open MPI processes are run:

  1. Hostfile, via "slots=N" clauses (N defaults to number of
     processor cores if not provided)
  2. The --host command line parameter, via a ":N" suffix on the
     hostname (N defaults to 1 if not provided)
  3. Resource manager (e.g., SLURM, PBS/Torque, LSF, etc.)
  4. If none of a hostfile, the --host command line parameter, or an
     RM is present, Open MPI defaults to the number of processor cores

In all the above cases, if you want Open MPI to default to the number
of hardware threads instead of the number of processor cores, use the
--use-hwthread-cpus option.

Alternatively, you can use the --oversubscribe option to ignore the
number of available slots when deciding the number of processes to
launch.

Do you have any ideas what could be going wrong?

@siggmo siggmo linked an issue Sep 8, 2024 that may be closed by this pull request
@sanathkeshav
Copy link
Member

sanathkeshav commented Sep 9, 2024

"For example, GitHub Actions only have 2 cores available for Windows and Linux, yet 3 for macOS."

mentioned in: vercel/turborepo#761

can't request 4 cores for building or for testing that's all.

@siggmo
Copy link
Collaborator Author

siggmo commented Sep 9, 2024

"For example, GitHub Actions only have 2 cores available for Windows and Linux, yet 3 for macOS."

mentioned in: vercel/turborepo#761

can't request 4 cores for building or for testing that's all.

Ahh okay makes sense! I used the CMake function ProcessorCount to determine the available cores, but I guess then sth. is not working correctly with this function.

@IshaanDesai IshaanDesai added the enhancement New feature or request label Sep 10, 2024
@siggmo siggmo marked this pull request as ready for review September 16, 2024 16:40
@IshaanDesai IshaanDesai requested review from IshaanDesai and sanathkeshav and removed request for IshaanDesai September 17, 2024 11:55
Copy link
Member

@sanathkeshav sanathkeshav left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hehe

test/CMakeLists.txt Outdated Show resolved Hide resolved
@IshaanDesai IshaanDesai merged commit 28782a0 into develop Sep 20, 2024
7 checks passed
@IshaanDesai IshaanDesai deleted the feature/ctest-testcases branch September 20, 2024 18:49
@IshaanDesai IshaanDesai mentioned this pull request Sep 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Build and test FANS in a GitHub Action
3 participants