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

ARM OSX Migrator #14

Conversation

regro-cf-autotick-bot
Copy link
Contributor

This feedstock is being rebuilt as part of the ARM OSX migration.

Feel free to merge the PR if CI is all green, but please don't close it
without reaching out the the ARM OSX team first at @conda-forge/help-osx-arm64.

If this PR was opened in error or needs to be updated please add the bot-rerun label to this PR. The bot will close this PR and schedule another one. If you do not have permissions to add this label, you can use the phrase @conda-forge-admin, please rerun bot in a PR comment to have the conda-forge-admin add it for you.

This PR was created by the regro-cf-autotick-bot. The regro-cf-autotick-bot is a service to automatically track the dependency graph, migrate packages, and propose package version updates for conda-forge. Feel free to drop us a line if there are any issues! This PR was generated by https://github.com/regro/cf-scripts/actions/runs/5238270940, please use this URL for debugging.

@conda-forge-webservices
Copy link

Hi! This is the friendly automated conda-forge-linting service.

I just wanted to let you know that I linted all conda-recipes in your PR (recipe) and found it was in an excellent condition.

@hadim hadim closed this Jun 18, 2023
@hadim hadim reopened this Jun 18, 2023
@hadim
Copy link
Member

hadim commented Jun 18, 2023

@conda-forge-admin, please rerender

@hadim
Copy link
Member

hadim commented Jun 18, 2023

see pyg-team/pyg-lib#211

@hadim
Copy link
Member

hadim commented Jun 18, 2023

@conda-forge/help-osx-arm64 I can't get the osx-arm64 build to work.

Compilations work fine but linking fails:

