Use variants to produce separate builds with and without cufile support #754

vyasr · 2025-06-12T23:38:58Z

On arm cufile was not supported until CUDA 12.2, whereas support exists since 12.0 on x86 architectures. To properly reflect these dependencies, we need to build separate variants of cufile on arm for cuda versions before and after 12.2. This PR updates the recipe to support that.

copy-pr-bot · 2025-06-12T23:39:02Z

Auto-sync is disabled for draft pull requests in this repository. Workflows must be run manually.

Contributors can view more details about this message here.

vyasr · 2025-06-12T23:39:07Z

/ok to test

vyasr · 2025-06-12T23:49:10Z

/ok to test

vyasr · 2025-06-13T00:20:07Z

/ok to test

vyasr · 2025-06-13T01:31:53Z

/ok to test

vyasr · 2025-06-13T01:55:09Z

/ok to test

jakirkham

Thanks Vyas! 🙏

This looks generally reasonable

Had a few comments below

jakirkham · 2025-06-13T02:47:59Z

ci/build_cpp.sh


+# Use sed to replace the RAPIDS_CUDA_VERSION string in the variants file with
+# the value of that variable in this shell.
+sed -i "s/RAPIDS_CUDA_VERSION/${RAPIDS_CUDA_VERSION}/g" "conda/recipes/libkvikio/variants_$(arch).yaml"


One thing that has worked well before, is to output this to a variant file

It does a similar thing admittedly. Though it can be a bit clearer about what is happening. That could just be personal preference though

I like that alternative! I agree it is clearer.

I considered this possibility as well. However, in this instance it's not quite so simple as the pynvjitlink example because we need other things to be encoded in the variants as well. The behavior on x86 and arm is different because the former is a single variant while the latter requires two, so we would need to write out variants conditionally based on that. Moreover, for arm we need to use a different compiler version for each CUDA version, so it's a longer file. If you both prefer that I can switch to it, we just wind up with two fairly long here-docs instead of this command. The flip side is that we can drop the committed variants files if we choose to go that route.

I've made the changes in 00b642a. If you like it we can stick with it, otherwise feel free to revert that commit.

conda/recipes/libkvikio/recipe.yaml

jakirkham · 2025-06-13T02:52:08Z

conda/recipes/libkvikio/recipe.yaml

+  #cuda_version: ${{ (env.get("RAPIDS_CUDA_VERSION") | split("."))[:2] | join(".") }}
+  #cuda_major: '${{ (env.get("RAPIDS_CUDA_VERSION") | split("."))[0] }}'


Should we drop these?

I chose to keep them to simplify reverting the changes, but if others feel like it would be cleaner to drop these I am fine with that.

conda/recipes/libkvikio/recipe.yaml

conda/recipes/libkvikio/variants_aarch64.yaml

vyasr · 2025-06-13T05:10:46Z

cpp/include/kvikio/shim/cufile.hpp

-#else
-constexpr int cufile_version() noexcept { return 0; }
-#endif


This change is necessary because the dead code optimizer will generally elide constexpr functions from the final binary if no runtime usage is detected, but we need the symbol in the DSO for usage from other libraries (including Python extensions). For the same reason we also avoid inlining the definition here.

vyasr · 2025-06-13T05:28:10Z

/ok to test

gforsyth · 2025-06-13T16:30:07Z

ci/build_cpp.sh

+# Construct the extra variants according to the architecture
+if [[ "$(arch)" == "x86_64" ]]; then
+    cat > variants.yaml << EOF
+    c_compiler_version:
+      - 13
+
+    cxx_compiler_version:
+      - 13
+
+    cuda_version:
+      - ${RAPIDS_CUDA_VERSION}
+EOF
+else
+    cat > variants.yaml << EOF
+    zip_keys:
+    - [c_compiler_version, cxx_compiler_version, cuda_version]
+
+    c_compiler_version:
+    - 12
+    - 13
+
+    cxx_compiler_version:
+    - 12
+    - 13
+
+    cuda_version:
+    - 12.1 # The last version to not support cufile
+    - ${RAPIDS_CUDA_VERSION}
+EOF
+fi
+


Non-blocking, but I think this would be better to encode in individual variant_$ARCH_$CUDA_VERSION.yaml files and then the branching logic here can be to select the appropriate variant file for the build at hand.

That way the various build variants are all in the conda recipe file and not hidden away in this bash script.

I'm fine with this being done in a follow-up to get CI unblocked.

lol, I see that was your original approach. well, not worth going back and forth over it.

My general take is "repeat yourself" if it keeps things clean, and heredocs in a random bash script isn't worth the tradeoff to me.

Yeah I liked the old way better too. This version feels wrong in how much of the logic is moved out of the recipe itself. I think we should reconsider what approach we like best, but I'll merge as-is to get things unblocked for now.

Provided a simplification on the current approach in PR: #755

bdice

Thank you!

vyasr · 2025-06-13T16:49:52Z

/merge

Now that we have dropped support for CUDA 11 we no longer require the nvidia channel. With the changes in rapidsai/rapids-dask-dependency#85, RAPIDS now only uses released versions of dask, so we no longer need the dask channel either. This PR also removes the explicit cufile dependence in the kvikio conda packages, which should no longer be necessary now that we have variants of the libkvikio package for different CUDA versions handling this dependency (see #754). Contributes to rapidsai/build-planning#184 Authors: - Vyas Ramasubramani (https://github.com/vyasr) Approvers: - James Lamb (https://github.com/jameslamb) URL: #759

vyasr added 4 commits June 12, 2025 14:05

Use preexisting linux64 variable

db41d5f

Add branching logic to recipe

16e8841

First pass at creating variants

015ff8a

Clean up some variable naming

3b33824

vyasr self-assigned this Jun 12, 2025

vyasr added improvement Improves an existing functionality non-breaking Introduces a non-breaking change labels Jun 12, 2025

Style

ee4e25b

Fix zip_keys spec

ca1f016

Avoid constexpr

9563814

Skip tests if not built with cufile support

5c20c2e

vyasr marked this pull request as ready for review June 13, 2025 02:17

vyasr requested review from a team as code owners June 13, 2025 02:17

vyasr requested a review from msarahan June 13, 2025 02:17

jakirkham reviewed Jun 13, 2025

View reviewed changes

vyasr commented Jun 13, 2025

View reviewed changes

vyasr added 2 commits June 13, 2025 05:23

Apply most review comments

fa4342e

Inline variant files via here-docs

00b642a

vyasr requested review from bdice and jakirkham June 13, 2025 13:20

gforsyth approved these changes Jun 13, 2025

View reviewed changes

bdice approved these changes Jun 13, 2025

View reviewed changes

rapids-bot bot merged commit c3509ac into rapidsai:branch-25.08 Jun 13, 2025
49 checks passed

vyasr deleted the feat/cufile_variants branch June 13, 2025 16:52

jakirkham mentioned this pull request Jun 18, 2025

Only use cufile on x86 #753

Merged

vyasr mentioned this pull request Jun 25, 2025

Remove nvidia and dask channels #759

Merged

jakirkham mentioned this pull request Aug 22, 2025

Remove CUDA 11 references rapidsai/cucim#905

Merged

kingcrimsontianyu mentioned this pull request Sep 8, 2025

Skip max_device_cache_size setter when BAR1 memory isn't present on the GPUs in the system #814

Merged

		#cuda_version: ${{ (env.get("RAPIDS_CUDA_VERSION") \| split("."))[:2] \| join(".") }}
		#cuda_major: '${{ (env.get("RAPIDS_CUDA_VERSION") \| split("."))[0] }}'

Use variants to produce separate builds with and without cufile support #754

Use variants to produce separate builds with and without cufile support #754

Uh oh!

Conversation

vyasr commented Jun 12, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

copy-pr-bot bot commented Jun 12, 2025

Uh oh!

vyasr commented Jun 12, 2025

Uh oh!

vyasr commented Jun 12, 2025

Uh oh!

vyasr commented Jun 13, 2025

Uh oh!

vyasr commented Jun 13, 2025

Uh oh!

vyasr commented Jun 13, 2025

Uh oh!

jakirkham left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

vyasr Jun 13, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

vyasr Jun 13, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

vyasr commented Jun 13, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

bdice left a comment

Choose a reason for hiding this comment

Uh oh!

vyasr commented Jun 13, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

vyasr commented Jun 12, 2025 •

edited

Loading

vyasr Jun 13, 2025 •

edited

Loading

vyasr Jun 13, 2025 •

edited

Loading