search

QMCPACK / qmcpack

Main repository for QMCPACK, an open-source production level many-body ab initio Quantum Monte Carlo code for computing the electronic structure of atoms, molecules, and solids with full performance portable GPU support
http://www.qmcpack.org
Other
306 stars 139 forks source link

Revise variables passing from ctest to cmake #350

Open ye-luo opened 7 years ago

ye-luo commented 7 years ago

In order to avoid too long ctest command line, the way to pass options from ctest to cmake needs to be revised.

See the exploration in PR #347

HDF5_ROOT is mananged by FindHDF5 module and used as environment variable only. It can be passed in as environment variable. QMC_COMPLEX is managed by our CMakeLists, it has to be passed by -D explicitly.

PDoakORNL commented 6 years ago

So are we actually hitting ARG_MAX to ctest or is this an aesthetics thing? Because you're losing information that will end up being easily visible in cdash and its database when you use environment variables over cmake -D's. I'd prefer to have all the dependencies HDF5, MKL, BOOST, etc. passed in explicitly and recorded in the cdash database. (scroll down to the whole cmake command)[https://cdash.qmcpack.org/CDash/buildSummary.php?buildid=28410] but you don't get the environment (at least by default).

For the end user the opposite is probably optimal, everything comes from the environment and the module load qmcpack makes sure that a series of other modules are loaded so the environmental paths bring in the correct things. So our build should support this when possible.

prckent commented 6 years ago

CMake/ctest_script.cmake has many sections like the excerpt below. This is not scalable and is currently the only way to route options to the cmake line when ctest is running the script and driving the build.

IF ( QMC_COMPLEX )
  SET( CTEST_OPTIONS "${CTEST_OPTIONS};-DQMC_COMPLEX='${QMC_COMPLEX}'" )
ENDIF()
prckent commented 6 years ago

I agree with the point about cdash browsability. This is very helpful and not something to lose.

PDoakORNL commented 6 years ago

But we can definitely come up with something more scalable than the above.

ye-luo commented 6 years ago

Why not just put the flags needed by the cmake invoked by ctest via CTEST_OPTIONS directly. It is scalable and captured by cdash.

ctest -DCTEST_OPTIONS="-DCMAKE_CXX_FLAGS=YYY;-DQMC_COMPLEX=ON"

prckent commented 3 years ago

Our current strategy seems OK imo. Reopen if needed.