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

Importing the package causes error #3660

Closed
tommy-waltmann opened this issue May 10, 2022 · 11 comments · Fixed by conda-forge/mdanalysis-feedstock#40
Closed

Importing the package causes error #3660

tommy-waltmann opened this issue May 10, 2022 · 11 comments · Fixed by conda-forge/mdanalysis-feedstock#40

Comments

@tommy-waltmann
Copy link

tommy-waltmann commented May 10, 2022

Expected behavior

I should be able to import MDAnalysis without errors.

Actual behavior

Installing MDAnalysis from conda-forge and then importing gives the following output:

>>> python -c "import MDAnalysis"

Traceback (most recent call last):
  File "<string>", line 1, in <module>
  File "/Users/tomwalt/miniconda3/envs/mdanalysis/lib/python3.9/site-packages/MDAnalysis/__init__.py", line 191, in <module>
    from .lib import log
  File "/Users/tomwalt/miniconda3/envs/mdanalysis/lib/python3.9/site-packages/MDAnalysis/lib/__init__.py", line 34, in <module>
    from . import transformations
  File "/Users/tomwalt/miniconda3/envs/mdanalysis/lib/python3.9/site-packages/MDAnalysis/lib/transformations.py", line 198, in <module>
    from .mdamath import angle as vecangle
  File "/Users/tomwalt/miniconda3/envs/mdanalysis/lib/python3.9/site-packages/MDAnalysis/lib/mdamath.py", line 63, in <module>
    from . import util
  File "/Users/tomwalt/miniconda3/envs/mdanalysis/lib/python3.9/site-packages/MDAnalysis/lib/util.py", line 217, in <module>
    from ._cutil import unique_int_1d
  File "MDAnalysis/lib/_cutil.pyx", line 1, in init MDAnalysis.lib._cutil
ValueError: numpy.ndarray size changed, may indicate binary incompatibility. Expected 96 from C header, got 88 from PyObject

Code to reproduce the behavior

Create new conda environment

>>> conda create --name mdanalysis python=3.9
>>> conda activate mdanalysis
>>> conda install -c conda-forge numpy==1.21.6 -y
>>> conda install -c conda-forge mdanalysis -y 

Now try to import in a python script

import MDAnalysis

Current version of MDAnalysis

  • Which version are you using? (run python -c "import MDAnalysis as mda; print(mda.__version__)") 2.1.0 (installed from conda-forge)
  • Which version of Python (python -V)? 3.9.12
  • Which operating system? macOS (intel)
  • Which version of numpy? 1.21.6
@IAlibay
Copy link
Member

IAlibay commented May 10, 2022

So I can reproduce this on my *nix machine.

It's a bit late so I'm probably forgetting something important / obvious, but the fact that works fine on numpy 1.22.x releases is rather odd, especially since I'm pretty sure I built the C/C++ files using 1.21.5 for 2.1.0.

@tylerjereddy would you happen to have any thoughts here?

@joaander
Copy link

I don't see anything obviously wrong in your conda-forge recipe, but the build definitely pulled in numpy 1.22 and built against that ABI: https://dev.azure.com/conda-forge/feedstock-builds/_build/results?buildId=502680&view=logs&j=517fe804-fa30-5dc2-1413-330699242c05&t=c10fa5f2-fdf6-5338-3bdb-c4bea7c23412&l=2242

You do have numpy >1.18.0 - maybe that somehow causes the issue? As far as I know the conda-forge-pinning config should fix all conda-forge builds to 1.19.0 at build time: https://github.com/conda-forge/conda-forge-pinning-feedstock/blob/79533b64160a1a5ae59e14f2db7478e2b03b705a/recipe/conda_build_config.yaml#L651-L655

@tylerjereddy
Copy link
Member

A decreased structure size almost certainly means that MDAnalysis was built with a too-new version of NumPy and then an attempt was made to use an older version of NumPy at runtime. On the PyPI side, this is exactly why we have the NumPy versions carefully pinned for building in pyproject.toml.

On the conda-forge side, I'm a bit less familiar, but when I look at: https://github.com/conda-forge/mdanalysis-feedstock/blob/main/.ci_support/linux_64_python3.9.____cpythonpython_implcpython.yaml

I don't see a NumPy version pin? That seems bad?

The SciPy version of the equivalent file does have an appropriate NumPy pin in the yaml: https://github.com/conda-forge/scipy-feedstock/blob/main/.ci_support/linux_64_numpy1.19python3.9.____cpythonpython_implcpython.yaml

We seem to differ in the same way for OSX, which is the original case reported. I can ping someone if you need help adjusting that--I believe someone on our team is reasonably familiar with changing the conda-forge settings though?

@joaander
Copy link

I don't see a NumPy version pin? That seems bad?

Yes, this is bad. I checked my suspicion this morning. The numpy >1.18.0 specification in meta.yaml causes conda-smithy to remove the pinning that fixes the numpy ABI across all of conda-forge. I opened conda-forge/mdanalysis-feedstock#40 to fix this.

@mikemhenry
Copy link

numpy and conda-forge both make the same suggestion re: how to to pin numpy as a dependency

host:
  - numpy
run:
  - {{ pin_compatible('numpy') }}

https://numpy.org/devdocs/user/depending_on_numpy.html#build-time-dependency
https://conda-forge.org/docs/maintainer/knowledge_base.html#building-against-numpy

@IAlibay
Copy link
Member

IAlibay commented May 12, 2022

re-opening, something broke on merge of the new feedstock -- investigating

@IAlibay IAlibay reopened this May 12, 2022
@bdice
Copy link
Contributor

bdice commented May 13, 2022

Just to mention this in case it helps simplify the MDAnalysis configuration -- it seems that many of the pinnings in pyproject.toml are similar to the oldest-supported-numpy package. That can be an easier way to guarantee broad support while still building against the oldest supported version for forward compatibility.

Comparison:

@IAlibay
Copy link
Member

IAlibay commented May 13, 2022

I believe there was a specific reason why @tylerjereddy decided to avoid oldest-supported-numpy, I can't remember specifically, it might be that we run slightly behind NEP29.

For this specific issue, pyproject.toml is (or at least should be) ignored by conda-forge, which is the cause of some of our latest issues 😓 (pip 22.1 decided that they'd add in validation by default). I've fixed this temporarily with a host build pin, but now I'm trying to force conda-forge to use a reasonable numpy version for its various versions, I think I've done it, I just need to sit down and work out if the matrix works ABI-wise.

@tylerjereddy
Copy link
Member

Yeah, we need a slightly newer version of NumPy for one Python version vs. oldest-supported-numpy. Most packages don't, but MDAnalysis and SciPy can't use it directly at the moment.

I suspect this will change in the future as we drop Python versions.

@tylerjereddy
Copy link
Member

@IAlibay is probably referring to this bug: pypa/pip#11116

Sounds like they will fix that upstream soon

@IAlibay
Copy link
Member

IAlibay commented May 31, 2022

Closing this as fixed via the feedstock (at least it no longer fails for me locally). Please do re-open if this is not the case @tommy-waltmann

@IAlibay IAlibay closed this as completed May 31, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants