search

lanl / nuDustC

Nucleating Dust Code in C++
BSD 3-Clause "New" or "Revised" License
0 stars 1 forks source link

possible syntax mixed up on option(....) in CMakeLists.text? #5

Open guadabsb15 opened 2 months ago

guadabsb15 commented 2 months ago

Looking at the lines 19-20 on the CMakeLists.txt 

option(NUDUSTC_ENABLE_OPENMP ON "Use OpenMP for cell/particle parallelization")
option(NUDUSTC_ENABLE_MPI ON "Use MPI for cell/particle parallelization")
option(NUDUSTC_USE_SUNDIALS OFF "Use sundials CVODE integrator")

and the cmake documentation https://cmake.org/cmake/help/v3.23/command/option.html , the parameters inside the parenthesis might be in the wrong order. Therefore MPI and OpenMP support are probably not really being set ON by default. These lines should look like option(NUDUSTC_ENABLE_OPENMP "Use OpenMP for cell/particle parallelization" ON) similarly to line 42 in the CMakeLists.txt

With the syntax as it is, after someone does cmake .. in the build directory, those three options are OFF. One can check that in the CMakeCache.txt inside the build directory.

I usually manually change and check the values using the terminal based "GUI" ccmake, so I had not noticed this behavior before.

guadabsb15 commented 1 month ago

This issue is also related to JOSS review https://github.com/openjournals/joss-reviews/issues/6637

steven-murray commented 1 month ago

This is also happening for me, so I would definitely recommend updating it. I have very little experience with cmake, so I am a good candidate for finding potential errors that only inexperienced users will come across. Interestingly though, even though the MPI is turned OFF by default (though it looks like it should be ON), my compilation still complains about not finding the MPI libraries...