3-06-18T18:01:58.9047160Z   [19/19] Linking CXX shared library /Users/runner/miniforge3/conda-bld/pyg-lib_1687110450563/work/build/lib.macosx-11.0-arm64-cpython-39/libpyg.so
2023-06-18T18:01:58.9050760Z   FAILED: /Users/runner/miniforge3/conda-bld/pyg-lib_1687110450563/work/build/lib.macosx-11.0-arm64-cpython-39/libpyg.so
2023-06-18T18:01:58.9061500Z   : && /Users/runner/miniforge3/conda-bld/pyg-lib_1687110450563/_build_env/bin/arm64-apple-darwin20.0.0-clang++ -ftree-vectorize -fPIC -fPIE -fstack-protector-strong -O2 -pipe -stdlib=libc++ -fvisibility-inlines-hidden -fmessage-length=0 -isystem /Users/runner/miniforge3/conda-bld/pyg-lib_1687110450563/_h_env_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_p/include -fdebug-prefix-map=/Users/runner/miniforge3/conda-bld/pyg-lib_1687110450563/work=/usr/local/src/conda/pyg-lib-0.2.0 -fdebug-prefix-map=/Users/runner/miniforge3/conda-bld/pyg-lib_1687110450563/_h_env_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_p=/usr/local/src/conda-prefix -D_GLIBCXX_USE_CXX11_ABI=0 -O3 -DNDEBUG -isysroot /Applications/Xcode_13.2.1.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX11.0.sdk -mmacosx-version-min=11.0 -dynamiclib -Wl,-headerpad_max_install_names -Wl,-pie -Wl,-headerpad_max_install_names -Wl,-dead_strip_dylibs -Wl,-rpath,/Users/runner/miniforge3/conda-bld/pyg-lib_1687110450563/_h_env_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_p/lib -L/Users/runner/miniforge3/conda-bld/pyg-lib_1687110450563/_h_env_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_p/lib   -Xlinker -rpath -Xlinker /Library/Frameworks -o /Users/runner/miniforge3/conda-bld/pyg-lib_1687110450563/work/build/lib.macosx-11.0-arm64-cpython-39/libpyg.so -install_name @rpath/libpyg.so CMakeFiles/pyg.dir/pyg_lib/csrc/library.cpp.o CMakeFiles/pyg.dir/pyg_lib/csrc/ops/autograd/matmul_kernel.cpp.o CMakeFiles/pyg.dir/pyg_lib/csrc/ops/autograd/sampled_kernel.cpp.o CMakeFiles/pyg.dir/pyg_lib/csrc/ops/cpu/index_sort_kernel.cpp.o CMakeFiles/pyg.dir/pyg_lib/csrc/ops/cpu/matmul_kernel.cpp.o CMakeFiles/pyg.dir/pyg_lib/csrc/ops/cpu/sampled_kernel.cpp.o CMakeFiles/pyg.dir/pyg_lib/csrc/ops/index_sort.cpp.o CMakeFiles/pyg.dir/pyg_lib/csrc/ops/matmul.cpp.o CMakeFiles/pyg.dir/pyg_lib/csrc/ops/sampled.cpp.o CMakeFiles/pyg.dir/pyg_lib/csrc/random/cpu/biased_sampling.cpp.o CMakeFiles/pyg.dir/pyg_lib/csrc/sampler/cpu/neighbor_kernel.cpp.o CMakeFiles/pyg.dir/pyg_lib/csrc/sampler/cpu/random_walk_kernel.cpp.o CMakeFiles/pyg.dir/pyg_lib/csrc/sampler/cpu/subgraph_kernel.cpp.o CMakeFiles/pyg.dir/pyg_lib/csrc/sampler/neighbor.cpp.o CMakeFiles/pyg.dir/pyg_lib/csrc/sampler/random_walk.cpp.o CMakeFiles/pyg.dir/pyg_lib/csrc/sampler/subgraph.cpp.o CMakeFiles/pyg.dir/pyg_lib/csrc/utils/check.cpp.o CMakeFiles/pyg.dir/pyg_lib/csrc/utils/convert.cpp.o  -Wl,-rpath,/Users/runner/miniforge3/conda-bld/pyg-lib_1687110450563/_build_env/venv/lib/python3.9/site-packages/torch/lib  /Users/runner/miniforge3/conda-bld/pyg-lib_1687110450563/_build_env/venv/lib/python3.9/site-packages/torch/lib/libc10.dylib  /Library/Frameworks/Python.framework/Versions/3.11/lib/libpython3.11.dylib  /Users/runner/miniforge3/conda-bld/pyg-lib_1687110450563/_build_env/venv/lib/python3.9/site-packages/torch/lib/libtorch.dylib  /Users/runner/miniforge3/conda-bld/pyg-lib_1687110450563/_build_env/venv/lib/python3.9/site-packages/torch/lib/libtorch_cpu.dylib  /Users/runner/miniforge3/conda-bld/pyg-lib_1687110450563/_h_env_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_p/lib/libprotobuf.dylib  /Users/runner/miniforge3/conda-bld/pyg-lib_1687110450563/_build_env/venv/lib/python3.9/site-packages/torch/lib/libc10.dylib  /Users/runner/miniforge3/conda-bld/pyg-lib_1687110450563/_h_env_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_p/lib/libomp.dylib && :
2023-06-18T18:01:58.9067160Z   ld: warning: -pie being ignored. It is only used when linking a main executable
2023-06-18T18:01:58.9068480Z   ld: warning: ignoring file /Users/runner/miniforge3/conda-bld/pyg-lib_1687110450563/_build_env/venv/lib/python3.9/site-packages/torch/lib/libc10.dylib, building for macOS-arm64 but attempting to link with file built for macOS-x86_64
2023-06-18T18:01:58.9069880Z   ld: warning: ignoring file /Users/runner/miniforge3/conda-bld/pyg-lib_1687110450563/_build_env/venv/lib/python3.9/site-packages/torch/lib/libtorch.dylib, building for macOS-arm64 but attempting to link with file built for macOS-x86_64
2023-06-18T18:01:58.9071790Z   ld: warning: ignoring file /Users/runner/miniforge3/conda-bld/pyg-lib_1687110450563/_build_env/venv/lib/python3.9/site-packages/torch/lib/libtorch_cpu.dylib, building for macOS-arm64 but attempting to link with file built for macOS-x86_64
2023-06-18T18:01:58.9074690Z   ld: warning: ignoring file /Users/runner/miniforge3/conda-bld/pyg-lib_1687110450563/_build_env/venv/lib/python3.9/site-packages/torch/lib/libc10.dylib, building for macOS-arm64 but attempting to link with file built for macOS-x86_64
2023-06-18T18:01:58.9077340Z   Undefined symbols for architecture arm64:
2023-06-18T18:01:58.9080530Z     "at::checkNumel(char const*, at::TensorGeometryArg const&, long long)", referenced from:
2023-06-18T18:01:58.9083690Z         pyg::ops::segment_matmul(at::Tensor const&, at::Tensor const&, at::Tensor const&) in matmul.cpp.o
2023-06-18T18:01:58.9086750Z     "at::TensorMaker::make_tensor()", referenced from:
2023-06-18T18:01:58.9090300Z         at::Tensor pyg::utils::from_vector<unsigned char>(std::__1::vector<std::__1::pair<unsigned char, unsigned char>, std::__1::allocator<std::__1::pair<unsigned char, unsigned char> > > const&, bool) in neighbor_kernel.cpp.o
2023-06-18T18:01:58.9095050Z         at::Tensor pyg::utils::from_vector<unsigned char>(std::__1::vector<unsigned char, std::__1::allocator<unsigned char> > const&, bool) in neighbor_kernel.cpp.o
2023-06-18T18:01:58.9098320Z         at::Tensor pyg::utils::from_vector<signed char>(std::__1::vector<std::__1::pair<signed char, signed char>, std::__1::allocator<std::__1::pair<signed char, signed char> > > const&, bool) in neighbor_kernel.cpp.o
2023-06-18T18:01:58.9101400Z         at::Tensor pyg::utils::from_vector<signed char>(std::__1::vector<signed char, std::__1::allocator<signed char> > const&, bool) in neighbor_kernel.cpp.o
2023-06-18T18:01:58.9104590Z         at::Tensor pyg::utils::from_vector<int>(std::__1::vector<std::__1::pair<int, int>, std::__1::allocator<std::__1::pair<int, int> > > const&, bool) in neighbor_kernel.cpp.o
2023-06-18T18:01:58.9107960Z         at::Tensor pyg::utils::from_vector<int>(std::__1::vector<int, std::__1::allocator<int> > const&, bool) in neighbor_kernel.cpp.o
2023-06-18T18:01:58.9111290Z         at::Tensor pyg::utils::from_vector<long long>(std::__1::vector<std::__1::pair<long long, long long>, std::__1::allocator<std::__1::pair<long long, long long> > > const&, bool) in neighbor_kernel.cpp.o

