Skip to content

Conversation

@msimberg
Copy link
Collaborator

It's set by default on daint (to a different value).

Only reason to keep it here is if we expect many users to be running CP2K on other clusters, but then I'd rather make sure CUDA_CACHE_PATH is set by default there instead?

@msimberg msimberg requested review from RMeli and bcumming March 14, 2025 12:29
@github-actions
Copy link

preview available: https://docs.tds.cscs.ch/43

@bcumming
Copy link
Member

I will leave it to @RMeli to comment definitively.

I think that documenting this variable is important - even if it is defined by default on the system.

@RMeli
Copy link
Member

RMeli commented Mar 14, 2025

As I expressed elsewhere, I think it would be beneficial to have a central (and by-platform) documentation of all these variables that are automatically set by us, so that users are aware.

If we want to be consistent with MPICH_GPU_SUPPORT_ENABLED not being there, we should probably remove this one too.

However, an alternative that is probably nicer for the end-user would be to explicitly set both (doesn't hurt), and add a note that they are here only for documentation purposes and they are already automatically set in the system? In such case I would do what @msimberg suggest and set it to the actual default /dev/shm/$USER/cuda_cache/.

@RMeli
Copy link
Member

RMeli commented Mar 14, 2025

Sorry @msimberg, I misread your suggestion. I agree CUDA_CACHE should be set on other clusters too, but documenting the default here doesn't hurt IMO (and if the default changes, it is not a big issue to set it to /dev/shm/$USER/cuda_cache/, it will do the job).

…launch scripts despite them being set on the HPC platform by default
@abussy
Copy link
Collaborator

abussy commented Mar 14, 2025

I agree with @RMeli, leaving it in the documentation does not hurt, even if there is a reasonable default on each cluster. A lot of time would have been saved on the validation of Alps last summer if this kind of stuff had remained in the docs (IIRC, something similar happened when Daint XC was tested, but the knowledge was lost).

@github-actions
Copy link

preview available: https://docs.tds.cscs.ch/43

@msimberg
Copy link
Collaborator Author

All your comments are good! I just pushed a change to restore CUDA_CACHE_PATH to the real value set on daint, and to set MPICH_GPU_SUPPORT_ENABLED explicitly. I added notes in the annotations.

Do you have a preference for keeping the other box mentioning MPICH_GPU_SUPPORT_ENABLED? Or adding another explicit box mentioning CUDA_CACHE_PATH?

I added a link to the new "GPU-aware MPI" section from the CP2K info box (35f48db).

@github-actions
Copy link

preview available: https://docs.tds.cscs.ch/43

@RMeli
Copy link
Member

RMeli commented Mar 14, 2025

Do you have a preference for keeping the other box mentioning MPICH_GPU_SUPPORT_ENABLED? Or adding another explicit box mentioning CUDA_CACHE_PATH?

At this point I'd go for consistency and add another mentioning CUDA_CACHE_PATH, if it's not too much trouble. Thanks for doing this!

@github-actions
Copy link

preview available: https://docs.tds.cscs.ch/43

@msimberg
Copy link
Collaborator Author

At this point I'd go for consistency and add another mentioning CUDA_CACHE_PATH, if it's not too much trouble.

Absolutely worth it. I pushed a change, mind having a look? I used slightly different wording in the new info box compared to the code annotation. Not sure if it's worth just copy pasting the two to be exactly the same...

Copy link
Member

@RMeli RMeli left a comment

Choose a reason for hiding this comment

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

LGTM, thanks!

@RMeli RMeli changed the title Remove explicit mention of CUDA_CACHE_PATH in CP2K docs Better document CUDA_CACHE_PATH in CP2K docs Mar 17, 2025
@RMeli RMeli merged commit 80adbb5 into main Mar 17, 2025
1 check passed
@RMeli RMeli deleted the cp2k-no-cuda-cache-path branch March 17, 2025 13:45
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