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

Splayed layout support #46

Closed
jakirkham opened this issue Jun 7, 2023 · 14 comments · Fixed by conda-forge/cuda-python-feedstock#83
Closed

Splayed layout support #46

jakirkham opened this issue Jun 7, 2023 · 14 comments · Fixed by conda-forge/cuda-python-feedstock#83
Assignees
Labels
support All things related to the project that can't be categorized

Comments

@jakirkham
Copy link
Collaborator

jakirkham commented Jun 7, 2023

Currently cuda-python relies on all binaries (like nvcc), all headers, and all libraries to live in a single directory (specified by $CUDA_HOME or similar).

However there are use cases (like cross-compilation, as with conda-build) where the build tools may live in one location (and perform builds on that architecture) whereas the headers and libraries may live in a different location (and target a different architecture). In this case not everything lives in $CUDA_HOME.

It would be helpful to have a way of specifying where these different components come from. Here are some options:

  1. Check $NVCC for the nvcc location
  2. Use $CUDA_BIN (if specified) to get build tool directory
  3. Support a list of directories in $CUDA_HOME
  4. ?

Maybe there are other reasonable options worth considering?

@robertmaynard
Copy link

Another option is to support $CUDA_HOME being a list of directories. So for header searching cuda-python would append include to each entry in $CUDA_HOME.

@jakirkham
Copy link
Collaborator Author

Thanks Rob! 🙏

Converted the ideas to an enumerated list and added that idea to the list

@jakirkham
Copy link
Collaborator Author

cc @bdice

@vyasr
Copy link

vyasr commented Dec 19, 2023

Would it make sense to standardize a tool that uses the path to nvcc to parse information out of nvcc.profile? Something that could be used by anyone, not just cuda-python?

@leofang
Copy link
Member

leofang commented Jun 12, 2024

Sorry for lack of response. Is this still an issue? It seems we have been able to build cuda-python on conda-forge?

@jollylili jollylili added the support All things related to the project that can't be categorized label Jun 18, 2024
@jakirkham
Copy link
Collaborator Author

Yep it is still an issue

We workaround it in conda-forge

Through improvements on the packaging side, we have minimized the workarounds needed ( for example: conda-forge/cuda-python-feedstock#75 ), but we are unable to eliminate them without this fix

@leofang
Copy link
Member

leofang commented Jun 25, 2024

@jakirkham I am sorry but I don't get it. In any case we need a way to specify where the CUDA headers are, and setting CUDA_HOME is the right solution that's been working fine. Are there anything specific to the splayed layout that we have not addressed (either in CUDA Python or in conda-forge)?

@jakirkham
Copy link
Collaborator Author

jakirkham commented Jun 25, 2024

Think an ideal solution (from my perspective), would be moving to a CMake-based build system here as CMake already understands how to work with splayed layouts correctly. For example moving to scikit-build-core would very likely solve this issue. This may be desirable anyways to simplify the build process and make it less bespoke

@leofang
Copy link
Member

leofang commented Jun 25, 2024

I still don't understand what specific changes are needed to support splayed layout. @jakirkham this issue might have been outdated since the earlier (CTK 12.0) efforts?

Right now, all headers are expected by CUDA Python to locate in one place, for the purposes of both parsing and compiling. So, it doesn't really matter if headers are in $PREFIX/include or $PREFIX/targets/<platform>/include or /usr/local/cuda/include or any place in the host environment, as long as $CUDA_HOME is set to take a single directory path. Can we please identify which part of CUDA Python is supposed to find headers locating in two or more places (which we'd fail to do so)? I've eyeballed the recent cuda-python recipe and I was unable to find such traces anymore, most likely they were removed starting conda-forge/cuda-python-feedstock#58 and further cleaned up later.

Also, there is no C++ components in this project (at least not yet; there will be once we start working on memory management but it would not happen before this Fall the earliest) and we need to balance with the currently limited engineering resource we have. I don't see the immediate benefit of moving away from setuptools to CMake (+ something that understands CMake like scikit-build-core) when things are already working. That said, if I can enlist a RAPIDS build expert to help with the build system migration, I would not object the change to CMake 😉

@jakirkham
Copy link
Collaborator Author

Nope. This issue was opened because of issues encountered adding CUDA 12 support

Yes CUDA-Python doesn't support splayed layouts. We agree 🙂

The issue is the CUDA compiler and tightly coupled headers and libraries live together. In Conda this means $BUILD_PREFIX (the place where requirements/build are installed). Meanwhile headers and libraries the package uses (IOW requirements/host) are installed to $PREFIX. So $BUILD_PREFIX != $PREFIX, but CUDA Python needs components from both and doesn't know how to handle this

As the wheel ecosystem continues to build out CUDA library wheels, it will likely wind up in the same situation with the same problems

Hopefully that explanation makes more sense. Please feel free to ask more questions

@leofang
Copy link
Member

leofang commented Jun 26, 2024

but CUDA Python needs components from both and doesn't know how to handle this

Could you tell me where this statement comes from? Sorry but I just don't think we are getting the points across and we don't understand each other through this media. Please feel free to arrange an offline meeting.

@robertmaynard
Copy link

but CUDA Python needs components from both and doesn't know how to handle this

Could you tell me where this statement comes from? Sorry but I just don't think we are getting the points across and we don't understand each other through this media. Please feel free to arrange an offline meeting.

I expect the problem is this:

include_dirs = os.path.join(CUDA_HOME, 'include')

@jakirkham
Copy link
Collaborator Author

jakirkham commented Jun 26, 2024

Right like this line

include_path = os.path.join(CUDA_HOME, 'include')


Though am now noticing that in CUDA-Python 12.4.0 there may have been related changes for splayed layouts

CUDA_HOME = CUDA_HOME.split(os.pathsep)

include_path_list = [os.path.join(path, 'include') for path in CUDA_HOME]

So maybe we should give this a try to see if it helps

@jakirkham
Copy link
Collaborator Author

Ok made some changes to the conda-forge build to take advantage of the changes in how CUDA_HOME is handled. This is in PR: conda-forge/cuda-python-feedstock#83

If someone could take a look, that would be appreciated 🙂

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
support All things related to the project that can't be categorized
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants