-
Notifications
You must be signed in to change notification settings - Fork 74
Relax libcufile dependency for old CTK 12 on ARM
#930
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
Changes from all commits
b0e17af
f5f3ae9
584e88c
f6f2d7d
4dbfd5a
2063e7c
61ab317
e447f6b
9dd7bff
99975e2
73dd613
f3fa2b1
5dbbb43
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -15,13 +15,13 @@ source: | |
|
|
||
| build: | ||
| number: {{ GIT_DESCRIBE_NUMBER }} | ||
| string: cuda{{ cuda_major }}_{{ date_string }}_{{ GIT_DESCRIBE_HASH }}_{{ GIT_DESCRIBE_NUMBER }} | ||
| string: cuda{{ cuda_major }}_{{ date_string }}_gh{{ GIT_DESCRIBE_HASH }}_gn{{ GIT_DESCRIBE_NUMBER }}}_ph{{ PKG_HASH }} | ||
|
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Since we are building for two different variants of |
||
| ignore_run_exports: | ||
| - openslide | ||
| ignore_run_exports_from: | ||
| - {{ compiler('cuda') }} | ||
| - cuda-cudart-dev | ||
| - libcufile-dev # [linux64] | ||
| - libcufile-dev | ||
|
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We want to ignore the This avoids overly tight version constraints that otherwise cause solver errors when testing CUDA 12.9 packages on CUDA 12.0 |
||
| - libnvjpeg-dev | ||
| script_env: | ||
| - AWS_ACCESS_KEY_ID | ||
|
|
@@ -60,9 +60,16 @@ requirements: | |
| - nvtx-c >=3.1.0 | ||
| - openslide | ||
| run: | ||
| {% if aarch64 and cuda_major == "12" %} | ||
| # Relax `libcufile` dependency for old CUDA 12 ARM installs | ||
| - {{ pin_compatible('cuda-version', lower_bound='12.0', upper_bound='12.2') }} # [not has_cufile] | ||
| - {{ pin_compatible('cuda-version', lower_bound='12.2', upper_bound='13.0') }} # [has_cufile] | ||
| - libcufile # [has_cufile] | ||
| {% else %} | ||
|
Comment on lines
+63
to
+68
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This ensures ARM packages that support CUDA 12.2+ still depend io cuFile whereas packages that require CUDA 12.0 or 12.1 packages on ARM don't require cuFile (as it is not given in this case) |
||
| - {{ pin_compatible('cuda-version', max_pin='x', min_pin='x') }} | ||
| - cuda-cudart | ||
| - libcufile | ||
| {% endif %} | ||
| - cuda-cudart | ||
| - libnvjpeg | ||
| run_constrained: | ||
| - {{ pin_compatible('openslide') }} | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -16,12 +16,7 @@ | |
| #ifndef CUCIM_CUFILE_STUB_H | ||
| #define CUCIM_CUFILE_STUB_H | ||
|
|
||
| // Try to include the real cufile.h, fall back to minimal types if not available | ||
| #if __has_include(<cufile.h>) | ||
| #include <cufile.h> | ||
| #else | ||
| #include "cufile_stub_types.h" | ||
| #endif | ||
| #include <cufile.h> | ||
|
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Trying to In file included from /opt/conda/conda-bld/work/gds/src/cufile_stub.cpp:17:
/opt/conda/conda-bld/work/gds/include/cufile_stub.h:23:14: fatal error: cufile_stub_types.h: No such file or directory
23 | #include "cufile_stub_types.h"
| ^~~~~~~~~~~~~~~~~~~~~
compilation terminated.Given we either have a new enough CUDA Toolkit and it has cuFile or it doesn't, we can simply check for cuFile and build or not Unsure what we gain by having this in-between mode. Though would be happy to learn if I'm missing something |
||
|
|
||
| #include "cucim/dynlib/helper.h" | ||
|
|
||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks like the current logic will have both in the aarch64+CUDA12 case?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let me commit this and see if it helps
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes this is intended. We want to build once for each of these values, which is what the original logic does
Also for CUDA 13+ or
x86_64builds we want this to only beTrueThe CI failure is due to something else unrelated to this syntax. Please see this comment: #930 (comment)
Have gone ahead and restored the original logic. Though happy to chat more if you have additional questions 🙂