Skip to content

refactor(rapids-artifact-name): add explicit --arch flag to override $(arch)#255

Merged
rapids-bot[bot] merged 3 commits into
rapidsai:mainfrom
gforsyth:rapids-artifact-name-again
May 6, 2026
Merged

refactor(rapids-artifact-name): add explicit --arch flag to override $(arch)#255
rapids-bot[bot] merged 3 commits into
rapidsai:mainfrom
gforsyth:rapids-artifact-name-again

Conversation

@gforsyth
Copy link
Copy Markdown
Contributor

@gforsyth gforsyth commented Apr 29, 2026

Sorry for the churn on this.
This PR reverts the change in #254 for a few reasons:

  1. I was wrong about the categories of "purity" for artifacts (or not right enough) -- distributed_ucxx has a dependency on cuda version (or is built for both cuda 12 and cuda 13, at least) but is CPU arch independent.
  2. My solution for the categories of "purity" made the inclusion of the CPU architecture in the artifact name a side-effect, which is bad.

So, instead, after reverting that change, I'm adding an additional (and
optional) --arch <value> flag, that, if used, overrides the value of
$(arch) with the user-provided string.

This could be useful for downloading artifacts with a different architecture
than the system a developer is currently using, and can also be used to set an
arbitrary value for arch for the cases like distributed_ucxx where we build
on x86_64 but use the artifacts built there on both x64_64 and aarch64
systems. (This is true for anythign that doesn't have compiled code but does have CUDA-version-specific dependencies, like nx-cugraph and cuopt-server)

@gforsyth gforsyth added improvement Improves an existing functionality non-breaking Introduces a non-breaking change labels Apr 29, 2026
@gforsyth gforsyth force-pushed the rapids-artifact-name-again branch from f9497f8 to 207ee5c Compare April 29, 2026 19:54
@gforsyth gforsyth marked this pull request as ready for review April 29, 2026 20:06
@gforsyth gforsyth requested a review from a team as a code owner April 29, 2026 20:06
@gforsyth gforsyth requested review from msarahan and removed request for a team April 29, 2026 20:06
Copy link
Copy Markdown
Member

@jameslamb jameslamb left a comment

Choose a reason for hiding this comment

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

I trust your testing, if this fixes the issues let's merge this.


distributed_ucxx has a dependency on cuda version (or is built for both cuda 12 and cuda 13, at least) but is CPU arch independent

This would be true for anything that doesn't have compiled code but does have CUDA-version-specific dependencies. nx-cugraph-{cu12,cu13} and cuopt-server-{cu12,cu13} are other examples.

Everything you said is right, just noting that we're not making changes only for the sake of distributed-ucxx.

@jameslamb jameslamb removed the request for review from msarahan May 6, 2026 15:22
@gforsyth
Copy link
Copy Markdown
Contributor Author

gforsyth commented May 6, 2026

Everything you said is right, just noting that we're not making changes only for the sake of distributed-ucxx.

Thanks for documenting that, @jameslamb ! Yeah, distributed-ucxx was where I ran into the issues, but it's not a one-off.

As an early open-source collaborator of mine once said, "your special case isn't special enough to allow"

@jameslamb
Copy link
Copy Markdown
Member

Love that haha

@gforsyth
Copy link
Copy Markdown
Contributor Author

gforsyth commented May 6, 2026

/merge

@rapids-bot rapids-bot Bot merged commit ac81d24 into rapidsai:main May 6, 2026
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

improvement Improves an existing functionality non-breaking Introduces a non-breaking change

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants