Skip to content

Conversation

bmanga
Copy link
Contributor

@bmanga bmanga commented Oct 13, 2020

This adds the RPATH to the torch libraries so that they can be found when a project links to torchvision

@vfdev-5 vfdev-5 changed the title Add RPATH to torchvision cmake tartet for torch libraries Add RPATH to torchvision cmake target for torch libraries Oct 20, 2020
@vfdev-5
Copy link
Collaborator

vfdev-5 commented Oct 20, 2020

@bmanga I checked this PR with the test from #2807 on MacOSX. The example works on the current master and on this PR giving 9 ops. The only difference is with linked libraries:

# master

libtorchvision.dylib:
        @rpath/libtorchvision.dylib (compatibility version 0.0.0, current version 0.0.0)
        @rpath/libc10.dylib (compatibility version 0.0.0, current version 0.0.0)
        /usr/local/opt/libpng/lib/libpng16.16.dylib (compatibility version 54.0.0, current version 54.0.0)
        /usr/local/opt/jpeg/lib/libjpeg.9.dylib (compatibility version 13.0.0, current version 13.0.0)
        /usr/local/opt/python/Frameworks/Python.framework/Versions/3.7/Python (compatibility version 3.7.0, current version 3.7.0)
        @rpath/libtorch.dylib (compatibility version 0.0.0, current version 0.0.0)
        @rpath/libtorch_cpu.dylib (compatibility version 0.0.0, current version 0.0.0)
        /usr/lib/libc++.1.dylib (compatibility version 1.0.0, current version 400.9.0)
        /usr/lib/libSystem.B.dylib (compatibility version 1.0.0, current version 1252.0.0)

and

# PR 2801

libtorchvision.dylib:
        @rpath/libtorchvision.dylib (compatibility version 0.0.0, current version 0.0.0)
        @rpath/libc10.dylib (compatibility version 0.0.0, current version 0.0.0)
        /usr/local/opt/libpng/lib/libpng16.16.dylib (compatibility version 54.0.0, current version 54.0.0)
        /usr/local/opt/jpeg/lib/libjpeg.9.dylib (compatibility version 13.0.0, current version 13.0.0)
        @rpath/libpython3.7m.dylib (compatibility version 3.7.0, current version 3.7.0)
        @rpath/libtorch.dylib (compatibility version 0.0.0, current version 0.0.0)
        @rpath/libtorch_cpu.dylib (compatibility version 0.0.0, current version 0.0.0)
        /usr/lib/libc++.1.dylib (compatibility version 1.0.0, current version 400.9.0)
        /usr/lib/libSystem.B.dylib (compatibility version 1.0.0, current version 1252.0.0)

@bmanga
Copy link
Contributor Author

bmanga commented Oct 20, 2020

@vfdev-5 Thanks for checking. Given that it seems to work fine also on osx, do you think it's fine to merge? I am not sure what is going on with the tests, but it doesn't seem to work for me locally without this change (I get a runtime error saying that the libtorch libraries could not be found).

@vfdev-5
Copy link
Collaborator

vfdev-5 commented Oct 20, 2020

@bmanga I updated the PR to current master, so let's see what CI shows (we can skip Travis as it is broken now) and see if possible to merge like that.

Copy link
Collaborator

@vfdev-5 vfdev-5 left a comment

Choose a reason for hiding this comment

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

Good to merge. Travis failure is unrelated.
Thanks @bmanga !

@codecov
Copy link

codecov bot commented Oct 21, 2020

Codecov Report

Merging #2801 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #2801   +/-   ##
=======================================
  Coverage   73.36%   73.36%           
=======================================
  Files          99       99           
  Lines        8787     8787           
  Branches     1387     1387           
=======================================
  Hits         6447     6447           
  Misses       1916     1916           
  Partials      424      424           

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update e280f61...f5ef9cb. Read the comment docs.

@vfdev-5 vfdev-5 merged commit 34708e4 into pytorch:master Oct 21, 2020
@fmassa
Copy link
Member

fmassa commented Oct 21, 2020

Thanks @bmanga and @vfdev-5 !

bryant1410 pushed a commit to bryant1410/vision-1 that referenced this pull request Nov 22, 2020
vfdev-5 added a commit to Quansight/vision that referenced this pull request Dec 4, 2020
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.

3 participants