Skip to content

Verify rebuild-skip against real wheels: fatbin arch coverage + build-manifest asset - #10

Merged
ETOgaosion merged 2 commits into
verl-project:mainfrom
ETOgaosion:skip-matching-build-config
Sep 4, 2026
Merged

Verify rebuild-skip against real wheels: fatbin arch coverage + build-manifest asset#10
ETOgaosion merged 2 commits into
verl-project:mainfrom
ETOgaosion:skip-matching-build-config

Conversation

@ETOgaosion

@ETOgaosion ETOgaosion commented Sep 4, 2026

Copy link
Copy Markdown
Contributor

Why

Skip detection today trusts (a) the release title string and (b) a config blob embedded in the release notes. That has three holes:

  1. Declared != built. Nothing verifies the published wheel actually fat-binned every arch in torch_cuda_arch_list. deep-ep's sm_80 cubin only exists because ci/patches/enable_deep_ep_sm80.py rewrites the checkout at build time - if that patch ever stops applying, a wheel that crashes on A100 ships and skip detection happily trusts it.
  2. Build-input drift is invisible. Editing a builder script, common.sh, _build.yml, or a patch without bumping versions.yaml leaves the manifest unchanged -> wrong skip.
  3. Arch strings are compared opaquely. "8.0;9.0;10.0" vs "80;90;100" vs "8.0 9.0 10.0" mean the same thing but compare unequal.

