Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Installing MDAnalysis from pip deps triggers numpy error #129

Closed
lilyminium opened this issue Jun 22, 2024 · 10 comments
Closed

Installing MDAnalysis from pip deps triggers numpy error #129

lilyminium opened this issue Jun 22, 2024 · 10 comments

Comments

@lilyminium
Copy link
Member

From mdakit-cookie CI -- https://github.com/MDAnalysis/mdakit-cookie/actions/runs/9615305167

Across Pythons 3.10-3.12 on MDAnalysis develop, we get a header size error:

Run pyver=$(python -c 'import MDAnalysis; print(MDAnalysis.__version__)')
Traceback (most recent call last):
  File "<string>", line 1, in <module>
  File "/opt/hostedtoolcache/Python/3.11.9/x64/lib/python3.11/site-packages/MDAnalysis/__init__.py", line 193, in <module>
    from .lib import log
  File "/opt/hostedtoolcache/Python/3.11.9/x64/lib/python3.11/site-packages/MDAnalysis/lib/__init__.py", line 34, in <module>
    from . import transformations
  File "/opt/hostedtoolcache/Python/3.11.9/x64/lib/python3.11/site-packages/MDAnalysis/lib/transformations.py", line 172, in <module>
    from .mdamath import angle as vecangle
  File "/opt/hostedtoolcache/Python/3.11.9/x64/lib/python3.11/site-packages/MDAnalysis/lib/mdamath.py", line 63, in <module>
    from . import util
  File "/opt/hostedtoolcache/Python/3.11.9/x64/lib/python3.11/site-packages/MDAnalysis/lib/util.py", line 221, in <module>
    from ._cutil import unique_int_1d
  File "MDAnalysis/lib/_cutil.pyx", line 1, in init MDAnalysis.lib._cutil
ValueError: numpy.dtype size changed, may indicate binary incompatibility. Expected 96 from C header, got 88 from PyObject

Possibly needs pinning to numpy < 2.0?

Error not triggered by conda.

@IAlibay
Copy link
Member

IAlibay commented Jun 22, 2024

Yeah I suspect that a pin is the way forward until the next release - sorry about that :(

@lilyminium
Copy link
Member Author

Thanks @IAlibay! Just thinkign about this some more wrt the workshop and future maintainability of any MDAKits that might be created here, I was thinking that maybe the best way to do this would actually be in the install-mdanalysis action -- just adding a 'numpy<2.0' around here https://github.com/MDAnalysis/install-mdanalysis/blob/60ef7aab832dad506a65e28f4c927121b028191a/action.yaml#L107 . Any thoughts on that? That way if we do start supporting 2.0 then it'll automatically propagate through to kits that use the install-mdanalysis action without them needing to change anything.

@IAlibay
Copy link
Member

IAlibay commented Jun 22, 2024

Actually, I'm pretty sure I started pinning an upper version of numpy a while back - what version of MDA are you picking up?

@IAlibay
Copy link
Member

IAlibay commented Jun 22, 2024

Ok just checked, I started putting an upper pin for numpy in the package with release 2.6 and above.

As a minimum here (in the cookiecutter) I would suggest pinning the minimum MDA to >2.6, at least that'll ensure a proper failure when trying to combine np 2.0+ and MDA right now. Thoughts on this @lilyminium ?

Something does need doing about install-mdanalysis, will think about that now.

@lilyminium
Copy link
Member Author

The failures only happen when installing from develop, not any particular release package. Happy to move the pin upwards but I'm not sure that's the issue here. The logs from the failed run indicate that MDA wasn't installed before the install-mdanalysis action, so I don't think it could be the case where an older MDA is installed with numpy 2.0, then force-replaced with a newer one.

@IAlibay
Copy link
Member

IAlibay commented Jun 22, 2024

when installing develop

Ah ok, that's a key bit of information I didn't know. I have an idea, need to double check something.

@IAlibay
Copy link
Member

IAlibay commented Jun 22, 2024

Ok I think I got it, sorry there's just too many threads going on and I completely forgot about this MDAnalysis/mdanalysis#4620

I'm going to aim to merge that PR and I think it'll clean things up, or at least it'll take us to a different state.

@IAlibay
Copy link
Member

IAlibay commented Jun 22, 2024

This should be fixed upstream - can you confirm that this is also resolved for you here @lilyminium ?

@IAlibay
Copy link
Member

IAlibay commented Jun 22, 2024

I re-ran the above-mentioned CI matrix and everything seems to run fine now: https://github.com/MDAnalysis/mdakit-cookie/actions/runs/9615305167 - probably means we're ok?

@lilyminium
Copy link
Member Author

Thanks for putting in the fix @IAlibay! I'm happy that CI is all green now on https://github.com/MDAnalysis/mdakit-cookie. Closing now :-)

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

No branches or pull requests

2 participants