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 CMake PROCESSORS properties #757

Merged
merged 1 commit into from
Jun 18, 2024

Conversation

LecrisUT
Copy link
Contributor

This property will allow to properly parallelize ctest -j calls

@jenkins-cscs
Copy link
Collaborator

Can one of the admins verify this patch?

@LecrisUT LecrisUT force-pushed the cmake/test-processors branch from 7b98cf0 to 15741a8 Compare January 30, 2024 09:43
@LecrisUT
Copy link
Contributor Author

Small note about TEST_MPI_RANKS. It should be fixed to 2 instead of auto in order to better take advantage of ctest parallelization

@alazzaro
Copy link
Member

alazzaro commented Feb 9, 2024

@LecrisUT Thanks for this PR. I like the idea to default the number of ranks to 2. Most of the tests are really intended for max 4 ranks in general... Still, it will be good to provide a way to cap the processors to the max available. I will work on that next week.

@LecrisUT
Copy link
Contributor Author

LecrisUT commented Feb 9, 2024

Still, it will be good to provide a way to cap the processors to the max available

This is typically done via ctest -j $(nproc)

@hfp
Copy link
Member

hfp commented May 16, 2024

If this PR targets the next release of CP2K/DBCSR, it would be the right time to finish/conclude it.

@LecrisUT
Copy link
Contributor Author

Seems like I may need this PR for #759. Tests are timing out due to parallelization there

Signed-off-by: Cristian Le <[email protected]>
@LecrisUT LecrisUT force-pushed the cmake/test-processors branch from 15741a8 to 5df60ee Compare May 17, 2024 11:43
@alazzaro
Copy link
Member

Small note about TEST_MPI_RANKS. It should be fixed to 2 instead of auto in order to better take advantage of ctest parallelization

I've changed the default to be 2, see d8116fd

@alazzaro alazzaro merged commit d332b30 into cp2k:develop Jun 18, 2024
22 checks passed
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.

5 participants