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

Document how to handle problems with CMake finding NumPy in cross-compilation #2260

Open
h-vetinari opened this issue Aug 9, 2024 · 13 comments · May be fixed by #2321
Open

Document how to handle problems with CMake finding NumPy in cross-compilation #2260

h-vetinari opened this issue Aug 9, 2024 · 13 comments · May be fixed by #2321

Comments

@h-vetinari
Copy link
Member

This is exacerbated by the fact that the error messages by CMake point at the wrong variables, so it can be quite frustrating to figure out. I wrote down my understanding of this in conda-forge/faiss-split-feedstock@80dab60, perhaps we can distill some of that into the knowledge base.

CC @traversaro

@traversaro
Copy link
Contributor

traversaro commented Aug 9, 2024

Just a comment on a part of the commit message:

  • The error messages are actively misleading, as
Could NOT find Python (missing: Python_INCLUDE_DIRS Python_LIBRARIES
Python_NumPy_INCLUDE_DIRS Development NumPy Development.Module
Development.Embed)

suggests that the variables in question use plural _DIRS / _LIBRARIES, yet
what's actually required is the singular. [1]
[1] : https://cmake.org/cmake/help/latest/module/FindPython.html

I totally understand why that message is confusing, but just to explain its rationale. Basically the singular variables (Python_INCLUDE_DIR, Python_NumPy_INCLUDE_DIR) are the one read by CMake's FindPython to read possible location specified by the user (see Artifacts Specification in the docs), while the error message refer to the variables that are the output of the find procedure. If the find is able to find Python on its own, it is totally normal that Python_INCLUDE_DIR, while it is not normal that Python_INCLUDE_DIRS is empty. So the error basically complains that the find procedure failed because the output are empty, while it may or not be possible for the input variable to be empty.

@traversaro
Copy link
Contributor

By the way, a kind of related aspect is it may be possible to actually fix CMake and/or conda-forge infrastructure to make sure that all headers are found out of the box if the cmake minimum required is high enough, instead of specifying manually everywhere. However, I tried to investigate a bit on that and at at the moment I did not get a clear idea on this.

Related recipes that use workarounds:

@traversaro
Copy link
Contributor

Yes another thing for numpy>=2, it seems that the right include dirs are:

-DPython3_NumPy_INCLUDE_DIR=$SP_DIR/numpy/_core/include

while with numpy<2 they were:

-DPython3_NumPy_INCLUDE_DIR=$SP_DIR/numpy/core/include

@hmaarrfk
Copy link
Contributor

hmaarrfk commented Sep 4, 2024

Is this code no longer correct

NUMPY_INC=$(python -c 'import numpy;print(numpy.get_include())')

@traversaro
Copy link
Contributor

traversaro commented Sep 4, 2024

Is this code no longer correct

NUMPY_INC=$(python -c 'import numpy;print(numpy.get_include())')

I guess it is still correct, but that point to the numpy in the build environment, not the one in the host, right? I do not know enough about numpy headers to know if the two are equivalent.

@hmaarrfk
Copy link
Contributor

hmaarrfk commented Sep 4, 2024

numpy in the build environment

I thought the whole point of the cross python stuff was that it actually tricked things into thinking it was the host environment. But maybe I was just assuming too much. I think at least today, you are correct:

# I created a debugging cross compiled environment for linux-aarch.
$ mamba list --prefix ${BUILD_PREFIX} | grep python
cross-python_linux-aarch64 3.10                 42_cpython    conda-forge
python                    3.10.14         hd12c33a_0_cpython    conda-forge
python_abi                3.10                    5_cp310    conda-forge
(/home/conda/feedstock_root/build_artifacts/debug_1725452674893/_build_env) [conda@a25b5bb2f77c work]$ python -c 'import numpy;print(numpy.get_include())'
/home/conda/feedstock_root/build_artifacts/debug_1725452674893/_build_env/venv/lib/python3.10/site-packages/numpy/core/include
(/home/conda/feedstock_root/build_artifacts/debug_1725452674893/_build_env) [conda@a25b5bb2f77c work]$ ${PREFIX}/bin/python -c 'import numpy;print(numpy.get_include())'
/home/conda/feedstock_root/build_artifacts/debug_1725452674893/_build_env/venv/lib/python3.10/site-packages/numpy/core/include

It might be related to some recent cross compilation failures.
conda-forge/ale-py-feedstock#16
Either or bug, or my misunderstanding is more common with some other upstream developers.
Looking how cmake tests for things: https://gitlab.kitware.com/cmake/cmake/-/blob/master/Modules/FindPython/Support.cmake?ref_type=heads#L3931

One thing we should highlight is the huge difference between PYTHON_ and Python_ for cmake. the Python_ is the "new-style" https://cmake.org/cmake/help/latest/module/FindPython.html so since "2018", and really helped in cross compilation workflows.

However, it took a while for packages to adopt in the source repos, often due to trying to ensure they targetted a wide range of python/cmake versions (e.g. scikit-build/scikit-build#1032). Without backward compatibility, it was hard to sell many developers on the "new style" flags.

@isuruf
Copy link
Member

isuruf commented Sep 4, 2024

I think at least today, you are correct:

That's the wrong conclusion. The headers there are correct. Notice the venv there.

@traversaro
Copy link
Contributor

Ah, good to know, this was never clear to me!

@hmaarrfk
Copy link
Contributor

hmaarrfk commented Sep 4, 2024

I guess i had alot of trouble finding "numpy" in cross compilation in vigra, and i still can't figure it out xref: conda-forge/vigra-feedstock#139

@hmaarrfk
Copy link
Contributor

Whatever the "right" way is to find the numpy and python, i would appreciate some help in
conda-forge/pytorch-cpu-feedstock#256

@traversaro
Copy link
Contributor

Whatever the "right" way is to find the numpy and python, i would appreciate some help in conda-forge/pytorch-cpu-feedstock#256

Sorry, I missed the comment, see https://github.com/conda-forge/pytorch-cpu-feedstock/pull/256/files#r1781037695 .

@traversaro
Copy link
Contributor

Based on what happened in conda-forge/idyntree-feedstock#109 and the input of this thread, I think I reached a clean enough strategy in conda-forge/idyntree-feedstock#109 .

@traversaro traversaro linked a pull request Oct 1, 2024 that will close this issue
3 tasks
@traversaro
Copy link
Contributor

I made a proposal to document this in #2321, feel free to comment and suggest improvements.

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

Successfully merging a pull request may close this issue.

4 participants