What

  • ci/cuda_archs.py (new) - normalizes every TORCH_CUDA_ARCH_LIST spelling (dotted/undotted, ;/space/, separators, 9.0a/12.0f suffixes, +PTX) into a canonical arch-token set, and reads the arches a wheel actually fat-binned: cuobjdump when the toolkit is present, otherwise a stdlib-only ELF .nv_fatbin section scan (host ELFs are never whole-blob scanned, so diagnostic strings can't fake coverage). Works as a CLI gate (--require exits 1 on missing arch).
  • Build-time gate - _build.yml fails the job before upload if dist/*.whl doesn't cover every declared arch.
  • wheelhouse-build-manifest.json release asset - every build uploads a machine-readable snapshot of everything that affects the build: dependency versions, arch list (raw + normalized), extra_env, the builder command, max_jobs, runs_on, and build_inputs - sha256 fingerprints of _build.yml, the builder script, common.sh, and declared patches. The release-notes HTML comment stays as a fallback for older releases.
  • Skip detection downloads the manifest asset and diffs it; for CUDA components it also downloads the release wheels and re-checks fatbin coverage. Anything unverifiable - no repo, download failure, missing manifest, old schema - fails closed and rebuilds. Bumping the manifest schema to 2 naturally rebuilds all existing releases once.
  • versions.yaml - deep-ep declares patches: [ci/patches/enable_deep_ep_sm80.py] (so patch edits change the fingerprint and force a rebuild); flashinfer, which rehosts prebuilt wheels (incl. a ~1.2 GB data wheel), opts out with verify_wheel_archs: false.
  • Caller workflows pass the new verify-wheel-archs matrix flag through.

CI: dry-run build plan on pull requests

A new Build plan dry run check (.github/workflows/pr-build-dry-run.yml) runs on every PR that touches versions.yaml, ci/, or the workflows. It compares the PR's matrix against the live releases and writes a markdown table - per component x arch row, build vs skip with the reason - to the job summary, and upserts it as a PR comment (same-repo PRs; fork PRs get the summary only since their token is read-only). Nothing is built or published: the plan is shown for a human to judge (e.g. a ref bump should build only that component; a docs-only change should skip everything). The dry run uses --no-verify-wheels to stay cheap (no multi-GB downloads); fatbin coverage is still gated pre-upload and re-verified at push time. Reproduce locally:

python3 ci/generate_matrix.py --skip-existing-releases \
  --repo verl-project/verl-wheelhouse --no-verify-wheels --report build-plan.md

The check already ran on this PR and passed (15/15 rows report build, expected: existing releases predate manifest schema 2).

Tests

  • 43 unit tests pass: 24 for cuda_archs (synthetic ELF32/64 + wheels, all arch-list spellings, fatbin/PTX extraction, EM_CUDA gating, CLI exit codes) and 19 for generate_matrix (manifest asset preferred over notes, old-schema fallback, fatbin-covered skip, missing-sm_80 rebuild, build-input hash change rebuild, opt-out, download-failure fail-closed, dry-run report decisions).
  • Verified against the real versions.yaml and the real upstream releases: full matrix, all component manifests, and the dry-run report all generate; all 14 workflow files parse as YAML.
  • Docs updated in docs/maintaining-components.md.

Supersedes #9 (closed): instead of encoding arches in the title, the manifest asset + wheel inspection cover that and more.

Skip detection previously trusted an opaque title string and a
config blob embedded in the release notes, so a wheel whose fatbin
silently dropped a promised SM arch (e.g. deep-ep sm_80 if the patch
fails to apply) or a changed build script with no versions.yaml bump
would be wrongly skipped.

- new ci/cuda_archs.py: normalize TORCH_CUDA_ARCH_LIST spellings into
  a canonical arch-token set and read the CUDA arches a wheel actually
  fat-binned (cuobjdump when present, stdlib-only ELF .nv_fatbin
  section scan otherwise); usable as a CLI gate
- _build.yml: gate the upload on the built wheels covering every
  declared arch, and upload wheelhouse-build-manifest.json as a
  release asset (release-notes HTML comment kept as fallback)
- build manifest schema 2 now records every build-affecting input:
  versions, arch list, extra env, builder command, max_jobs, runs_on,
  and sha256 fingerprints of _build.yml, the builder script,
  common.sh and declared patches
- skip detection downloads the manifest asset and, for components
  that compile CUDA, downloads the release wheels and re-checks their
  fatbin coverage; unverifiable cases (no repo, download failure,
  missing/old manifest) fail closed and rebuild
- versions.yaml: declare deep-ep's sm80 patch under patches:, opt
  flashinfer (rehosted prebuilt wheels) out of arch verification via
  verify_wheel_archs: false
Add a Build plan dry run check that runs on PRs touching versions.yaml,
ci/ or the workflows: generate_matrix.py gains --report (markdown
build/skip table with per-row reasons) and --no-verify-wheels (skip the
multi-GB wheel download; fatbin coverage stays gated at build time and
re-verified at push time). The workflow appends the report to the job
summary and upserts it as a PR comment (same-repo PRs only; fork PRs
get the summary, as their token is read-only). No builds run - the
plan is shown for a human to judge against the change's intent.

- evaluate_matrix_rows() centralizes the build/skip decision (reason
  attached to every matrix row); rows_needing_build() is a thin filter
  over it
- 3 new tests: dry-run covers without downloading wheels, report rows
  carry build/skip decisions and reasons, missing release => build
@ETOgaosion
ETOgaosion merged commit db6bfcf into verl-project:main Sep 4, 2026
1 check passed
ETOgaosion added a commit that referenced this pull request Sep 7, 2026
…size in patch (#11)

* deep-ep sm80 patch: use real cuda_fp8.h instead of fake FP8 typedefs

The Ampere fallback in configs.cuh re-declares __nv_fp8_interpretation_t /
__nv_fp8x4_e4m3 / __nv_fp8_storage_t as int/uint8_t. During the sm_80
device pass, torch/nvshmem headers pull the toolkit <cuda_fp8.h> in
transitively, causing "invalid redeclaration" errors (71 errors in
intranode.cu, CUDA 13.0). This path was never compiled before: pre-#10
skip logic skipped the x86_64 deep-ep build, so the sm_80 cubin was
never exercised in CI.

No sm_80 compilation unit references FP8 symbols (the only FP8 call
sites are in internode_ll.cu, whose sm_80 cubin is compiled out by the
same patch), so configs.cuh now unconditionally includes <cuda_fp8.h>
like the host / sm_90 / sm_100 passes already did.

* deep-ep sm80 patch: hoist smem_size out of SM90 guard in intranode.cu

The runtime-dispatched SET_SHARED_MEMORY_FOR_TMA macro (unlike upstream's
Ampere build, where it #defines to void()) always expands to code that
references the caller's smem_size, but intranode.cu declares that
constant inside #ifndef DISABLE_SM90_FEATURES. In the sm_80 device pass
the snippet defines DISABLE_SM90_FEATURES, so the declaration vanishes
and the launch macro expansion fails with "identifier smem_size is
undefined" at intranode.cu:521 and :941.

Move both smem_size declarations out of the guard (dispatch and combine
launch wrappers); the cudaFuncSetAttribute call stays behind the runtime
__use_sm90_launch check. Patch fails loudly unless exactly the two
expected guarded declarations are present.
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.

1 participant