Any idea?

@hadim
Copy link
Member

hadim commented Jun 18, 2023

It seems to be due to the build process picking the wrong python/pytorch version (host vs build vs native). Not sure how to fix that.

Copy link
Member

@h-vetinari h-vetinari left a comment

Choose a reason for hiding this comment

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

It seems to be due to the build process picking the wrong python/pytorch version (host vs build vs native). Not sure how to fix that.

Yup:

ld: warning: ignoring file [...]/_build_env/venv/lib/python3.9/site-packages/torch/lib/libc10.dylib, building for macOS-arm64 but attempting to link with file built for macOS-x86_64
ld: warning: ignoring file [...]/_build_env/venv/lib/python3.9/site-packages/torch/lib/libtorch.dylib, building for macOS-arm64 but attempting to link with file built for macOS-x86_64
ld: warning: ignoring file [...]/_build_env/venv/lib/python3.9/site-packages/torch/lib/libtorch_cpu.dylib, building for macOS-arm64 but attempting to link with file built for macOS-x86_64
ld: warning: ignoring file [...]/_build_env/venv/lib/python3.9/site-packages/torch/lib/libc10.dylib, building for macOS-arm64 but attempting to link with file built for macOS-x86_64

You'll have to ensure that the host pytorch gets picked. I don't know the specific case here, but since it finds the one in the $BUILD_PREFIX first, you could try removing that one.

recipe/meta.yaml Outdated Show resolved Hide resolved
recipe/meta.yaml Outdated
Comment on lines 33 to 34
- pytorch # [build_platform != target_platform]
- pytorch =*={{ torch_proc_type }}* # [build_platform != target_platform]
Copy link
Member

Choose a reason for hiding this comment

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

I'd try removing these two, because you're picking up pytorch from the build environment rather than host.

Copy link
Member

Choose a reason for hiding this comment

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

Thanks. Trying that now. But then I am confused, does that mean you don't need arm64 pytorch at all for the cross compilation?

Copy link
Member

Choose a reason for hiding this comment

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

Do you have an example of a pytorch compiled package that has an osx arm64 build?

Copy link
Member

Choose a reason for hiding this comment

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

There's a bunch of them if you go to the conda-forge GH org and search for repos starting with torch or pytorch (or you can look which feedstocks get touched by the pytorch migration on conda-forge.org/status)

Copy link
Member

Choose a reason for hiding this comment

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

I checked torchani for example and indeed they dont have pytorch in build. But If I remove it here, then all the build fails.

