diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 3ea40e7ce..0cf13ee2f 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -68,6 +68,8 @@ jobs: - '**/Cargo.toml' - 'Cargo.lock' - 'deny.toml' + - 'build.rs' + - 'scripts/iree/**' - '.github/workflows/ci.yml' mlx_pin: - 'src/lib/mlx-cpp/CMakeLists.txt' @@ -224,3 +226,81 @@ jobs: BASE_REF: origin/${{ github.base_ref }} run: python3 scripts/ci/check_cross_repo_refs.py + # ============================================================================ + # OpenXLA feature compile (self-hosted GB10) + # ============================================================================ + # Nothing else in CI compiles any XLA feature. `deny`, `fmt` and the other + # jobs here never build 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 a new `ModelRequest` variant landed, and a dead + # re-export that `-D warnings` would have rejected. Both were found by hand + # while rebasing an unrelated branch. + # + # Why this runs on the self-hosted GB10 runner rather than a hosted one: + # `xla-iree` links a C shim against a prebuilt IREE runtime, which + # `scripts/iree/setup-cuda.sh` provisions by building the runtime from source + # against a pinned revision. That is far too slow to do per PR from scratch, + # but the GB10 runner already holds the build under + # `~/.cache/mlxcel/iree-cuda-` and the script is idempotent, so a + # warm runner reuses it and only a fresh one pays the one-time cost. + # `xla-diagnostics` additionally implies `cuda`, which points at the same + # runner. + # + # Deliberately NOT covered, so the gap is recorded rather than assumed shut: + # - `cargo test`. This is a compile gate; running the XLA suites needs a + # GPU that is not contended with development work on the same host. + # - Full `-D warnings`. The XLA feature combination carries a pre-existing + # dead-code backlog in `mlxcel-xla` and `mlxcel`, and clippy fails on it + # today, so denying every lint would make this job red on arrival and + # therefore ignored. It denies `unused_imports` instead, which is the + # exact lint class of the dead re-export above, and compile errors cover + # the other. Broadening to `-D warnings` needs that backlog cleared + # first. + # - macOS and `IREE_DIST` builds of the same features. Neither has a runner + # with the required distribution. + xla-compile: + name: OpenXLA feature compile + needs: changes + if: github.repository == 'lablup/mlxcel' && needs.changes.outputs.rust == 'true' + runs-on: GB10 + permissions: + contents: read + timeout-minutes: 120 + env: + # The value shipped for this runner's architecture. Auto-detection yields + # `121a`, which is numerically identical here but is not what releases + # build, so the gate compiles what ships. + MLX_CUDA_ARCHITECTURES: "121" + # See the note above: this denies the lint that let a defect through, not + # every lint. + RUSTFLAGS: "-D unused_imports" + steps: + - uses: actions/checkout@v7 + with: + persist-credentials: false + + - name: Use a persistent target directory + run: | + # Separate from the release job's directory so a CI check cannot + # invalidate a release build's cache, or be slowed by it. + CARGO_TARGET="$HOME/.cargo-target/mlxcel-xla-ci" + mkdir -p "$CARGO_TARGET" + echo "CARGO_TARGET_DIR=$CARGO_TARGET" >> $GITHUB_ENV + + - name: Provision the IREE runtime + run: | + # Idempotent: reuses `~/.cache/mlxcel/iree-cuda-` when the + # runtime is already built, and builds it once when it is not. + bash scripts/iree/setup-cuda.sh + # The script emits shell `export VAR=value` lines; GITHUB_ENV wants + # bare `VAR=value`, and without stripping the prefix the variables + # would be named "export IREE_CUDA_HOME" and build.rs would abort + # claiming no IREE distribution is configured. + bash scripts/iree/setup-cuda.sh --env | sed 's/^export //' >> "$GITHUB_ENV" + + - name: Compile the production XLA feature set + run: cargo check --features cuda,xla-iree --all-targets + + - name: Compile the diagnostics XLA feature set + run: cargo check --no-default-features --features xla-diagnostics --all-targets diff --git a/TECHNICAL_REPORTS/1282-xla-compile-ci-gate-20260822.en.md b/TECHNICAL_REPORTS/1282-xla-compile-ci-gate-20260822.en.md new file mode 100644 index 000000000..9fc8f0487 --- /dev/null +++ b/TECHNICAL_REPORTS/1282-xla-compile-ci-gate-20260822.en.md @@ -0,0 +1,84 @@ +# Technical Report: PR #1282 - Compile the OpenXLA feature combinations in CI + +## Executive Summary + +No CI job compiled any XLA feature, and two defects reached `main` through that gap. This adds a compile gate on the self-hosted GB10 runner covering `cuda,xla-iree` and `xla-diagnostics`. + +Two decisions carry the report. The gate denies `unused_imports` rather than every warning, because measured on the current tree a full `-D warnings` policy fails immediately and a job that is red on arrival stops being a gate. And the gate was observed failing on a deliberately reintroduced defect before being merged, because a job that only ever passes is not evidence that it catches anything. + +## 1. Problem Statement + +`ci.yml` runs `deny`, `fmt`, crate-version, kernel-dtype-key, MLX-pin and cross-repo-ref jobs and never builds the crate. `pipeline-parallel-ci.yml` runs clippy on default features. `nightly-verify.yml` runs clippy with `metal,accelerate`. The OpenXLA serve worker sits behind `#[cfg(feature = "xla-iree")]`, so no check run had ever compiled it. + +| defect | class | found by | +| --- | --- | --- | +| `ModelRequest::PromptCacheWarmup` unhandled in the OpenXLA worker | E0004 | hand, during an unrelated rebase | +| dead `load_weights_from_dir_with_filter` re-export | `unused_imports` | hand, during the same rebase | +| integration tests could not link under `cuda,xla-iree` | link error | hand, while validating another PR | + +## 2. Technical Decisions + +### 2.1 The provisioning question the issue raised does not exist here + +The issue treated IREE provisioning as the real decision: an actions cache keyed on the pinned revision, a container image, or a self-hosted runner that already has one. Inspection settled it. The GB10 runner is the same host that carries `~/.cache/mlxcel/iree-cuda-`, and `scripts/iree/setup-cuda.sh` is idempotent, logging "reusing runtime build" when the tree is present. So the job provisions by calling the script: warm runners pay nothing and a fresh one pays the one-time build. `xla-diagnostics` implies `cuda`, which points at the same runner anyway. + +### 2.2 Deny the lint that let a defect through, not every lint + +Measured before choosing: + +```text +cargo clippy --features cuda,xla-iree --lib --tests -- -D warnings + -> 4 errors in mlxcel-xla alone, before reaching mlxcel +cargo check --features cuda,xla-iree --all-targets + -> ~10 dead-code warnings across both crates +``` + +`-D warnings` would therefore have landed red. A gate that is red on arrival is routed around and then ignored, which is worse than no gate because it also produces false confidence in a green summary elsewhere. + +`unused_imports` is the exact class of the dead re-export, and compile errors cover the other defect, so both historical breaks are caught with a policy that is green today. Broadening needs the dead-code backlog cleared and is left as separate work rather than smuggled in here. + +### 2.3 A `$GITHUB_ENV` format bug worth recording + +`setup-cuda.sh --env` emits shell `export VAR=value` lines. `$GITHUB_ENV` expects bare `VAR=value`, so appending the output directly would have defined variables literally named `export IREE_CUDA_HOME`. The failure mode is misleading rather than obvious: `build.rs` would abort claiming no IREE distribution is configured, on a runner that has one. The prefix is stripped with `sed` and the reason is recorded at the call site. + +### 2.4 State what the gate does not cover, in the workflow file + +A gate that closes part of a gap invites the assumption that the gap is shut. Three exclusions are written where someone editing the job will see them: no test execution, since running the XLA suites needs a GPU not contended with development work on the same host; no full warning policy, for the reason above; and no macOS or `IREE_DIST` build, since neither has a runner. The third is why the link failure above would still not be caught: `cargo check` never links. + +## 3. Change Summary + +| File | Change | +| --- | --- | +| `.github/workflows/ci.yml` | `xla-compile` job on GB10, `RUSTFLAGS: -D unused_imports`, separate persistent `CARGO_TARGET_DIR`, `MLX_CUDA_ARCHITECTURES: 121`, path filter extended to `build.rs` and `scripts/iree/**` | +| `tests/molmo2_xla_vision_parity.rs` | Two dead-code warnings this repository's own parity test introduced under the non-diagnostics feature set, silenced so the gate starts from a clean tree | + +## 4. Review Findings + +The `$GITHUB_ENV` bug in 2.3 was caught by reading the emitted format rather than by a run, which matters because the job would still have failed, just for a reason pointing at the wrong subsystem. + +`MLX_CUDA_ARCHITECTURES` is pinned to `121` rather than left to auto-detection, which yields `121a` on this host. The two are numerically identical here, but releases build `121`, and a gate should compile what ships. + +## 5. Validation + +Locally, with the job's own commands and `RUSTFLAGS`: + +```text +RUSTFLAGS="-D unused_imports" cargo check --features cuda,xla-iree --all-targets -> 0 +RUSTFLAGS="-D unused_imports" cargo check --no-default-features --features xla-diagnostics --all-targets -> 0 +``` + +On the PR, the job was driven through all three states rather than only observed passing: + +| commit | change | verdict | +| --- | --- | --- | +| `763bc8a0` | the job as proposed | SUCCESS | +| `6f193333` | `PromptCacheWarmup` arm removed | FAILURE | +| `f88709bc` | revert | SUCCESS | + +The failing run reported `error[E0004]: non-exhaustive patterns: ModelRequest::PromptCacheWarmup { .. } not covered`, the same error the gap let through. The middle commit and its revert stay in the branch history deliberately: they cancel out in the squash merge, and until then they are the evidence. + +## 6. Related Work + +Issue #1270 is closed by this PR. It was originally filed as a bug titled after the compile break, closed by the fix for that break, then reopened and retitled once it was clear only one of its four acceptance criteria had been met. The remaining three were this job, its documented feature matrix and provisioning strategy, and the demonstration in section 5. + +The uncovered classes named in 2.4 are the honest remainder. The link failure in particular needs a job that actually links a binary, which is a different cost profile from a compile check and deserves its own decision rather than being folded in here. diff --git a/TECHNICAL_REPORTS/1282-xla-compile-ci-gate-20260822.ko.md b/TECHNICAL_REPORTS/1282-xla-compile-ci-gate-20260822.ko.md new file mode 100644 index 000000000..355dabdac --- /dev/null +++ b/TECHNICAL_REPORTS/1282-xla-compile-ci-gate-20260822.ko.md @@ -0,0 +1,84 @@ +# 기술 보고서: PR #1282 - CI에서 OpenXLA feature 조합 컴파일 + +## 요약 + +어떤 CI 잡도 XLA feature를 컴파일하지 않았고, 그 공백으로 결함 두 개가 `main`에 도달했다. self-hosted GB10 러너에서 `cuda,xla-iree`와 `xla-diagnostics`를 컴파일하는 게이트를 추가한다. + +보고서를 지탱하는 판단은 둘이다. 게이트는 모든 경고가 아니라 `unused_imports`만 막는다. 현재 트리에서 측정한 결과 전면 `-D warnings`는 즉시 실패하고, **도착하자마자 빨간불인 잡은 게이트이기를 멈추기** 때문이다. 그리고 머지 전에 의도적으로 재현한 결함에서 게이트가 실패하는 것을 관측했다. 통과만 하는 잡은 무언가를 잡아낸다는 증거가 되지 않기 때문이다. + +## 1. 문제 + +`ci.yml`은 `deny`, `fmt`, crate-version, kernel-dtype-key, MLX-pin, cross-repo-ref 잡을 돌리며 크레이트를 빌드하지 않는다. `pipeline-parallel-ci.yml`은 기본 feature로 clippy를 돌린다. `nightly-verify.yml`은 `metal,accelerate`로 clippy를 돌린다. OpenXLA serve 워커는 `#[cfg(feature = "xla-iree")]` 뒤에 있으므로 어떤 체크런도 이를 컴파일한 적이 없다. + +| 결함 | 부류 | 발견 경로 | +| --- | --- | --- | +| OpenXLA 워커의 `ModelRequest::PromptCacheWarmup` 미처리 | E0004 | 무관한 리베이스 중 수작업 | +| 죽은 `load_weights_from_dir_with_filter` re-export | `unused_imports` | 같은 리베이스 중 수작업 | +| `cuda,xla-iree`에서 통합 테스트 링크 불가 | 링크 에러 | 다른 PR 검증 중 수작업 | + +## 2. 기술적 판단 + +### 2.1 이슈가 제기한 프로비저닝 문제는 여기 존재하지 않는다 + +이슈는 IREE 프로비저닝을 진짜 결정 사안으로 봤다. 고정 리비전을 키로 하는 액션 캐시, 컨테이너 이미지, 또는 이미 갖춘 self-hosted 러너 중 선택이다. 조사로 정리됐다. GB10 러너가 `~/.cache/mlxcel/iree-cuda-<버전>`을 가진 바로 그 호스트이고, `scripts/iree/setup-cuda.sh`는 멱등이라 트리가 있으면 "reusing runtime build"를 로그한다. 그래서 잡은 스크립트를 호출해 프로비저닝한다. 따뜻한 러너는 비용이 없고 새 러너만 일회성 빌드를 치른다. `xla-diagnostics`는 `cuda`를 함의하므로 어차피 같은 러너를 가리킨다. + +### 2.2 모든 lint가 아니라 결함을 통과시킨 lint를 막는다 + +선택 전에 측정했다. + +```text +cargo clippy --features cuda,xla-iree --lib --tests -- -D warnings + -> mlxcel에 닿기도 전에 mlxcel-xla에서만 에러 4건 +cargo check --features cuda,xla-iree --all-targets + -> 두 크레이트에 걸쳐 dead-code 경고 약 10건 +``` + +즉 `-D warnings`는 빨간불로 착륙했을 것이다. 도착하자마자 빨간불인 게이트는 우회되고 곧 무시되는데, 이는 게이트가 없는 것보다 나쁘다. 다른 곳의 초록 요약에 거짓 신뢰까지 얹기 때문이다. + +`unused_imports`는 죽은 re-export의 정확한 부류이고 컴파일 에러가 다른 결함을 덮으므로, 오늘 초록불인 정책으로 역사적 파손 두 건을 모두 잡는다. 정책 확대는 dead-code 백로그 정리가 필요하고, 여기에 끼워 넣지 않고 별도 작업으로 남긴다. + +### 2.3 기록해둘 `$GITHUB_ENV` 형식 버그 + +`setup-cuda.sh --env`는 셸 `export VAR=값` 줄을 내보낸다. `$GITHUB_ENV`는 벌거벗은 `VAR=값`을 기대하므로, 출력을 그대로 붙이면 문자 그대로 `export IREE_CUDA_HOME`이라는 이름의 변수가 정의된다. 실패 양상이 명백하지 않고 오히려 오도한다. IREE 배포판을 가진 러너에서 `build.rs`가 배포판이 설정되지 않았다며 중단한다. `sed`로 접두어를 벗기고 이유를 호출 지점에 기록했다. + +### 2.4 게이트가 덮지 않는 것을 워크플로 파일에 밝힌다 + +공백의 일부를 메우는 게이트는 공백이 닫혔다는 가정을 부른다. 이 잡을 편집할 사람이 보게 될 자리에 세 가지 제외를 적었다. 테스트 실행 없음. XLA 스위트 실행은 같은 호스트의 개발 작업과 경합하지 않는 GPU가 필요하다. 전면 경고 정책 없음. 위의 이유다. macOS와 `IREE_DIST` 빌드 없음. 어느 쪽도 러너가 없다. 세 번째가 위의 링크 실패가 여전히 안 잡히는 이유다. `cargo check`는 링크를 하지 않는다. + +## 3. 변경 요약 + +| 파일 | 변경 | +| --- | --- | +| `.github/workflows/ci.yml` | GB10의 `xla-compile` 잡, `RUSTFLAGS: -D unused_imports`, 분리된 영속 `CARGO_TARGET_DIR`, `MLX_CUDA_ARCHITECTURES: 121`, 경로 필터에 `build.rs`와 `scripts/iree/**` 추가 | +| `tests/molmo2_xla_vision_parity.rs` | 이 저장소의 파리티 테스트가 non-diagnostics feature에서 만든 dead-code 경고 2건 정리, 게이트가 깨끗한 트리에서 출발하도록 | + +## 4. 리뷰 지적사항 + +2.3의 `$GITHUB_ENV` 버그는 실행이 아니라 출력 형식을 읽어서 잡았다. 중요한 이유는, 그대로 뒀어도 잡은 실패했겠지만 **엉뚱한 서브시스템을 가리키는 이유로** 실패했을 것이기 때문이다. + +`MLX_CUDA_ARCHITECTURES`는 자동 감지에 맡기지 않고 `121`로 고정했다. 이 호스트에서 자동 감지는 `121a`를 낸다. 둘은 여기서 수치적으로 동일하지만 릴리스는 `121`을 빌드하고, 게이트는 실제 출하되는 것을 컴파일해야 한다. + +## 5. 검증 + +로컬에서 잡과 동일한 명령·동일한 `RUSTFLAGS`로: + +```text +RUSTFLAGS="-D unused_imports" cargo check --features cuda,xla-iree --all-targets -> 0 +RUSTFLAGS="-D unused_imports" cargo check --no-default-features --features xla-diagnostics --all-targets -> 0 +``` + +PR에서는 통과만 관측하지 않고 세 상태를 모두 거치게 했다. + +| 커밋 | 변경 | 판정 | +| --- | --- | --- | +| `763bc8a0` | 제안한 잡 | SUCCESS | +| `6f193333` | `PromptCacheWarmup` arm 제거 | FAILURE | +| `f88709bc` | 되돌림 | SUCCESS | + +실패한 실행은 `error[E0004]: non-exhaustive patterns: ModelRequest::PromptCacheWarmup { .. } not covered`를 보고했다. 공백이 통과시킨 바로 그 에러다. 중간 커밋과 되돌림은 브랜치 이력에 일부러 남긴다. squash 머지에서 상쇄되고, 그때까지는 증거다. + +## 6. 관련 작업 + +이슈 #1270이 이 PR로 닫힌다. 원래 컴파일 파손을 제목으로 한 버그로 제기됐고 그 파손의 수정으로 닫혔다가, 네 개 수용 기준 중 하나만 충족됐음이 분명해지자 다시 열리고 제목이 바뀌었다. 남은 셋이 이 잡, 문서화된 feature 매트릭스와 프로비저닝 전략, 그리고 5절의 실증이었다. + +2.4에 적은 미커버 부류가 정직한 나머지다. 특히 링크 실패는 실제로 바이너리를 링크하는 잡이 필요하고, 이는 컴파일 검사와 비용 구조가 달라서 여기 끼워 넣지 않고 별도 판단을 받아야 한다. diff --git a/tests/molmo2_xla_vision_parity.rs b/tests/molmo2_xla_vision_parity.rs index 1e53e66f9..5ab2041f5 100644 --- a/tests/molmo2_xla_vision_parity.rs +++ b/tests/molmo2_xla_vision_parity.rs @@ -118,8 +118,15 @@ struct Comparison { /// Worst `difference / (atol + rtol * |expected|)` seen. At most 1.0 the /// tensor is inside the contract; above 1.0 it is not. worst_ratio: f32, + // Read only by `detail`, which only the diagnostics-gated assertion path + // calls. A build without those features still constructs them, so they are + // allowed rather than cfg-gated: the values are cheap and keeping one + // definition avoids two shapes of the same struct. + #[allow(dead_code)] ratio_index: usize, + #[allow(dead_code)] ratio_actual: f32, + #[allow(dead_code)] ratio_expected: f32, } @@ -134,6 +141,8 @@ impl Comparison { } } + /// Only the diagnostics-gated `assert_within` reports this. + #[allow(dead_code)] fn detail(&self) -> String { format!( "max_abs={} at {} (actual={}, expected={}, relative={:.4}%), rms={}, \