Skip to content

Ignore Python from virtualenvs in GROMACS configure via -DPython3_FIND_VIRTUALENV=STANDARD#3283

Merged
ocaisa merged 3 commits intoeasybuilders:developfrom
Flamefire:20240405162525_new_pr_gromacs
Sep 18, 2024
Merged

Ignore Python from virtualenvs in GROMACS configure via -DPython3_FIND_VIRTUALENV=STANDARD#3283
ocaisa merged 3 commits intoeasybuilders:developfrom
Flamefire:20240405162525_new_pr_gromacs

Conversation

@Flamefire
Copy link
Copy Markdown
Contributor

@Flamefire Flamefire commented Apr 5, 2024 •
edited
Loading

(created using eb --new-pr)

As reported in Slack GROMACS may pick up Python from an active virtualenv (e.g. when EasyBuild is installed in a virtualenv) instead of from the Python module

This is caused by GROMACS explicitly preferring that:

if(NOT Python3_FIND_VIRTUALENV)
    # We advocate using Python venvs to manage package availability, so by default
    # we want to preferentially discover user-space software.
    set(Python3_FIND_VIRTUALENV FIRST)
endif()

This happens when find_package(Python3) is used which is the case for version 2021 and up. The current self.cfg.update('configopts', "-DPYTHON_EXECUTABLE=%s" % bin_python) only applies to usages of find_package(PythonInterp ...) which are still present (e.g. in the gtest submodule) but not used exclusively as they are deprecated by CMake 3.12+ in favor of FindPython/2/3

This change avoids it picking up the virtualenv by setting the CMake variable which avoids GROMACS changing it.

Especially together with #3282 the intended Python should be picked up.

I also did a quick Search&Replace to avoid constructing LooseVersion(self.version) many times in this function.

@Flamefire
Copy link
Copy Markdown
Contributor Author

Flamefire commented Apr 5, 2024

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.

@boegel boegel added the bug fix label Apr 24, 2024
@boegel boegel added this to the release after 4.9.1 milestone Apr 24, 2024
boegel
boegel approved these changes Apr 24, 2024
View reviewed changes
Copy link
Copy Markdown
Member

@boegel boegel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@boegel boegel changed the title Ignore Python from virtualenvs in GROMACS configure Ignore Python from virtualenvs in GROMACS configure via -DPython3_FIND_VIRTUALENV=STANDARD Apr 24, 2024
@boegel boegel modified the milestones: 4.9.2, release after 4.9.2 Jun 6, 2024
@Flamefire Flamefire mentioned this pull request Jul 4, 2024
Closed
@boegel boegel modified the milestones: 4.9.3, release after 4.9.3 Sep 11, 2024
@bedroge
Copy link
Copy Markdown
Contributor

bedroge commented Sep 17, 2024

I also just ran into this issue when trying to make/test an easyconfig for GROMACS 2024.3. Initially the configure step showed this:

-- Found Python3: /home/bob/venvs/eb/bin/python3.12 (found suitable version "3.12.3", minimum required is "3.7") found components: Interpreter Development Development.Module Developmen
t.Embed

and it failed in some test step because gmxapi was built for this wrong Python version.

Then I used the fix from this PR, and it picked up the right Python (which also allowed the tests to run):

-- Found Python3: /data/apps/software/Python/3.11.5-GCCcore-13.2.0/bin/python3.11 (found suitable version "3.11.5", minimum required is "3.7") found components: Interpreter Development Development.Module Development.Embed

@boegel was there any reason why you hadn't hit the merge button after having approved the changes?

@bedroge bedroge mentioned this pull request Sep 17, 2024
Merged
@ocaisa
Copy link
Copy Markdown
Member

ocaisa commented Sep 18, 2024

Tested in easybuilders/easybuild-easyconfigs#21430

@ocaisa ocaisa merged commit 3040e10 into easybuilders:develop Sep 18, 2024
@Flamefire Flamefire deleted the 20240405162525_new_pr_gromacs branch September 18, 2024 10:59
@boegel boegel modified the milestones: release after 4.9.3, 4.9.4 Sep 19, 2024
@Crivella Crivella mentioned this pull request Sep 26, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@boegel boegel boegel approved these changes

@ocaisa ocaisa ocaisa approved these changes

Assignees

No one assigned

Labels

bug fix

Projects

None yet

Milestone

4.9.4

Development

Successfully merging this pull request may close these issues.

4 participants

@Flamefire @bedroge @ocaisa @boegel