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

Lazy import cuSOLVER #7843

Merged
merged 3 commits into from
Sep 5, 2023
Merged

Lazy import cuSOLVER #7843

merged 3 commits into from
Sep 5, 2023

Conversation

kmaehashi
Copy link
Member

@kmaehashi kmaehashi commented Sep 3, 2023

This is a part of #7620.

I tried to softlink cuSOLVER as I did in NVRTC but found there is a native function calls to cuSOLVER in cupy_lapack.h where softlink doesn't work.
For cuSOLVER I'm thinking of using lazy-import instead of softlink.

Depends on #7840 (merged).

@kmaehashi kmaehashi marked this pull request as ready for review September 3, 2023 07:26
@leofang
Copy link
Member

leofang commented Sep 3, 2023

What if I change to use dlopen in cupy_lapack.h? Would it help unify the style?

@takagi takagi self-assigned this Sep 4, 2023
@takagi takagi added cat:enhancement Improvements to existing features prio:medium blocking Issue/pull-request is mandatory for the upcoming release labels Sep 4, 2023
@takagi takagi added this to the v13.0.0rc1 milestone Sep 4, 2023
@kmaehashi
Copy link
Member Author

kmaehashi commented Sep 4, 2023

TBH, after working on this PR, I'm a bit surprised how it is simple to lazy-import cuSOLVER than I initially expected. Although I still think soft linking (or cuSOLVER/cuSPARSE/cuFFT/... support in CUDA Python) is something we will need to support "one CuPy package for all CUDA versions", this is not mandatory to achieve #7620 in v13.

For the modules listed in #7620, I'm currently assessing only Runtime & Driver require soft linking (which is imported/cimported from many places), and the remaining can be done just by lazy-importing. I'm thinking of next try lazy-importing cuSPARSE and see if my assumption is correct.

As Jitify and CUB modules also have C/C++ level dependencies to CTK, we will need lazy-importing for them anyway.

@kmaehashi
Copy link
Member Author

/test mini

@kmaehashi kmaehashi mentioned this pull request Sep 4, 2023
@leofang
Copy link
Member

leofang commented Sep 4, 2023

For the modules listed in #7620, I'm currently assessing only Runtime & Driver require soft linking (which is imported/cimported from many places), and the remaining can be done just by lazy-importing. I'm thinking of next try lazy-importing cuSPARSE and see if my assumption is correct.

As Jitify and CUB modules also have C/C++ level dependencies to CTK, we will need lazy-importing for them anyway.

In light of #7727, I would suggest to always statically link to cudart and soft-link to driver/NVRTC. Then,

  1. this eliminates completely cudart from the discussion
  2. this follows CUDA Python's current approach
  3. the CUB module no longer has external dependency (cudart is its sole dependency)

@kmaehashi
Copy link
Member Author

kmaehashi commented Sep 4, 2023

In light of #7727, I would suggest to always statically link to cudart and soft-link to driver/NVRTC.
...
3. the CUB module no longer has external dependency (cudart is its sole dependency)

Is it safe to mix multiple versions of CUDA runtimes within the same process? If so, let's static link CUDART to CUB to avoid lazy-importing. 😃

@leofang
Copy link
Member

leofang commented Sep 4, 2023

Is it safe to mix multiple versions of CUDA runtimes within the same process?

If you're referring to static linking to cudart, yes it is safe by design. As long as all DSOs see the same user-mode driver (libcuda), which is already upheld by design, it'd work just fine. Think of it as removing cudart from the dependency chain:

any CUDA library -> libcudart -> libcuda

@leofang
Copy link
Member

leofang commented Sep 4, 2023

(btw NVCC by default statically links to cudart, I suspect we don't see that for any CuPy modules because we use gcc instead of nvcc to link, which is also fine, just FYI)

Copy link
Member

@takagi takagi left a comment

Choose a reason for hiding this comment

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

LGTM!

@takagi takagi merged commit 3efd570 into cupy:main Sep 5, 2023
47 of 48 checks passed
@kmaehashi kmaehashi deleted the lazy-load-cusolver branch September 5, 2023 05:40
@kmaehashi
Copy link
Member Author

Thanks for the clarification, Leo! Just submitted a PR to static link cudart to CUB (#7850)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
blocking Issue/pull-request is mandatory for the upcoming release cat:enhancement Improvements to existing features prio:medium
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants