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

BLD: detect xsimd if it's installed and add to pythran dependency #18439

Merged
merged 1 commit into from
May 9, 2023

Conversation

rgommers
Copy link
Member

@rgommers rgommers commented May 7, 2023

Fixes xsimd/xsimd.hpp: No such file or directory, as reported in gh-15698 and gh-18415.

The root cause of that issue is a compiler activation issue that occurs with some of the more niche shells with conda, as discussed in the last comments of gh-15698. The required: false should make this robust.

A useful side-effect is that the xsimd version is now reported in the build config terminal output:

Program pythran found: YES (/home/rgommers/mambaforge/envs/scipy-dev/bin/pythran)
Dependency xsimd found: YES 8.0.5

Fixes `xsimd/xsimd.hpp: No such file or directory`,
as reported in scipygh-15698 and scipygh-18415.
@rgommers rgommers added Build issues Issues with building from source, including different choices of architecture, compilers and OS Pythran Items related to internal Pythran code base labels May 7, 2023
@rgommers rgommers added this to the 1.11.0 milestone May 7, 2023
@h-vetinari
Copy link
Member

Dependency xsimd found: YES 8.0.5

Do we limit the xsimd version somewhere? I'm just curious because xsimd is at v11 already (and pythran depends on newer versions of xsimd occasionally, as we saw for pythran 0.13.0 (leading them to bump their vendored headers to xsimd 11 + an unreleased patch in 0.13.1)

@rgommers
Copy link
Member Author

rgommers commented May 8, 2023

Do we limit the xsimd version somewhere?

No, that is not relevant here I'd say. It's a transitive dependency, so the place to manage matching versions is in the pythran metadata (for conda-forge, in the feedstock recipe).

@h-vetinari
Copy link
Member

No, that is not relevant here I'd say.

It was more out of curiosity, but it's good I asked, because I forgot that there was a cap on the feedstock at all: conda-forge/pythran-feedstock#76

Copy link
Contributor

@tylerjereddy tylerjereddy left a comment

Choose a reason for hiding this comment

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

Ok, CI is passing and this seems like a small enough patch. Note that I don't see xsimd as available in the meson config/build output when using the standard PyPI package for pythran, which I assume is expected for the vendored scenario (I don't know if Pythran intends for the xsimd it vendors to be probed for a version string, etc.).

Anyway if it fixes a tricky build problem it seems sensible enough. Is it clear what would happen if I manually spack installed bleeding-edge xsimd and loaded that into my virtualenv with the PyPI version of pythran? Since that's asking for trouble doesn't make sense to block this on that, but somewhat curious...

@tylerjereddy tylerjereddy merged commit 878285c into scipy:main May 9, 2023
20 checks passed
@tylerjereddy
Copy link
Contributor

I tried the experiment--meson will show i.e., xsimd 11.0.0, though I can't tell if that means it will use that version over the vendored pythran version of xsimd installed via PyPI. The testsuite passed at least...

@eli-schwartz
Copy link
Contributor

You will simply end up with both ..../site-packages/pythran/ and <prefix>/include/ in your include paths, after which point the compiler will do as the compiler does.

But maybe people that patch pythran to devendor xsimd should symlink it into the pythran site-packages if the include path isn't guaranteed to be included by the compiler. Or patch pythran.get_include() to return both directories -- except that will not work due to list vs string. So the symlink is probably best.

@rgommers rgommers deleted the fix-xsimd-notfound branch May 9, 2023 08:52
@rgommers
Copy link
Member Author

rgommers commented May 9, 2023

But maybe people that patch pythran to devendor xsimd should symlink it into the pythran site-packages if the include path isn't guaranteed to be included by the compiler.

I suspect conda is the only package manager with a problem here. It's of course a pretty bad bug that the default <prefix>/include is not visible, that is not normal. And I'm trying to get conda-forge to stop its de-vendoring, because it's a pretty bad idea as it currently stands. At that point we can remove this patch again.

though I can't tell if that means it will use that version over the vendored pythran version of xsimd installed via PyPI.

 description = Generating$ scipy/stats/_stats_pythran$ with$ a$ custom$ command

build scipy/stats/_stats_pythran.cpython-310-x86_64-linux-gnu.so.p/meson-generated_..__stats_pythran.cpp.o: cpp_COMPILER scipy/stats/_stats_pythran.cpp
 DEPFILE = scipy/stats/_stats_pythran.cpython-310-x86_64-linux-gnu.so.p/meson-generated_..__stats_pythran.cpp.o.d
 DEPFILE_UNQUOTED = scipy/stats/_stats_pythran.cpython-310-x86_64-linux-gnu.so.p/meson-generated_..__stats_pythran.cpp.o.d
 ARGS = -Iscipy/stats/_stats_pythran.cpython-310-x86_64-linux-gnu.so.p -Iscipy/stats -I../scipy/stats -I../../../mambaforge/envs/scipy-dev/lib/python3.10/site-packages/pythran -I../../../mambaforge/envs/scipy-dev/lib/python3.10/site-packages/numpy/core/include -I/home/rgommers/mambaforge/envs/scipy-dev/include/python3.10 -fvisibility=hidden -fvisibility-inlines-hidden -fdiagnostics-color=always -D_FILE_OFFSET_BITS=64 -Wall -Winvalid-pch -std=c++14 -O2 -g -fvisibility-inlines-hidden -fmessage-length=0 -march=nocona -mtune=haswell -ftree-vectorize -fPIC -fstack-protector-strong -fno-plt -O2 -ffunction-sections -pipe -isystem /home/rgommers/mambaforge/envs/scipy-dev/include -DNDEBUG -D_FORTIFY_SOURCE=2 -O2 -isystem /home/rgommers/mambaforge/envs/scipy-dev/include -fPIC -DENABLE_PYTHON_MODULE -D__PYTHRAN__=3 -DPYTHRAN_BLAS_NONE -Wno-cpp -Wno-deprecated-declarations -Wno-unused-but-set-variable -Wno-unused-function -Wno-unused-variable -Wno-int-in-bool-context

build scipy/stats/_stats_pythran.cpython-310-x86_64-linux-gnu.so.p/_stats_pythran.cpython-310-x86_64-linux-gnu.so.symbols: SHSYM scipy/stats/_stats_pythran.cpython-310-x86_64-linux-gnu.so
 IMPLIB = scipy/stats/_stats_pythran.cpython-310-x86_64-linux-gnu.so

build scipy/stats/_stats_pythran.cpython-310-x86_64-linux-gnu.so: cpp_LINKER scipy/stats/_stats_pythran.cpython-310-x86_64-linux-gnu.so.p/meson-generated_..__stats_pythran.cpp.o
 LINK_ARGS = -L/home/rgommers/mambaforge/envs/scipy-dev/lib -Wl,--as-needed -Wl,--allow-shlib-undefined -shared -fPIC -Wl,-O2 -Wl,--sort-common -Wl,--as-needed -Wl,-z,relro -Wl,-z,now -Wl,--disable-new-dtags -Wl,--gc-sections -Wl,--allow-shlib-undefined -Wl,-rpath,/home/rgommers/mambaforge/envs/scipy-dev/lib -Wl,-rpath-link,/home/rgommers/mambaforge/envs/scipy-dev/lib -fvisibility-inlines-hidden -fmessage-length=0 -march=nocona -mtune=haswell -ftree-vectorize -fPIC -fstack-protector-strong -fno-plt -O2 -ffunction-sections -pipe -isystem /home/rgommers/mambaforge/envs/scipy-dev/include -DNDEBUG -D_FORTIFY_SOURCE=2 -O2 -isystem /home/rgommers/mambaforge/envs/scipy-dev/include -Wl,--version-script=/home/rgommers/code/scipy/scipy/_build_utils/link-version-pyinit.map

The search order shows that the vendored version is picked up first I believe.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Build issues Issues with building from source, including different choices of architecture, compilers and OS Pythran Items related to internal Pythran code base
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants