-
Notifications
You must be signed in to change notification settings - Fork 142
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
Make CMake GPU options compact #5250
Comments
I think this adds complexity and another layer we don't really need. What are you/we hoping to achieve with it? Unless we are providing a good and simple default setting for these different architectures, we will end up writing explicit instructions for each vendor and architecture anyway => we don't need this QMC_GPU feature since users will be copying from our documentation and provided installation scripts. Therefore, we don't need this additional layer. For comparison, GROMACS has https://manual.gromacs.org/current/install-guide/exotic.html and https://manual.gromacs.org/documentation/current/install-guide/index.html |
All the Our documentation can be significantly simplified as
Users learn one option instead of 4, basically like |
OK, on reflection lets try this or something close to it. I'll note we also need to specify the GPU architecture via QMC_GPU_ARCHS. I suggest that we set a default QMC_GPU based on this so that in practice most users won't need to do anything other than specify the architecture. e.g. the user puts QMC_GPU_ARCHS=gfx1234 and gets QMC_GPU=openmp;hip by default. Edit: Simpler still -- we could set defaults for ENABLE_OFFLOAD, ENABLE_CUDA, QMC_CUDA2HIP, ENABLE_SYCL based on QMC_GPU_ARCHS and avoid creating QMC_GPU entirely. Regular users therefore only have one setting to specify for GPU builds. The need to change these defaults is quite rare -- e.g. compiler people looking to optimize pure OpenMP offload, or test builds aiming to activate only one set of code paths. Recognizing Peter's comments in the prior closed issue, any explicit ENABLE_xyz settings should override the defaults implied by QMC_GPU_ARCHS (easy to implement). |
The default value of potentially interacting The following assumes Currently if What you suggested was to add another case that In summary, |
My old attempt was
QMC_GPU=no/NVIDIA/AMD/INTEL
and I no longer think it is a good simplification see discussion #3930My new suggestions is
-DQMC_GPU="openmp;cuda"
, translate to internally ENABLE_OFFLOAD=ON and ENABLE_CUDA=ON-DQMC_GPU="hip"
, translate to internally ENABLE_CUDA and QMC_CUDA2HIP@prckent @PDoakORNL what do you think?
The text was updated successfully, but these errors were encountered: