Skip to content

Conversation

@subodh-dubey-amd
Copy link

@subodh-dubey-amd subodh-dubey-amd commented Nov 13, 2025

Motivation - ⁠SWDEV-563344

SWDEV-563344
[ROCm QA][TheRock] Build specific JAX whl are not available in artifactory

This PR adds an wheel tagging feature enable rocm version management of JAX ROCm jaxlib wheels in automated build pipelines, which is essential for maintaining consistent versioning across different ROCm builds and integrating with existing S3 upload workflows.

Technical Details

Pattern Transformation:

jaxlib-0.7.1-cp312-cp312-manylinux_2_28_x86_64.whl
→ jaxlib-0.7.1+rocm7.1.0a20251107-cp312-cp312-manylinux_2_28_x86_64.whl

Test Result

Submission Checklist

[*]Look over the contributing guidelines at https://github.com/ROCm/ROCm/blob/develop/CONTRIBUTING.md#pull-requests.

@subodh-dubey-amd subodh-dubey-amd force-pushed the subodube/jaxlib-rocm-version-extra branch from 08c44fc to ffe67f5 Compare November 19, 2025 10:55
@subodh-dubey-amd subodh-dubey-amd marked this pull request as ready for review November 19, 2025 10:56
@subodh-dubey-amd subodh-dubey-amd changed the title Subodube/jaxlib rocm version extra jaxlib with rocm version extra tag Nov 19, 2025
@subodh-dubey-amd
Copy link
Author

python_tag = "cp"

if args.rocm_path:
rocm_tag = args.rocm_path.split("-")[-1]
Copy link
Collaborator

Choose a reason for hiding this comment

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

Pretty sure this isn't going to capture the full ROCm version. I've never seen a ROCm install called anything like /opt/rocm-7.1.0devabc123. Does ROCm store the full version string somewhere, like maybe in .info/version?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Or you could repurpose the --rocm_version argument for this use-case. Have it accept the full ROCm version and use that as a tag to jaxlib

Choose a reason for hiding this comment

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

bazel_dir,
output_path,
f"{wheel_dir}*{wheel_version_suffix}.tar.gz",
rocm_tag=rocm_tag,
Copy link
Collaborator

Choose a reason for hiding this comment

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

This is not the proper place for this. We don't ship the jax wheel, only jaxlib and the jax-rocm7... wheels. Whatever version changes we make to the jaxlib wheel have to be compatible with upstream jax.

Copy link
Collaborator

@charleshofer charleshofer left a comment

Choose a reason for hiding this comment

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

I did some local testing, and, as is, this change has some problems.

If I pass in --rocm_path=/opt/rocm-7.2.0 I get a wheel called jaxlib-0.8.0.dev0+selfbuilt+rocm7.2.0-cp312-cp312-manylinux_2_27_x86_64.whl, which is not a valid wheel name.

Paths to ROCm that don't contain the version number are also valid, so a path like /opt/rocm/ will break this.

I'd suggest two things:

  1. Use --rocm_version instead of --rocm_path. --rocm_path will not get you full version names like 7.1.0dev12345.
  2. Don't add the other version suffix if --rocm_version is set.



def copy_individual_files(src: str, dst: str, glob_pattern: str):
def copy_individual_files(src: str, dst: str, glob_pattern: str, rocm_tag: str):
Copy link
Collaborator

Choose a reason for hiding this comment

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

The typing on this is wrong. It should be Optional[str] and it should take None as a default.

python_tag = "cp"

if args.rocm_path:
rocm_tag = args.rocm_path.split("-")[-1]
Copy link
Collaborator

Choose a reason for hiding this comment

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

Or you could repurpose the --rocm_version argument for this use-case. Have it accept the full ROCm version and use that as a tag to jaxlib

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants