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.

Sign up for GitHub

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

Jump to bottom

Adding cuda-cudart #21723

Merged
merged 45 commits into from
Apr 12, 2023
Merged

Adding cuda-cudart #21723

Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
45 commits
Select commit Hold shift + click to select a range
5e7ab63
Adding cuda-cudart
adibbley Jan 13, 2023
54e9ad1
Update license
adibbley Jan 13, 2023
77d5387
Set targets directory linux only
adibbley Jan 13, 2023
cb4ddf1
Fixup windows libs
adibbley Jan 13, 2023
aeabc3a
Test block expected above outputs
adibbley Jan 13, 2023
753fdda
Fixing copy paste in test
adibbley Jan 13, 2023
2a95315
Update recipes/cuda-cudart/meta.yaml
adibbley Jan 14, 2023
cb06971
Update recipes/cuda-cudart/meta.yaml
adibbley Jan 14, 2023
e47fc74
Update recipes/cuda-cudart/meta.yaml
adibbley Jan 14, 2023
47c0e12
Add targets include directory
adibbley Jan 16, 2023
1a3b333
Pick up a few more missing headers
adibbley Jan 16, 2023
f8e274b
Update Windows install paths
adibbley Jan 17, 2023
c0cd174
Adding pkgconfigs
adibbley Jan 18, 2023
3ab1ad9
Moving post-link.sh into build.sh
adibbley Jan 19, 2023
dc4a28d
Fixing run/dev split on windows and adding output tests
adibbley Jan 20, 2023
dada5b9
Add cuda-cudart-static and cuda-driver-dev tests
adibbley Jan 20, 2023
ec2a827
Add run_exports for cuda-cudart-dev
adibbley Jan 23, 2023
3d09008
Flip dev to require static package
adibbley Jan 23, 2023
7fdd23e
Update recipes/cuda-cudart/meta.yaml
adibbley Jan 24, 2023
d966b9a
Add cross package proposal
adibbley Feb 2, 2023
855014c
Fix dependencies and set cross packages to noarch
adibbley Feb 2, 2023
c098e5d
Flip build/requirements in outputs
adibbley Feb 2, 2023
039461a
Enable target package for windows
adibbley Feb 6, 2023
a3c7bae
Adding run_exports to dev packages
adibbley Feb 6, 2023
fae8026
More specific wildcards for runtime package
adibbley Feb 7, 2023
7fd615f
Adding cuda-cccl dependency
adibbley Feb 14, 2023
cbce911
Update recipes/cuda-cudart/meta.yaml
adibbley Mar 15, 2023
d0847bb
Apply suggestions from code review
adibbley Mar 17, 2023
9278294
Apply suggestions from code review
adibbley Mar 22, 2023
a974fbb
Merge branch 'main' into cuda-cudart
jakirkham Mar 27, 2023
f4c6f2d
Update recipe.
bdice Mar 31, 2023
7b0d9a7
Depend on cuda-cccl_{{ target_platform }} in cuda-cudart-dev_{{ targe…
bdice Apr 4, 2023
5eb9324
Merge pull request #2 from bdice/cuda-cudart
adibbley Apr 4, 2023
f239ddc
Fix stubs/libcuda.so link
adibbley Apr 5, 2023
5106c80
Add about sections to all outputs
adibbley Apr 5, 2023
26d7a52
Update recipes/cuda-cudart/meta.yaml
adibbley Apr 5, 2023
97d62e3
Move cuda-cccl to run requirement
adibbley Apr 5, 2023
097daa9
Apply suggestions from code review
adibbley Apr 5, 2023
e48d9b1
Merge branch 'main' into cuda-cudart
jakirkham Apr 7, 2023
01063be
Merge branch 'main' into cuda-cudart
jakirkham Apr 7, 2023
52bd85d
Skip files section for Windows metapackages
adibbley Apr 11, 2023
20212b9
Update recipes/cuda-cudart/meta.yaml
adibbley Apr 11, 2023
622cf4d
Fix cuda-cudart tests.
bdice Apr 11, 2023
251fa03
Merge pull request #4 from bdice/fix-cudart-tests
adibbley Apr 11, 2023
2af2140
Normalize the whitespace to match the other lines like this.
adibbley Apr 11, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 17 additions & 0 deletions recipes/cuda-cudart/bld.bat
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
if not exist %PREFIX% mkdir %PREFIX%

rem Directories...
for /D %%i in (.\*) do (
robocopy /E %%i %PREFIX%\%%i
)

rem Files...
for %%i in (.\*) do (
if not %%~ni==build_env_setup (
if not %%~ni==conda_build (
if not %%~ni==metadata_conda_debug (
xcopy %%i %PREFIX%
)
)
)
)
24 changes: 24 additions & 0 deletions recipes/cuda-cudart/build.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
#!/bin/bash

# Install to conda style directories
[[ -d lib64 ]] && mv lib64 lib

[[ ${target_platform} == "linux-64" ]] && targetsDir="targets/x86_64-linux"
[[ ${target_platform} == "linux-ppc64le" ]] && targetsDir="targets/ppc64le-linux"
[[ ${target_platform} == "linux-aarch64" ]] && targetsDir="targets/sbsa-linux"

for i in `ls`; do
[[ $i == "build_env_setup.sh" ]] && continue
[[ $i == "conda_build.sh" ]] && continue
[[ $i == "metadata_conda_debug.yaml" ]] && continue
if [[ $i == "lib" ]] || [[ $i == "include" ]]; then
mkdir -p ${PREFIX}/${targetsDir}
mkdir -p ${PREFIX}/$i
cp -rv $i ${PREFIX}/${targetsDir}
for j in `ls $i`; do
ln -s ../${targetsDir}/$i/$j ${PREFIX}/$i/$j
done
else
cp -rv $i ${PREFIX}
fi
done
89 changes: 89 additions & 0 deletions recipes/cuda-cudart/meta.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,89 @@
{% set name = "cuda-cudart" %}
{% set version = "12.0.107" %}
adibbley marked this conversation as resolved.
Show resolved Hide resolved
{% set platform = "linux-x86_64" %} # [linux64]
{% set platform = "linux-ppc64le" %} # [ppc64le]
{% set platform = "linux-sbsa" %} # [aarch64]
{% set platform = "windows-x86_64" %} # [win]
{% set extension = "tar.xz" %} # [not win]
{% set extension = "zip" %} # [win]

package:
name: {{ name|lower }}
Copy link
Member

Choose a reason for hiding this comment

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

Currently the top-level package and one of the outputs have the same name. We should change these to different names to avoid issues. We could rename the top-level name to something like (cuda-cudart-split) or we could rename the library to cuda-cudart-shared (or similar)

Copy link
Member

Choose a reason for hiding this comment

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

cuda-cudart-split is better

Copy link
Member

Choose a reason for hiding this comment

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

ping

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Can I get some context on the issue here please? Ideally we would have consistent naming for all component feedstocks without needing to know if that component has multiple outputs.

Copy link
Contributor

Choose a reason for hiding this comment

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

I think the issue is that the top level package and the output defined on line 40 have the same name which makes things ambiguous at best and likely breaks things.

Copy link
Member

Choose a reason for hiding this comment

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

That sounds right. It use to be the case different names were required just to get through staged-recipes or run conda-build.

Though it is possible this is less of an issue than has been in the past. For example libnuma was added in PR ( #21176 ) and use the same top-level package name and output name. The feedstock still follows this pattern. AFAICT it is working ok.

So maybe this isn't actually an issue anymore

version: {{ version }}

source:
url: https://developer.download.nvidia.com/compute/cuda/redist/cuda_cudart/{{ platform }}/cuda_cudart-{{ platform }}-{{ version }}-archive.{{ extension }}
sha256: c0e16e9593a69afe4997ae96a8b024e751ebce0254523fd969b08bb028582d23 # [linux64]
sha256: d785fd613e35e3761fdb0016a31a56417f570605cbfb9a93ca9dc68be6789188 # [ppc64le]
sha256: 5de248c6c203f6c91ce34256bbfc028bc87c1b0c552f9f51edb65e60ad665f63 # [aarch64]
sha256: 1cd8516feee068c6d718453e76b0dfcbee27844a35d4f3aa608a3d316190309c # [win]

build:
number: 0
skip: true # [osx]
Copy link
Contributor

Choose a reason for hiding this comment

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

Windows CI appears to be failing because of overlinking.

conda_build.exceptions.OverLinkingError: overlinking check failed 
['  ERROR (cuda-cudart,Library/bin/cudart64_12.dll): $RPATH/api-ms-win-core-libraryloader-l1-2-0.dll not found in packages, sysroot(s) nor the missing_dso_whitelist.\n.. is this binary repackaging?', '  ERROR (cuda-cudart,Library/bin/cudart64_12.dll): $RPATH/api-ms-win-security-systemfunctions-l1-1-0.dll not found in packages, sysroot(s) nor the missing_dso_whitelist.\n.. is this binary repackaging?']
##[error]Cmd.exe exited with code '1'.

Seems related to the problems mentioned in previous PRs here:

The solution in #21924 was disabling the overlinking check. Is this what we should do, or is there a different approach?

Suggested change
build:
number: 0
skip: true # [osx]
build:
number: 0
skip: true # [osx]
error_overlinking: false # [win]

Copy link
Member

Choose a reason for hiding this comment

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

At one point the recipes in the aforementioned PR had error_overlinking ( 3ed95cf ), but it seems to be reverted later ( 96f6ad0 ). Am not seeing error_overlinking in the final changes in that PR or in the recipes. So think Leo came up with another solution, but it is not obvious what it was based on the commit history of the PR what it was. Might be worth studying the libnvjitlink recipe more closely (since this is where it was needed and later dropped)

Copy link
Contributor

Choose a reason for hiding this comment

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

Hmm. I looked over the diff between those commits and compared the requirements of those packages to cuda-cudart but I'm not seeing much that points to the original cause or solution.

Copy link
Member

@jakirkham jakirkham Apr 7, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

Yeah it wasn't obvious to me how this was fixed after a cursory look. Though assumed I had just missed something

Let's try merging in main (as we discussed offline) and see if the issue persists. Did this below ( 01063be )

Copy link
Member

Choose a reason for hiding this comment

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

Still seeing the same issue after updating the branch. This Microsoft support thread looks relevant

Copy link
Member

Choose a reason for hiding this comment

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

So I do see api-ms-win-core-libraryloader-*.dll in the vc package, but the SONAME(?) appears different. Maybe we are getting the wrong version?

With api-ms-win-security-systemfunctions-*.dll, it doesn't not appear to be in that list. Unclear as to whether it should be (possibly in a newer version?)

Copy link
Member

Choose a reason for hiding this comment

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

My impression was that after I followed the libcublas approach and added compiler to build, I no longer saw warnings on the dlls, so I assumed it's safe to undo the WAR and gave it a shot.

Another issue was that I noticed the internal recipe had a wrong cusparse/nvjitlink version, mismatching the CUDA 12.0 one, so I fixed it and that change might also explain. Not sure which is the real key.

Copy link
Member

Choose a reason for hiding this comment

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

Thanks Leo! 🙏 Really appreciate you taking time to chime in here 🙂

Yeah the compilers part seems like the key. Did look at that. Though this recipe already seems to include these for multiple packages.

That said, I do see noarch: generic in a few places where we are adding compilers and doing Linux/Windows specific things. Maybe these are holdovers from before we started doing architecture specific things. Not sure if they are causing this problem, but would expect them to cause some issues. So we may want to drop them. Have outlined them in review comments below

Copy link
Contributor

Choose a reason for hiding this comment

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

Discussed this overlinking issue with @jakirkham / @adibbley. We agreed to skip Windows for now, and then work on fixing this problem in the feedstock. That way Linux packages can be unblocked and we can continue on cuda-nvcc while we investigate the Windows overlinking further.

Suggested change
build:
number: 0
skip: true # [osx]
build:
number: 0
skip: true # [osx or win]

Copy link
Member

Choose a reason for hiding this comment

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

Raised feedstock issue ( conda-forge/cuda-cudart-feedstock#1 ) for further discussion/investigation.

run_exports:
strong:
- __glibc >=2.17 # [linux]
adibbley marked this conversation as resolved.
Show resolved Hide resolved

requirements:
build:
- {{ compiler("c") }}
- {{ compiler("cxx") }}
host:
- sysroot_{{ target_platform }} 2.17 # [linux]
- __glibc >=2.17 # [linux]
run:
- sysroot_{{ target_platform }} 2.17 # [linux]
- __glibc >=2.17 # [linux]

test:
commands:
- test -f $PREFIX/lib/libcudart.so.{{ version }} # [unix]
- if not exist %LIBRARY_LIB%\\cudart.lib exit 1 # [win]

outputs:
- name: cuda-cudart
jakirkham marked this conversation as resolved.
Show resolved Hide resolved
files:
- lib/*.so.* # [linux]
- targets/*/lib/*.so.* # [linux]
- lib/x64/cuda.lib # [win]
- lib/x64/cudart.lib # [win]

- name: cuda-cudart-dev
jakirkham marked this conversation as resolved.
Show resolved Hide resolved
jakirkham marked this conversation as resolved.
Show resolved Hide resolved
requirements:
run:
- cuda-cudart >={{ version }}
files:
- lib/*.so # [linux]
- targets/*/lib/*.so # [linux]
- include
- lib/x64/cudadevrt.lib # [win]
- bin # [win]

- name: cuda-cudart-static
adibbley marked this conversation as resolved.
Show resolved Hide resolved
requirements:
run:
- cuda-cudart-dev >={{ version }}
files:
- lib/*.a # [linux]
- targets/*/lib/*.a # [linux]
- lib/x64/cudart_static.lib # [win]

- name: cuda-driver-dev # [linux]
adibbley marked this conversation as resolved.
Show resolved Hide resolved
files: # [linux]
- lib/stubs/libcuda.so # [linux]
- targets/*/lib/stubs/libcuda.so # [linux]
adibbley marked this conversation as resolved.
Show resolved Hide resolved

about:
home: https://developer.nvidia.com/cuda-toolkit
license_file: LICENSE
license: LicenseRef-NVIDIA-End-User-License-Agreement
license_url: https://docs.nvidia.com/cuda/eula/index.html
summary: CUDA Runtime native Libraries
description: |
CUDA Runtime native Libraries
doc_url: https://docs.nvidia.com/cuda/index.html

extra:
recipe-maintainers:
- adibbley