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.

Sign up for GitHub

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

Preserving libtorch_python in package #246

Closed
wants to merge 10 commits into from
Closed

Preserving libtorch_python in package #246

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
74c5d7a
Keep libtorch_python
jeongseok-meta Jul 9, 2024
7e83af6
Increase build number
jeongseok-meta Jul 9, 2024
84af8ac
Add test for libtorch_python
jeongseok-meta Jul 9, 2024
ab28630
Revert "Keep libtorch_python"
isuruf Aug 2, 2024
4ae4eb3
Revert "Add test for libtorch_python"
isuruf Aug 2, 2024
a620157
move libtorch_python.so to PREFIX/lib
isuruf Aug 2, 2024
11ed4ee
trigger
hmaarrfk Aug 16, 2024
e1ce95b
MNT: Re-rendered with conda-build 24.7.1, conda-smithy 3.38.0, and co…
Aug 16, 2024
8ed8fec
no need to specify c_stdlib for linux anymore
hmaarrfk Aug 20, 2024
31956b1
try to pin magma to 2.7.2
hmaarrfk Aug 21, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion recipe/build.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -218,7 +218,6 @@ if [[ "$PKG_NAME" == "libtorch" ]]; then
for f in ATen caffe2 tensorpipe torch c10; do
mv torch/include/$f ${PREFIX}/include/$f
done
rm ${PREFIX}/lib/libtorch_python.*
Copy link
Member

Choose a reason for hiding this comment

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

This won't work AFAIU; libtorch_python.so depends on a specific python version, while the conda package libtorch intentionally does not depend on python at all.

If we want to ship libtorch_python.so, we should most likely package it in the pytorch output (or perhaps create a separate output).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thank you for pointing out that libtorch should not depend on Python. Moving it or creating a separate output seems like a viable solution.

I am relatively new to the build script of this feedstock and find it more complex than other packages I've worked with. Could you please provide some guidance on how to implement your suggestion?

Copy link
Member

Choose a reason for hiding this comment

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

Could you please provide some guidance on how to implement your suggestion?

As a first approach, it should work to "install" libtorch_python.so in build_pytorch.sh, by copying from the build cache (that's created & populated during the execution of build.sh) into $PREFIX/lib.

popd
popd

Expand Down
4 changes: 2 additions & 2 deletions recipe/meta.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
{% set version = "2.3.1" %}
{% set build = 1 %}
{% set build = 2 %}

{% if cuda_compiler_version != "None" %}
{% set build = build + 200 %}
Expand Down Expand Up @@ -159,7 +159,7 @@ requirements:
test:
commands:
# libraries
{% for each_lib in [ 'libc10', 'libshm', 'libtorch', 'libtorch_cpu', 'libtorch_global_deps'] %}
{% for each_lib in [ 'libc10', 'libshm', 'libtorch', 'libtorch_cpu', 'libtorch_global_deps', 'libtorch_python'] %}
- test -f $PREFIX/lib/{{ each_lib }}.so # [linux]
- test -f $PREFIX/lib/{{ each_lib }}.dylib # [osx]
{% endfor %}
Expand Down
Loading