chore(ci): compile the OpenXLA feature combinations - #1282
Merged
Conversation
Nothing in CI compiled any XLA feature. `ci.yml` never builds the crate, `pipeline-parallel-ci.yml` runs clippy on default features, and `nightly-verify.yml` uses `metal,accelerate`. Two defects reached `main` through that gap: a non-exhaustive `match` in the OpenXLA serve worker after `ModelRequest` gained a variant, and a dead re-export that `-D warnings` would have rejected. Both were found by hand while rebasing an unrelated branch. The job compiles `cuda,xla-iree` and `xla-diagnostics` with `--all-targets` on the GB10 runner. That runner is chosen because `xla-iree` links against a prebuilt IREE runtime that `scripts/iree/setup-cuda.sh` builds from source against a pinned revision, which is far too slow per PR from scratch; the runner already holds it under `~/.cache/mlxcel/iree-cuda-<version>` and the script is idempotent, so a warm runner reuses it. `xla-diagnostics` also implies `cuda`, which points at the same host. It denies `unused_imports` rather than every warning. The XLA combination carries a pre-existing dead-code backlog and clippy fails on it today, so `-D warnings` would make this job red on arrival and therefore ignored. `unused_imports` is the exact lint class of the dead re-export, and compile errors cover the other defect, so both historical breaks are caught without requiring that backlog to be cleared first. Broadening the policy needs that cleanup and is left as separate work. What the job deliberately does not cover is recorded in the workflow file rather than left to be discovered: no test execution, no full warning policy, and no macOS or `IREE_DIST` build, none of which have a runner that can do it. Also silences two dead-code warnings this repository's own parity test introduced under the non-diagnostics feature set, so the new gate starts from a clean tree.
TEMPORARY. Removes the ModelRequest::PromptCacheWarmup arm, reintroducing the exact E0004 that reached main unseen, so the OpenXLA feature compile job can be observed failing. Reverted in the next commit.
… catch" This reverts commit 6f19333. The demonstration is complete. With the arm removed, the OpenXLA feature compile job failed on exactly the error that reached main unseen: error[E0004]: non-exhaustive patterns: `model_provider::ModelRequest::PromptCacheWarmup { .. }` not covered so the check-run history on this PR now shows the gate passing, failing on the reintroduced defect, and passing again. A job that only ever passes is not evidence that it catches anything.
Recorded before the merge, per the TECHNICAL_REPORTS/.keep-reports contract.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Add a CI job that compiles the OpenXLA feature combinations. Nothing did, and two defects reached
mainthrough the gap.The gap
ci.ymlrunsdeny,fmt, crate-version, kernel-dtype-key, MLX-pin and cross-repo-ref jobs and never builds the crate.pipeline-parallel-ci.ymlruns clippy on default features.nightly-verify.ymlruns clippy withmetal,accelerate. The OpenXLA serve worker lives behind#[cfg(feature = "xla-iree")], so no check run has ever compiled it.What got through:
ModelRequest::PromptCacheWarmupnot handled in the OpenXLA worker (#1273)cargo checkload_weights_from_dir_with_filterre-exportunused_importscuda,xla-iree(#1275)Both of the first two were found by hand while rebasing an unrelated branch, not by any gate.
Why the GB10 runner
xla-ireelinks a C shim against a prebuilt IREE runtime.scripts/iree/setup-cuda.shprovisions it by building the runtime from source against a pinned revision, which is far too slow to do per PR from scratch. The GB10 runner already holds that build under~/.cache/mlxcel/iree-cuda-<version>and the script is idempotent, so a warm runner reuses it and only a fresh one pays the one-time cost.xla-diagnosticsadditionally impliescuda, which points at the same host. That removes the provisioning question #1270 raised: no actions cache keyed on the IREE revision and no container image is needed.Why it denies
unused_importsand not every warningMeasured on the current tree,
cargo clippy --features cuda,xla-iree --lib --tests -- -D warningsfails with four errors inmlxcel-xlaalone, before reachingmlxcel, andcargo check --all-targetsreports about ten dead-code warnings across both crates. A job that is red the day it lands gets ignored and stops being a gate at all.unused_importsis the exact lint class of the dead re-export, and compile errors cover the other defect, so both historical breaks are caught without first clearing that backlog. Broadening to-D warningsneeds the cleanup and is separate work.Verified locally with the job's own commands and
RUSTFLAGS, both green:What it deliberately does not cover
Recorded in the workflow file rather than left to be discovered later:
IREE_DISTbuild. Neither has a runner with the required distribution. This is why fix(build): link libc after the IREE archives so integration tests link #1275's link failure would still not be caught:cargo checknever links, so a compile gate cannot see it.Also here
Two dead-code warnings that this repository's own Molmo2 parity test introduced under the non-diagnostics feature set are silenced, so the new gate starts from a clean tree rather than needing an exception on arrival.
The gate was observed catching the defect, not just passing
A job that only ever passes is not evidence that it catches anything, so the check-run history on this PR shows it in all three states:
OpenXLA feature compile763bc8a06f193333ModelRequest::PromptCacheWarmuparm removed, reintroducing the exact defect that reachedmainf88709bcThe failing run reported the same error the gap let through:
The middle commit and its revert stay in the branch history on purpose: squashed into
mainthey cancel out, and until then they are the evidence.Closes #1270