Use CUDA version ranges in release selector. #524

bdice · 2024-07-22T21:58:13Z

This PR uses CUDA version ranges in the release selector. This follows from discussions in #522.

netlify · 2024-07-22T21:59:27Z

✅ Deploy Preview for docs-rapids-ai ready!

Built without sensitive environment variables

Name	Link
🔨 Latest commit	`ab1bbce`
🔍 Latest deploy log	https://app.netlify.com/sites/docs-rapids-ai/deploys/669f26653dfcc00008065d58
😎 Deploy Preview	https://deploy-preview-524--docs-rapids-ai.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

bdice · 2024-07-22T22:38:27Z

@jarmak-nv @jakirkham Can you review and play with the site preview to see if this works like you expect?

I tried to get one button for conda to have CUDA 12.0-12.2 that would change to 12.0-12.5 when enabling nightlies but I didn't want to mess with any more of this selector logic at the moment.

jakirkham

Thanks Bradley! 🙏

Wonder if we can generalize this a bit to simplify the updating cycle for new CUDA versions

Took a first pass at this below, but likely more needs to change (like Docker). Also there may be better ways to capture the same logic (like a bool variable that simply states whether nightly and stable versions match)

jakirkham · 2024-07-22T22:56:38Z

_includes/selector.html

@@ -371,8 +380,9 @@

            // all possible values
            python_vers: ["3.9", "3.10", "3.11"],
-            cuda_vers: ["11.2", "11.8", "12.0", "12.2", "12.5"],
-            pip_cuda_vers: ["11.2 - 11.8", "12"],
+            conda_cuda_vers: ["11.4 - 11.8", "12.0 - 12.2", "12.0 - 12.5"],


Suggested change

conda_cuda_vers: ["11.4 - 11.8", "12.0 - 12.2", "12.0 - 12.5"],

conda_cuda_vers: ["11.4 - 11.8", "12.0 - 12.2", "12.0 - 12.5"],

conda_cuda_ver_nightly: "12.0 - 12.5",

// TODO: Change to "12.0 - 12.5" once RAPIDS 24.08 releases

conda_cuda_ver_stable: "12.0 - 12.2",

jakirkham · 2024-07-22T22:57:31Z

_includes/selector.html

