easybuild-easyblocks icon indicating copy to clipboard operation
easybuild-easyblocks copied to clipboard

Set CMake hints for path to Python installation if it's included as a dependency (when using CMake >= 3.12)

Open Flamefire opened this issue 10 months ago • 9 comments

(created using eb --new-pr)

Flamefire avatar Apr 05 '24 14:04 Flamefire

Test report by @Flamefire

Overview of tested easyconfigs (in order)

  • SUCCESS GROMACS-2020-foss-2019b.eb
  • SUCCESS GROMACS-2021.3-foss-2021a.eb
  • SUCCESS GROMACS-2021.5-foss-2021b.eb
  • SUCCESS GROMACS-2021-foss-2020b.eb
  • SUCCESS GROMACS-2023.1-foss-2022a.eb
  • SUCCESS GROMACS-2023.3-foss-2022a.eb
  • SUCCESS GROMACS-2023.3-foss-2023a.eb
  • SUCCESS GROMACS-2024.1-foss-2023b.eb

Build succeeded for 8 out of 8 (8 easyconfigs in total) n1459 - Linux RHEL 8.7 (Ootpa), x86_64, Intel(R) Xeon(R) Platinum 8470 (icelake), Python 3.8.13 See https://gist.github.com/Flamefire/d6a8736ee2f5f5cb16b22896fbb80ac2 for a full test report.

Flamefire avatar Apr 05 '24 15:04 Flamefire

@Micket Anything required here?

Flamefire avatar Apr 25 '24 13:04 Flamefire

@boegel ping

Flamefire avatar Jun 06 '24 12:06 Flamefire

Is this included in new Easybuild v4.9.2?

beeeback avatar Jun 25 '24 13:06 beeeback

Is this included in new Easybuild v4.9.2?

No, this is not even included in the develop branch yet. Otherwise the status on top of this page would say "Merged" instead of "Open"

Flamefire avatar Jun 25 '24 13:06 Flamefire

Looks fine, but this should be tested a bit more broadly than just with GROMACS imho.

Now's a good time to merge this, assuming that the next release will be EasyBuild 5.0, since I wouldn't be surprised if there's some fallout because of this (see #3088, which also shouldn't cause trouble, but I believe it did somewhere)

boegel avatar Jul 03 '24 14:07 boegel

Looks fine, but this should be tested a bit more broadly than just with GROMACS imho.

GROMACS was used because that is where it fails without this.

I wouldn't be surprised if there's some fallout because of this

This one is much less likely to cause fallout as we basically tell CMake: "Use this!" If it did use anything else before it was (very likely) just wrong and if it did not, this doesn't change anything.

Flamefire avatar Jul 03 '24 14:07 Flamefire

@Flamefire Do we fully understand why what we have now fails with GROMACS? If so, can we document this somewhere (either in a dedicated issue, or in here)?

boegel avatar Jul 03 '24 16:07 boegel

@Flamefire Do we fully understand why what we have now fails with GROMACS? If so, can we document this somewhere (either in a dedicated issue, or in here)?

This is an accompanying PR to https://github.com/easybuilders/easybuild-easyblocks/pull/3283 after a report and discussion in Slack

Basically GROMACS changed the default Python search (by CMake) to prefer a virtual env supplied Python. So CMake picked up the Python used for EasyBuild when that was installed in a virtual env, instead of the Python used as a dependency. For GROMACS specifically we can workaround by passing -DPython3_FIND_VIRTUALENV=STANDARD (which might be a useful default for CMakeMake too), see #3283

This is an alternative that also works by passing the Path to the Python root we want to use to CMake. This way it will consider this first before going through the other defaults (like the active virtual env)

Both PRs fix the issue for GROMACS.

This one should be more reliable especially for CMake options like "prefer virtual env" or "prefer the highest version you find" which we dealt with in the past. So less surprises with this.

Flamefire avatar Jul 04 '24 07:07 Flamefire

There just was an issue reported on Slack where OPENFOAM found a system Python:

CMake Error at /cluster/software/CMake/3.23.1-GCCcore-11.3.0/share/cmake-3.23/Modules/FindPackageHandleStandardArgs.cmake:230 (message):
  Could NOT find Python3 (missing: Python3_INCLUDE_DIRS Development.Module)
  (found suitable version "3.11.9", minimum required is "3.10")
Call Stack (most recent call first):
  /cluster/software/CMake/3.23.1-GCCcore-11.3.0/share/cmake-3.23/Modules/FindPackageHandleStandardArgs.cmake:594 (_FPHSA_FAILURE_MESSAGE)
  /cluster/software/CMake/3.23.1-GCCcore-11.3.0/share/cmake-3.23/Modules/FindPython/Support.cmake:3181 (find_package_handle_standard_args)
  /cluster/software/CMake/3.23.1-GCCcore-11.3.0/share/cmake-3.23/Modules/FindPython3.cmake:490 (include)
  /cluster/software/ParaView/5.10.1-foss-2022a-mpi/lib64/cmake/paraview-5.10/vtk/VTK-vtk-module-find-packages.cmake:538 (find_package)
  /cluster/software/ParaView/5.10.1-foss-2022a-mpi/lib64/cmake/paraview-5.10/vtk/vtk-config.cmake:150 (include)
  /cluster/software/ParaView/5.10.1-foss-2022a-mpi/lib64/cmake/paraview-5.10/paraview-config.cmake:169 (find_package)
  CMakeLists.txt:7 (FIND_PACKAGE)

So OPENFOAM depends on Paraview which depends on Python but during configure of OPENFOAM ParaView is searched for which searches for Python and finds the system version (even though I'd expect our -DCMAKE_POLICY_DEFAULT_CMP0094=NEW) to avoid that.
As Python is not a direct dependency of OPENFOAM the previous implementation of this wouldn't have added the hints. I hence changed it to always add them when Python is a dependency.

Flamefire avatar Jul 31 '24 07:07 Flamefire