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

Include libcublas.so in libcublas #1

Closed
jakirkham opened this issue Mar 31, 2023 · 7 comments
Closed

Include libcublas.so in libcublas #1

jakirkham opened this issue Mar 31, 2023 · 7 comments

Comments

@jakirkham
Copy link
Member

jakirkham commented Mar 31, 2023

I've seen code that uses dlopen / dlsym as well as ctypes code that expects to use just the lib*.so as opposed to the versioned shared library at runtime. Not a blocker, but have we considered having the libcublas.so symlink in this package as opposed to the dev package? I know that the SONAME points to libcublas.so.12, but there's definitely code out there that will end up requiring libcublas-dev at runtime because of this.

Definitely not a blocker for getting this in though!

Originally posted by @kkraus14 in conda-forge/staged-recipes#21901 (comment)

@jakirkham jakirkham changed the title I've seen code that uses dlopen / dlsym as well as ctypes code that expects to use just the lib*.so as opposed to the versioned shared library at runtime. Not a blocker, but have we considered having the libcublas.so symlink in this package as opposed to the dev package? I know that the SONAME points to libcublas.so.12, but there's definitely code out there that will end up requiring libcublas-dev at runtime because of this. Include libcublas.so in libcublas Mar 31, 2023
@jakirkham
Copy link
Member Author

cc @adibbley @bdice @robertmaynard

@jakirkham
Copy link
Member Author

jakirkham commented Mar 31, 2023

We've been reworking RAPIDS code to use the proper SONAME form, like libcublas.so.12, anywhere that we've seen that. I could see an argument that it's a bug to rely on dlopening the unversioned form? That said, I don't have a reference for how we've packaged this in other CUDA Toolkit deployments to know if this approach is "normal" or not. I agree that we shouldn't treat this as a blocker, and I'd be happy to discuss further if users run into specific issues.

Originally posted by @bdice in conda-forge/staged-recipes#21901 (comment)


See also:

Originally posted by @bdice in conda-forge/staged-recipes#21901 (comment)

@jakirkham
Copy link
Member Author

Think this has been resolved by teaching the compilation tooling to look in the right place. If we are still missing some combination of flags, let's discuss that in a new issue with a minimal reproducer

@kkraus14
Copy link

kkraus14 commented Jun 2, 2023

Think this has been resolved by teaching the compilation tooling to look in the right place.

In the beginning of the issue I directly referenced dlopen and dlsym which doesn't go through compilation tooling and handles things at runtime, so that is not resolved.

@kkraus14 kkraus14 reopened this Jun 2, 2023
@jakirkham
Copy link
Member Author

After a fair bit of discussion, the conclusion we came to was SOVERSIONs should be included in libraries that are being dlopened

We went through making similar changes in RAPIDS relating to libcufile where this was an issue. Here's some related PRs that implemented that:

@kkraus14
Copy link

kkraus14 commented Jun 2, 2023

I have a strong suspicion we'll see things still dlopen libcublas.so and just depend on the dev package instead of trying to dlopen the versioned binaries, but we can revisit if needed then I guess?

@kkraus14 kkraus14 closed this as completed Jun 2, 2023
@jakirkham
Copy link
Member Author

Yeah we started looking through other Python libraries for this behavior. Generally they seemed to be doing the right thing. That said, there certainly could be more out there

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

No branches or pull requests

2 participants