+                // Remove once stable releases support CUDA 12.5
+                if (this.active_release === "Stable" && this.active_conda_cuda_ver === "12.0 - 12.5") {
+                    this.active_conda_cuda_ver = "12.0 - 12.2";
+                }
+                if (this.active_release === "Stable" && this.active_docker_cuda_ver === "12.5") {
+                    this.active_docker_cuda_ver = "12.2";


Suggested change

// Remove once stable releases support CUDA 12.5

if (this.active_release === "Stable" && this.active_conda_cuda_ver === "12.0 - 12.5") {

this.active_conda_cuda_ver = "12.0 - 12.2";

}

if (this.active_release === "Stable" && this.active_docker_cuda_ver === "12.5") {

this.active_docker_cuda_ver = "12.2";

if (this.active_release === "Stable" && this.active_conda_cuda_ver === this.conda_cuda_ver_nightly) {

this.active_conda_cuda_ver = this.conda_cuda_ver_nightly;

}

if (this.active_release === "Stable" && this.active_docker_cuda_ver === "12.5") {

this.active_docker_cuda_ver = "12.2";

jakirkham · 2024-07-22T22:59:34Z

_includes/selector.html

+                // Remove once stable releases support CUDA 12.5
+                if (this.active_release === "Stable" && cuda_version.includes("12.5")) isDisabled = true;


Suggested change

// Remove once stable releases support CUDA 12.5

if (this.active_release === "Stable" && cuda_version.includes("12.5")) isDisabled = true;

if (this.active_release === "Stable" && cuda_version == conda_cuda_ver_nightly) isDisabled = (conda_cuda_ver_stable != conda_cuda_ver_nightly);

jakirkham · 2024-07-22T23:00:46Z

_includes/selector.html

@@ -789,7 +808,7 @@
                    return;
                }
                this.active_additional_packages = [...this.active_additional_packages, package];
-                if (this.active_additional_packages.includes("TensorFlow") && (this.active_cuda_ver !== "12.0")) this.active_cuda_ver = "12.0";
+                if (this.active_additional_packages.includes("TensorFlow") && (!this.active_conda_cuda_ver.includes("12.0"))) this.active_conda_cuda_ver = "12.0 - 12.2";


Suggested change

if (this.active_additional_packages.includes("TensorFlow") && (!this.active_conda_cuda_ver.includes("12.0"))) this.active_conda_cuda_ver = "12.0 - 12.2";

if (this.active_additional_packages.includes("TensorFlow") && (!this.active_conda_cuda_ver.includes("12.0"))) this.active_conda_cuda_ver = this.conda_cuda_ver_stable;

jarmak-nv · 2024-07-23T00:02:56Z

Logic seems good, no issues after lots of clicks/testing.

I think having both 12.0-12.2 and 12.0-12.5 buttons is a bit awkward though. While I like the specificity of ranges, I think I'm still lightly in favor of just cuda 11 cuda 12 and cuda 12 nightly or something.

jakirkham · 2024-07-23T00:08:30Z

If we don't show the version ranges on the buttons, where do we specify these ranges?

jakirkham · 2024-07-23T02:58:45Z

Thanks Bradley! 🙏

This is looking pretty good

Noticed one thing with pip that I wanted to highlight (though maybe you already saw this)

All the others look ok. So probably something particular to the pip case

bdice · 2024-07-23T03:00:33Z

@jakirkham Glad this is going in the right direction -- I'm still working on some improvements, and I'm using CI as my test for website rendering. I may push some commits that don't work. I will let you know when this is ready for a final look.

This reverts commit a1fdc38.

bdice · 2024-07-23T03:37:44Z

@jakirkham @jarmak-nv I think this is ready to review.

jakirkham

Thanks Bradley! 🙏

Played around with the install selector. Liked the ergonomics of these improvements. Also felt like it was clear from a user perspective what they were getting. Not to mention they could further refine it if needed (like picking a specific CUDA version or working with other dependencies that have such constraints). Tried various combinations of selections to make sure the output was looking as expected

Think it is great that the code is being generalized so that we have fewer hard-coded versions in it. That will only benefit us longer term

Noted a couple spots that still have some hard-coding (likely not a focus on the recent revisions). Am curious if there are ways we can turn those into variables as well. Would be interested to hear your thoughts on this 🙂

jakirkham · 2024-07-23T05:42:21Z

_includes/selector.html

+                var cuda_version_info = {
+                    "Stable": {
+                        "11": ["11.4", "11.8"],
+                        "12": ["12.0", "12.2"]
+                    },
+                    "Nightly": {
+                        "11": ["11.4", "11.8"],
+                        "12": ["12.0", "12.5"]
+                    }
+                };


Wonder if we can collect this info next to the "all possible values" section. That way there would be one place to change all of the versions

I struggled with this. I don't know why, but I couldn't get the JavaScript to work happily when I did that. See my attempt here (I force-pushed it out of the history for this PR since it wasn't going well): 3a6e9a5

jakirkham · 2024-07-23T05:44:00Z

_includes/selector.html

+                if (this.active_release === "Stable" && this.active_docker_cuda_ver === "12.5") {
+                    this.active_docker_cuda_ver = "12.2";
                }


What do you think about capturing 12.5 & 12.2 in variables in the "all possible values" section?

Perhaps we could do something like...?

Suggested change

if (this.active_release === "Stable" && this.active_docker_cuda_ver === "12.5") {

this.active_docker_cuda_ver = "12.2";

}

if (this.active_release === "Stable" && this.active_docker_cuda_ver === this.nightly_docker_cuda_ver) {

this.active_docker_cuda_ver = this.stable_docker_cuda_ver;

}

Could we leave this for a later PR? I think this ties in with the above thread, where I struggled to get a "structured" JSON format to work.

jakirkham · 2024-07-23T05:45:24Z

_includes/selector.html

@@ -673,8 +702,7 @@
            },
            disableUnsupportedCuda(cuda_version) {
                var isDisabled = false;
-                if (this.active_additional_packages.includes("TensorFlow") && (cuda_version !== "12.0")) isDisabled = true;
-                if (this.active_method === "Docker" && cuda_version < "11.8") isDisabled = true;
+                if (this.active_additional_packages.includes("TensorFlow") && (!cuda_version.startsWith("12"))) isDisabled = true;
                if (this.active_release === "Stable" && cuda_version === "12.5") isDisabled = true;


Curious if we could use a variable for 12.5 here as well

Could we leave this for a later PR? I think this ties in with the above thread, where I struggled to get a "structured" JSON format to work.

_includes/selector.html

jarmak-nv · 2024-07-23T18:45:05Z

If we don't show the version ranges on the buttons, where do we specify these ranges?

Yeah that's the challenge, my thought is that the selector should not be the main way of communicating what ranges of CUDA 12 the user can install alongside RAPIDS, but it does serve that purpose. Ranges work for this and the current implementation looks very nice/clean so I'm happy here.

I like how this works and the back-end changes! Thanks @bdice

Only concern is we no longer highlight that the nightly has a higher CUDA version available than the stable. Not sure if there's a straightforward path to resolve this or if it's a big deal.

Also on testing I noticed something I think might be considered undesirable, when selecting pip as the installation method, it moves you to CUDA 12 regardless of your previous selection. I think recently we've been settling on "don't change something the user has set unless the selection cannot work" and in that design theme we'd want to remove this behavior.

I think its controlled by the code here:
https://github.com/rapidsai/docs/blob/ab1bbce41968893d9f0d03aff60fa06fac5725f7/_includes/selector.html#L771C1-L773C18

bdice · 2024-07-23T21:13:37Z

Only concern is we no longer highlight that the nightly has a higher CUDA version available than the stable. Not sure if there's a straightforward path to resolve this or if it's a big deal.

I am leaning towards "this is not a big deal." I think most users are happy as long as some CUDA minor version for the selected major version works with their environment.

bdice · 2024-07-23T21:14:38Z

Also on testing I noticed something I think might be considered undesirable, when selecting pip as the installation method, it moves you to CUDA 12 regardless of your previous selection.

I will revisit this in a follow-up PR. I have a solution in mind but I think this PR is in a fairly complete state and would be better to merge as is.

jakirkham · 2024-07-23T22:55:00Z

Only concern is we no longer highlight that the nightly has a higher CUDA version available than the stable. Not sure if there's a straightforward path to resolve this or if it's a big deal.

I am leaning towards "this is not a big deal." I think most users are happy as long as some CUDA minor version for the selected major version works with their environment.

Agreed. The window where these differ is usually short. So don't think it is worth the effort

jakirkham · 2024-07-23T22:56:23Z

FWIW am ok merging and iterating in follow ups. This feels like a substantial improvement

jarmak-nv

FWIW am ok merging and iterating in follow ups. This feels like a substantial improvement

Same! Thanks @bdice

jakirkham · 2024-07-24T16:41:21Z

Thank you both! 🙏

Use CUDA version ranges in release selector.

fd62f53

bdice force-pushed the cuda-version-ranges branch from ae6c328 to 5a60880 Compare July 22, 2024 22:24

bdice added 6 commits July 22, 2024 17:24

Fix active names.

5a60880

Fix missing paren.

6fca371

Fix extra space.

c0fc8ec

Generate pinning.

8472be0

Handle Docker stable version reset.

f539281

Permit patch version to have .*.

269c18d

bdice marked this pull request as ready for review July 22, 2024 22:36

bdice requested a review from a team as a code owner July 22, 2024 22:36

jakirkham reviewed Jul 22, 2024

View reviewed changes

bdice force-pushed the cuda-version-ranges branch 3 times, most recently from ec821ee to 3a6e9a5 Compare July 23, 2024 02:33

Add CUDA version pinnings.

c68ec79

bdice force-pushed the cuda-version-ranges branch from c99bd91 to c68ec79 Compare July 23, 2024 02:49

Try to fix labels.

193ca3b

bdice added 6 commits July 22, 2024 22:02

Try to fix conda command.

9641a92

Update pinning.

57a0d13

Try selecting only node contents.

a1fdc38

Revert "Try selecting only node contents."

6437ef1

This reverts commit a1fdc38.

Fix pip selector issue.

4f736ed

Disable CUDA 12.5 in Docker Stable builds.

4f00791

bdice requested a review from jarmak-nv July 23, 2024 03:37

bdice requested a review from jakirkham July 23, 2024 03:37

bdice added 2 commits July 22, 2024 22:40

Reset Docker CUDA version when using Stable release.

6807a75

Simplify TensorFlow CUDA 12 option.

ab1bbce

bdice self-assigned this Jul 23, 2024

jakirkham reviewed Jul 23, 2024

View reviewed changes

jarmak-nv approved these changes Jul 24, 2024

View reviewed changes

bdice merged commit bbc384f into rapidsai:main Jul 24, 2024
5 checks passed

raydouglass mentioned this pull request Aug 8, 2024

Updates for RAPIDS 24.08 #529

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Use CUDA version ranges in release selector. #524

Use CUDA version ranges in release selector. #524

bdice commented Jul 22, 2024

netlify bot commented Jul 22, 2024 •

edited

Loading

bdice commented Jul 22, 2024

jakirkham left a comment

jakirkham Jul 22, 2024

jakirkham Jul 22, 2024

jakirkham Jul 22, 2024

jakirkham Jul 22, 2024

jarmak-nv commented Jul 23, 2024

jakirkham commented Jul 23, 2024

jakirkham commented Jul 23, 2024

bdice commented Jul 23, 2024

bdice commented Jul 23, 2024

jakirkham left a comment

jakirkham Jul 23, 2024

bdice Jul 23, 2024

jakirkham Jul 23, 2024

bdice Jul 23, 2024

jakirkham Jul 23, 2024

bdice Jul 23, 2024

jarmak-nv commented Jul 23, 2024

bdice commented Jul 23, 2024

bdice commented Jul 23, 2024

jakirkham commented Jul 23, 2024

jakirkham commented Jul 23, 2024

jarmak-nv left a comment

jakirkham commented Jul 24, 2024

-            conda_cuda_vers: ["11.4 - 11.8", "12.0 - 12.2", "12.0 - 12.5"],
+            conda_cuda_vers: ["11.4 - 11.8", "12.0 - 12.2", "12.0 - 12.5"],
+            conda_cuda_ver_nightly: "12.0 - 12.5",
+            // TODO: Change to "12.0 - 12.5" once RAPIDS 24.08 releases
+            conda_cuda_ver_stable: "12.0 - 12.2",

		// Remove once stable releases support CUDA 12.5
		if (this.active_release === "Stable" && cuda_version.includes("12.5")) isDisabled = true;

	if (this.active_additional_packages.includes("TensorFlow") && (!this.active_conda_cuda_ver.includes("12.0"))) this.active_conda_cuda_ver = "12.0 - 12.2";
	if (this.active_additional_packages.includes("TensorFlow") && (!this.active_conda_cuda_ver.includes("12.0"))) this.active_conda_cuda_ver = this.conda_cuda_ver_stable;

Use CUDA version ranges in release selector. #524

Use CUDA version ranges in release selector. #524

Conversation

bdice commented Jul 22, 2024

netlify bot commented Jul 22, 2024 • edited Loading

✅ Deploy Preview for docs-rapids-ai ready!

bdice commented Jul 22, 2024

jakirkham left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jarmak-nv commented Jul 23, 2024

jakirkham commented Jul 23, 2024

jakirkham commented Jul 23, 2024

bdice commented Jul 23, 2024

bdice commented Jul 23, 2024

jakirkham left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jarmak-nv commented Jul 23, 2024

bdice commented Jul 23, 2024

bdice commented Jul 23, 2024

jakirkham commented Jul 23, 2024

jakirkham commented Jul 23, 2024

jarmak-nv left a comment

Choose a reason for hiding this comment

jakirkham commented Jul 24, 2024

netlify bot commented Jul 22, 2024 •

edited

Loading