feat: (jax) Add Build JAX wheels in Multi-Arch CI#6439
Conversation
✅ All Policy Checks Passed
📖 Need help? See the Policy FAQ for details on every check and how to fix failures. |
|
🎉 All checks passed! This PR is ready for review. |
…LOWS_CI_FILENAMES
marbre
left a comment
There was a problem hiding this comment.
Please see compare with PyTorch where we limit to only build for Python 3.12. We shouldn't run the full Python matrix on every CI build.
Thanks - that makes sense - just updated the code |
marbre
left a comment
There was a problem hiding this comment.
Is it worth to build rocm-jaxlib-v0.10.0 and rocm-jaxlib-v0.10.2?
Furthermore, the CI description needs to be updated with regards to the Python versions:
Default CI currently generates:
* rocm-jaxlib-v0.10.0
* rocm-jaxlib-v0.10.2
* Python 3.11, 3.12, 3.13, and 3.14
| docker build \ | ||
| -t "${MANYLINUX_IMAGE_TAG}" \ | ||
| --file=Dockerfile.jax-manylinux_2_28-therock \ | ||
| --build-arg=THEROCK_INDEX_URL="${ROCM_PACKAGE_FIND_LINKS_URL}" \ | ||
| --build-arg=THEROCK_VERSION="${THEROCK_VERSION}" \ | ||
| --build-arg=GFX_ARCH="${GFX_ARCH}" \ | ||
| --progress=plain \ | ||
| . |
There was a problem hiding this comment.
As this workflow is intended to run on every CI run we should avoid building a docker container every time. This overhead might be acceptable as an intermediate solution for release builds but I think it is not for CI builds.
There was a problem hiding this comment.
I see the point but I am not controlling that part - If you want I can keep only rocm-jaxlib-v0.10.0 until other solution comes from JAX Team or should we wait?
There was a problem hiding this comment.
I am aware you're not in control of it but I would rather delay this PR until it was fixed upstream. Would switching to 0.10.0 let us avoid re-building the Docker container every time? Normally I would prefer to stick with the latest patch version which in our case seems to be 0.10.2.
Anyway, let's wait for @ScottTodd to weight in. I would rather push back to merge this as is.
There was a problem hiding this comment.
0.10.0 is the most stable one - 0.10.2 has some issues from the upstream. If we chose one, I think 0.10.0 is the best candidate.
Would switching to 0.10.0 let us avoid re-building the Docker container every time? Just an idea, It will reduce the pressure on the runners
Sounds good to me to wait for Scott's input.
There was a problem hiding this comment.
Well if it still requires Docker my push back remains. As 0.10.2 is a patch release I would also rather go with that version instead of 0.10.0. Let's wait for Scott to weight in.
There was a problem hiding this comment.
@ScottTodd , If I gate this for the rocm-systems and rocm-libraries bumps only, would it be ok? Since my main purpose is to detect how JAX is affected by those bump PRs.
There was a problem hiding this comment.
I'll take a more detailed look tomorrow (only got through ~50% of my PR review queue today). Some quick thoughts:
- At least this depends on
build_python_packagesand not tarball builds. If it depended on tarball builds that would bloat the storage size needed for each CI run. - We should compare the job time between JAX and PyTorch and look for opportunities to optimize both. I see ~1h of build time for both with opportunities (ccache/sccache and other tricks) to get that down to ~20 minutes.
- As for what actually happens during the build, I see that JAX is still building LLVM from source (we already build a copy in ROCm, ideally we wouldn't need another copy) and I see that we're still building a dockerfile every run (which pulls in uncontrolled dependencies).
- If we build for releases already, we should at least have the ability to build on CI, even if only opt-in via a label or workflow_dispatch argument. I do like the idea of running these new jobs on at least bump PRs in TheRock, so we can catch if/when changes to rocm-systems and rocm-libraries break the JAX build (we've seen changes to RCCL break the PyTorch build).
So I think we can find a reasonable middle-ground for the CI system that gets us this much desired coverage, but there are still a few things to continue development on for the final shape that we'll all be happy with. Let's make sure that enough of that is tracked (and prioritized/assigned) so we don't let it slip for too long.
(will continue reviewing tomorrow)
There was a problem hiding this comment.
Thanks, I opened an issue to track follow ups #6388, I will modify and fill out as they are determined.
There was a problem hiding this comment.
Please add a TODO here with a link to an issue covering removing this dockerfile build. Our build containers should be frozen and updated infrequently, not build as part of every workflow run. This container in particular (https://github.com/ROCm/rocm-jax/blob/rocm-jax-infra/docker/manylinux/Dockerfile.jax-manylinux_2_28-therock) has multiple issues:
- It builds LLVM from source (when we already build and distribute LLVM as part of ROCm)
- It installs the AWS CLI from a domain we do not control
- It likely has poor interactions with caches (docker layer caches are a poor substitute for ccache)
- Dockerfiles setups like this are awkward for local development (including debugging, development, and reproducing what CI does)
We shouldn't need this at all, all we should need is to run in our existing https://github.com/ROCm/TheRock/blob/main/dockerfiles/build_manylinux_x86_64.Dockerfile image or a slightly modified image that also includes Bazel and any other build dependencies (for the entire job), install rocm into a venv, and then run the python build/build.py script.
ScottTodd
left a comment
There was a problem hiding this comment.
I'm okay with the extra time (~1h6m) that this will add to CI runs here in TheRock, but yes let's follow up later on some of the specifics to optimize both how long that takes and when it triggers (e.g. exclude jax builds for PRs that only modify pytorch code)
| docker build \ | ||
| -t "${MANYLINUX_IMAGE_TAG}" \ | ||
| --file=Dockerfile.jax-manylinux_2_28-therock \ | ||
| --build-arg=THEROCK_INDEX_URL="${ROCM_PACKAGE_FIND_LINKS_URL}" \ | ||
| --build-arg=THEROCK_VERSION="${THEROCK_VERSION}" \ | ||
| --build-arg=GFX_ARCH="${GFX_ARCH}" \ | ||
| --progress=plain \ | ||
| . |
There was a problem hiding this comment.
Please add a TODO here with a link to an issue covering removing this dockerfile build. Our build containers should be frozen and updated infrequently, not build as part of every workflow run. This container in particular (https://github.com/ROCm/rocm-jax/blob/rocm-jax-infra/docker/manylinux/Dockerfile.jax-manylinux_2_28-therock) has multiple issues:
- It builds LLVM from source (when we already build and distribute LLVM as part of ROCm)
- It installs the AWS CLI from a domain we do not control
- It likely has poor interactions with caches (docker layer caches are a poor substitute for ccache)
- Dockerfiles setups like this are awkward for local development (including debugging, development, and reproducing what CI does)
We shouldn't need this at all, all we should need is to run in our existing https://github.com/ROCm/TheRock/blob/main/dockerfiles/build_manylinux_x86_64.Dockerfile image or a slightly modified image that also includes Bazel and any other build dependencies (for the entire job), install rocm into a venv, and then run the python build/build.py script.
Progress #3878.
Summary
Adds JAX wheel build coverage to Multi-Arch CI.
This replaces the placeholder JAX CI workflow with a real manylinux JAX
wheel build flow, wires it into Linux Multi-Arch CI, and enables JAX
wheel builds by default alongside PyTorch.
Changes
Replaces the placeholder JAX CI reusable workflow with a real build flow:
jax-rocm-pluginandjax-rocm-pjrtwheelsAdds
build_jaxtomulti_arch_ci.yml:trueWires
build_jax_wheelsintomulti_arch_ci_linux.yml:build_python_packagesbuild_python_packages.outputs.package_find_links_urljax_build_matrixfrom the generated Linux build configAdds a CI-specific JAX matrix helper:
CI vs release matrix behavior
The release JAX matrix still includes:
rocm-jaxlib-v0.9.1rocm-jaxlib-v0.10.0rocm-jaxlib-v0.10.2The CI JAX matrix filters out the legacy/native
rocm-jaxlib-v0.9.1entry because the new CI workflow is manylinux-only.
Default CI currently generates:
rocm-jaxlib-v0.10.0Python 3.12This produces 1 JAX CI build cell.
Testing
Ran local Python checks:
CI Test: https://github.com/ROCm/TheRock/actions/runs/29396032944/job/87311830950?pr=6439
Follow up
TODO
#6388