Copy link
Member

Choose a reason for hiding this comment

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

I've often seen it in build as well (presumably for anything that actually needs to execute some code from pytorch during installation), but for linking we need the one from host.

The right way would probably be to re-add the build dependency but ensure that whatever feeds the linkage mechanism looks in host first (might need a small patch)

Copy link
Member

Choose a reason for hiding this comment

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

The build dependency is already here.

The current hack works, and it's already used by other packages. I understand it's far from ideal, but I don't really see another way at the moment.

@hadim hadim requested a review from h-vetinari June 19, 2023 22:45
@hadim
Copy link
Member

hadim commented Jun 19, 2023

@h-vetinari the builds are passing now. I still need to test the osx-arm64 locally to make sure they are working before merging.

Copy link
Member

@h-vetinari h-vetinari left a comment

Choose a reason for hiding this comment

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

This does not look good to me

conda list -p ${BUILD_PREFIX} >packages.txt
cat packages.txt
PYTORCH_PACKAGE_VERSION=$(grep pytorch packages.txt | awk -F ' ' '{print $2}')
CONDA_SUBDIR=osx-arm64 conda create -y -p ${LIBTORCH_DIR} --no-deps pytorch=${PYTORCH_PACKAGE_VERSION} python=${PY_VER}
Copy link
Member

Choose a reason for hiding this comment

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

This looks like quite the hack. I'm not aware of any other pytorch-dependent package to need to drop down to installing something in the build scripts, and I doubt that this feedstock is special enough to need it.

Copy link
Member

Choose a reason for hiding this comment

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

I agree. It's coming from https://github.com/conda-forge/openmm-torch-feedstock/blob/f7b09cd93f69d7213acd88dca1b0b1770b0ac2bc/recipe/build.sh#L7.

I looked at many other repos but couldn't find similar enough ones to pyg-lib. Only this hack works.

Can you think of something else? I agree it looks overcomplicated for cross compiling a torch package.

Copy link
Member

@h-vetinari h-vetinari Jun 20, 2023

Choose a reason for hiding this comment

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

@conda-forge/openmm-torch @conda-forge/pytorch-cpu
I think we should come up with a better way here (and generally) to use pytorch in cross-compilation, and help pytorch-dependent feedstocks to do so.

FWIW, the openmm-bits where this is taken from were added in conda-forge/openmm-torch-feedstock#33 without much visible discussion, and I would have said the same thing there:

Calling conda in build scripts is something that should never1 be necessary.

Footnotes

  1. The only legitimate cases I'm aware of in all of conda-forge are all extremely low-level; this doesn't apply here.

Copy link

Choose a reason for hiding this comment

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

It looks to me as though this should be in the build section (it is!) and then we need to figure out where that is to set the correct Torch_DIR. Though this may have been tried in one of the many labor intensive iterations to get here. And if that doesn't work but this manual installation does, that is very much a mystery.

Copy link
Member

Choose a reason for hiding this comment

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

I tried that already. If someone wants to give it a new shot, I am all good. I am a bit clueless about what to try next on my side here.

Copy link
Member

Choose a reason for hiding this comment

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

If someone has an example of a package with the same constraints, I could take a look and try to adapt it to that feedstock.

Copy link
Member

Choose a reason for hiding this comment

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

@jakirkham any idea on this?

To summarize: pyg needs torch from build but torch from env during linking and it's not clear what is the best way to achieve this without the hack above. The build is CMake based.

Copy link
Member

Choose a reason for hiding this comment

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

@h-vetinari I think I got it to work. Can you review #23 ?

@hadim
Copy link
Member

hadim commented Jun 20, 2023

I have been able to test the osx-arm64 on a Mac M2 machine and the package works (using the artifacts from the Azure CI). I will hold here until we find a better solution given the hacky conda installation during the build.

@mikemhenry
Copy link

Sorry about that hack, but I am open to something that works to enable pytorch cross-compilation to be less painful.

@xhochy
Copy link
Member

xhochy commented Jun 21, 2023

I'm trying a different approach over at conda-forge/torchvision-feedstock#74 but that is based on a setup.py and not a CMake setup.

@hadim hadim mentioned this pull request Nov 28, 2023
5 tasks
@hadim
Copy link
Member

hadim commented Nov 28, 2023

closing in favour of #23

@hadim hadim closed this Nov 28, 2023
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.

6 participants