-
Notifications
You must be signed in to change notification settings - Fork 11
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
cmake updates worth considering #9
Comments
We are working on updating CMake; first, we are trying to get it to work with GitHub actions. It looks to me that the installation instructions are not quite right. I've removed the use of find_package(CUDA), but it still has an issue finding/linking the cuda libraries. I'm wondering, though, if we should be using FindCUDAToolkit instead. Currently, we are experimenting with it at, no successful build yet: |
Always an adventure 🙂 Are CMake changes needed to get them running at all? It could be an environment problem, I don't see anything overtly wrong with the current system -- it's just the old cmake style.
The process is a bit more involved, partly because you have to adopt some target-based approaches related to the packaging. From what I can see this library needs to be able to (1) compile and link against the cuda runtime and (2) specifically Line 36 in 4906c76
is going to ultimately include / link against Lines 85 to 93 in 4906c76
where find_package(CUDAToolkit)
# ... at some point after `add_library` aka this target exists ...
# note: you can `add_library(target-name "")` at the beginning of your CMake with no sources
# so that the target exists immediately and then do `target_sources` later on, mentioning because
# your dependencies search happens before the `add_subdirectory`
# https://cmake.org/cmake/help/latest/module/FindCUDAToolkit.html#cuda-toolkit-rt-lib
target_link_libraries(hdf5_vfd_gds PUBLIC CUDA::cudart) # or CUDA::cudart_static but less common Similarly for Line 73 in 4906c76
just do it after finding Note that once you start linking against targets in your build system, consumers need it in your config file. Hope that helps clarify some! |
Indeed, if I don't use PR #5 CMake changes, it compiles successfully. But when I try to run the tests it fails because it is looking for libOpenCL.so |
Hmm, that does seem like an installation problem. That's a pretty critical library to be missing (that's the driver api, as opposed to the runtime api vfd-gds/.github/workflows/main.yml Line 25 in 69d0c16
I'm not sure about the ubuntu packaging and the Alternatively if you wanted to get fancy you might be able to download the runfile and cache the download rather than reinstalling the cuda packages each time. Or possibly use one of the cuda docker images?
All that said, even if |
Right, I don't think it works without a GPU attached. So we decided to leave GitHub actions to test if it builds only. I'm not aware of any ECP machines which support GPU storage direct. Is there currently a system that E4S will be installed on that supports it that you know of? |
I am so sorry, it turns out this package was more in the "nice to have" category. I mistakenly thought one of the sites did have GDS enabled which is why I got so excited about just stamping a new release w/o any cmake changes 🙃 |
Redirect from #8 (comment)
This project should consider
find_package(CUDA)
. You only need the cuda runtime for this project from a quick glance (so no need to haveproject(... LANGUAGES CUDA)
since no CUDA source files. The update process may be painful depending on what you were or were not using fromfind_package(CUDA)
. You should only need to link againstCUDA::cudart
from a cursory glance.hdf5-shared
shows up in the generated cmake config files that are installed. That may be problematic for an application that is consuminghdf5-static
? Unclear what the right solution is.NOTE 4
at the bottom of the linked code.The text was updated successfully, but these errors were encountered: