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

Bundle libcupti with libtorch to make it work with cuda-platform-redist #1307

Merged
merged 2 commits into from
Jan 12, 2023

Conversation

sbrunk
Copy link
Contributor

@sbrunk sbrunk commented Jan 8, 2023

I've managed to bundle libcupti by adding it to the link list. Is this the right approach in general?

One issue is that it adds libcupti.so without the version information, but libcuda links with libcupti.so.11.8.

@saudet
Copy link
Member

saudet commented Jan 9, 2023

I've managed to bundle libcupti by adding it to the link list. Is this the right approach in general?

I think that's OK for this case, yes.

One issue is that it adds libcupti.so without the version information, but libcuda links with libcupti.so.11.8.

There's a dot missing, it needs to be cupti@.11.8.

@saudet
Copy link
Member

saudet commented Jan 9, 2023

But since the Linux CI-build did not fail, it should have been able to find the library on centos as well right?
So I've just changed the Windows path for now.

Seems to be located there on the CI server as well, yes, but let's add that extras path for Linux as well since it sometimes gets placed there by default, for example, see tensorflow/tensorflow#39526.

@sbrunk
Copy link
Contributor Author

sbrunk commented Jan 9, 2023

Seems to be located there on the CI server as well, yes, but let's add that extras path for Linux as well since it sometimes gets placed there by default, for example, see tensorflow/tensorflow#39526.

Done.

@sbrunk sbrunk marked this pull request as ready for review January 9, 2023 16:38
@saudet
Copy link
Member

saudet commented Jan 12, 2023

Actually, we should try to get this done with "preload" since we know that works well for the core libraries from CUDA. I've pushed a commit in that regards to your branch. Could you test this out to make sure it works?

@sbrunk
Copy link
Contributor Author

sbrunk commented Jan 12, 2023

Yes works for me (tested only on Linux so far after building locally).

Do you know if there's a way to download the CI artifacts from GH actions? I've seen repos where they are cached and are available for download for a few days but perhaps there are size restrictions.

@saudet saudet merged commit fc69358 into bytedeco:master Jan 12, 2023
@saudet
Copy link
Member

saudet commented Jan 12, 2023

Yes works for me (tested only on Linux so far after building locally).

It looks like the profiler is disabled on Windows by default anyway, so no need to worry about that for now I guess:
https://github.com/pytorch/pytorch/blob/master/cmake/Dependencies.cmake#L1898

Do you know if there's a way to download the CI artifacts from GH actions? I've seen repos where they are cached and are available for download for a few days but perhaps there are size restrictions.

Yes, all builds get deployed as snapshots: http://bytedeco.org/builds/

@sbrunk sbrunk deleted the bundle-libcupti branch January 12, 2023 13:20
@sbrunk
Copy link
Contributor Author

sbrunk commented Jan 12, 2023

But only builds on master right? At least the sonatype snapshot seems to correspond to the latest pytorch related commit on master (before this PR) on Jan 3.

What I meant was the ability to download artifacts from PRs for testing things. I think what I've seen was a result of the upload-artifact action: https://github.com/actions/upload-artifact#where-does-the-upload-go

I could try to add that if you think it could make sense.

@saudet
Copy link
Member

saudet commented Jan 13, 2023

But only builds on master right? At least the sonatype snapshot seems to correspond to the latest pytorch related commit on master (before this PR) on Jan 3.

Not just the master branch, any branch. You'll need to set the "CI_DEPLOY_*" secrets in your fork to values corresponding to your account on Sonatype OSSRH, and I can give you access to the "org.bytedeco" groupId to have your artifacts pushed there as well, with different versions probably to avoid conflicts, but still it works.

What I meant was the ability to download artifacts from PRs for testing things. I think what I've seen was a result of the upload-artifact action: https://github.com/actions/upload-artifact#where-does-the-upload-go

I could try to add that if you think it could make sense.

Possibly, but it's usually easier to just use a Maven repository than a manual system like that. If we could wire this to GitHub Packages somehow, then being able to use our GitHub Account would be a clear advantage over Sonatype OSSRH: https://docs.github.com/en/packages/working-with-a-github-packages-registry/working-with-the-apache-maven-registry

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.

None yet

2 participants