CMake: Introduce CMake dependent options and print build summary#7626
CMake: Introduce CMake dependent options and print build summary#7626Growl1234 wants to merge 10 commits into
Conversation
|
There is one RFC about how we should detect and show the version of MPI, as existing version detection logic seems invalid. I currently choose to show the real MPI-standard what an MPI implementation follows, as this is generally available for any MPI implementations through a single
However, one concern is that users might not be used to it, so might there be other easier way? |
b2be2ef to
ef85e98
Compare
|
Side note: an inconsistent change is made here: If enable LibRI support without MPI, compilation will report error: Same as Also, require out-of-source CMake build as in-source is dangerous and might affect the real source code in the build process. |
4892519 to
c6a9c5f
Compare
- Output crucial information as a build summary - Make MPI implementation detection valid (at least for non-cross build) - Use version of MPI standard one MPI implementation follows [RFC]
These warnings might be treated as noise, and can be removed once devs and downstream are adapted to it.
AsTonyshment
left a comment
There was a problem hiding this comment.
Thanks for introducing cmake_dependent_option; it makes the option dependencies much clearer. One concern is that an explicitly requested feature may now be silently disabled. For example:
-DENABLE_NCCL_PARALLEL_DEVICE=ON -DENABLE_MPI=OFFCMake will force ENABLE_NCCL_PARALLEL_DEVICE to OFF but still configure successfully. Users may therefore assume NCCL support was enabled unless they carefully inspect the build summary.
I noticed that you have already raised this upstream:
CMakeDependentOption: Add warning when overriding value #27960
Since generic warnings inside cmake_dependent_option may be noisy, could ABACUS add targeted warnings for hard requirements such as NCCL, CUDA-aware MPI, LibRI, and PEXSI? For example:
ENABLE_NCCL_PARALLEL_DEVICE was requested, but USE_CUDA and ENABLE_MPI are not enabled. Disabling it.
This would preserve the cleaner dependency logic while making overridden user requests visible.
| cmake_dependent_option(ENABLE_CUSOLVERMP "Enable cusolvermp" OFF "USE_CUDA" OFF) | ||
| cmake_dependent_option(ENABLE_CUBLASMP "Enable cublasmp" OFF "ENABLE_CUSOLVERMP" OFF) |
There was a problem hiding this comment.
Although this naming predates this PR, could we use the official capitalization consistently here: cusolvermp -> cuSOLVERMp and cublasmp -> cuBLASMp?
Honestly I didn't find a proper way to do it without introducing another weird and redundant helper (no reason to increase the complication just for options clarification). Introducing a modified Regarding the CMake issue, the upstream discussion is still ongoing and has not yet reached a conclusion. Therefore, further messages from the upstream developers can still be expected. That said, users and downstream should definitely be clear what they are doing anyways, whether the warning is added or not... |
Notes:
Out-of-source CMake build is required.
ENABLE_LIBRI/PEXSIrequiresENABLE_MPI(see #7626 (comment)).USE_CUDA_MPIrequresUSE_CUDACUBLASMP/NCCLwill quietly turn off when requirement not satisfied, with no error reportedThis PR introduces
cmake_dependent_optionwith intention to make the dependency handling logic clearer from the beginning of CMake logic.Example:
This means
EXX_DEVis available only whenENABLE_LIBRIis set toON, and defaults toOFF; ifENABLE_LIBRI=OFF, theEXX_DEVoption will be forced to be off. So if one passes-DENABLE_LIBRI=OFF -DEXX_DEV=ONto CMake (by mistake?), the actual resulting logic will be-DENABLE_LIBRI=OFF -DEXX_DEV=OFF. (One thing that might need to care is that this logic is done by CMake without any warnings; the comment# Options requiring ...is therefore added to remind users to know what they are doing.)This means
ENABLE_ELPAis available only when bothENABLE_LCAOandENABLE_MPIare set toON.