Skip to content

Making the CI compatible with a maintained python version (Reprise of #1350)#1363

Draft
Iximiel wants to merge 9 commits intoplumed:masterfrom
Iximiel:feature/py310work
Draft

Making the CI compatible with a maintained python version (Reprise of #1350)#1363
Iximiel wants to merge 9 commits intoplumed:masterfrom
Iximiel:feature/py310work

Conversation

@Iximiel
Copy link
Member

@Iximiel Iximiel commented Jan 21, 2026

Note

I will likely reopen this as a standard PR soon I have finished
The original PR is #1350

I am doing some researches on testing with python 3.10.

Since I am getting a segmentation fault, following this blogpost, I started trying to use pytest --collect-only to have a list of tests and then "bisect" it to find the culprit.

The problem is that instead of getting the list of error I get a segmentation fault...

So I dug a little by manipulating the test collection. Turns out that if I hide test_input_builder.py I do not get the segmentation fault in the collection phase (I still get it while running the tests, but that is a future me problem for now). This is strange because in #1350 it works (at least not in dbg mode).

I did some "manipulation" to test_input_builder.py and the culprit for the segmentation fault seems to be the initialization of the plumed.InputBuilder.

And this Is what I get when I try to run something similar directly in the interpreter:

>>> import plumed
>>> ib = plumed.InputBuilder()
+++ Loading the PLUMED kernel runtime +++
+++ PLUMED_KERNEL="/---/installdir/lib/libplumedKernel.so" +++

funnily this one below does not segfault, it loads the kernel twice (one for plmd, and one for guessing where to load the vim syntax file in ib) but goes smoothly

import plumed
plmd = plumed.Plumed()
ib = plumed.InputBuilder()

###EDIT

I am breaking this PR in multiple more focused pr:

@Iximiel Iximiel force-pushed the feature/py310work branch 3 times, most recently from 369b10c to a2fa8b4 Compare January 23, 2026 11:03
@Iximiel
Copy link
Member Author

Iximiel commented Jan 23, 2026

@Luthaf I moved the required python modules in a file within the pytorch package. (I need to check if the url can be specified within the requirements files). So that it is possible to change them without having to remember to change them also in the CI or in other exotic places. (and can be used as an hash for the pip cache in future). Are you ok with this?

@Luthaf
Copy link
Contributor

Luthaf commented Jan 23, 2026

I'm not sure this makes sense, these where dependencies for CI, not the actual code.

I pulled metatomic-torch so we can link against libmetatomic without having to build it first, and featomic-torch is only used in the tests for the metatomic action. So in both cases I don't think it makes sense to have these in src/pytorch/.

We could put them in .ci/requirements-torch.txt to centralize them, or have the torch requirement in src/pytorch and the CI specific requirements elsewhere

@Iximiel
Copy link
Member Author

Iximiel commented Jan 23, 2026

We could put them in .ci/requirements-torch.txt to centralize them, or have the torch requirement in src/pytorch and the CI specific requirements elsewhere

Separating ci requirements from actual requirements is a good idea. I would still prefer to encapsulate things the more possible in their own directory and to make change propagate automatically (see how it snowballed "removing" cython here). To avoid backtracking problems for "future-us" or for the next one that will have to touch the code.

I was thinking at something crunchy like:
requirements.txt:

# torch 2.7 above is the first one to use cxx11 ABI for the PyPI wheels
--extra-index-url=https://download.pytorch.org/whl/cpu
torch>=2.7

requirements_ci.txt:

--extra-index-url=https://download.pytorch.org/whl/cpu
-r requirements.txt
metatomic-torch>=0.1.3,<0.2
featomic-torch==0.7.0

(you can write the requirement file as it is the pip install command line and use all the possible options (-r,-i...))
Both files in the pytorch dir so that the second can include the first with no problems.

For installing the accelerated version, maybe it will be better to change the extra-index-url in the main requirements file

Now Cython will automatically installed in the build phase and will not
need to appear in the work environment

plumed.c and plumed.pyx will get the configure configure.ac treatment in
the next commit
@Iximiel
Copy link
Member Author

Iximiel commented Jan 29, 2026

@GiovanniBussi @gtribello here's the funny thing in the python interface:

in test_input_builder in python >=3.10 crashes because something goes wrong in os.environ (pytest updates its state in an environment variable).
Funnily enough, if:

  • I add a global plumed object
  • Or I force the input builder to not load the dictionary.
  • Or if I am in the "standard CI" (not the -debug- one).
    I do no get the segfault

I think something is left behind in the "destructor" of the plumed object in the with statement in _guessroot

@GiovanniBussi
Copy link
Member

Do you mean it is a bug in the python interpreter? I think it should not happen

@Iximiel
Copy link
Member Author

Iximiel commented Jan 29, 2026

I think it is more something like we some resource gets blocked and stops the os lib from accessing, or setting the variables

because if you go back to some old run you get the error in the ci with -degug- when i switched to python 3.10.

In the CI the problem is in unitest, here

The pyx changes save some diskspace when calling the inputbuilder by
deleting the test file
@Iximiel
Copy link
Member Author

Iximiel commented Jan 30, 2026

On my pc I get the error

ib=plumed.InputBuilder()
p=plumed.Plumed()

but not if I setup the IB like this.

p=plumed.Plumed()
ib=plumed.InputBuilder()

In the CI this seems not to be a problem. On my laptop with a cranky WSL I do no get the error even without the declared plumed in the tests. So the os.environ error it may be and edge case of what my workstation does when calling plumed_finalize? I still can't explain the one we get in the CI in File "/opt/hostedtoolcache/Python/3.10.19/x64/lib/python3.10/unittest/case.py", line 549 in _callTestMethod.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants