Repository navigation
ci: harden the flash-attn cache and run unit-test on retargeted PRs - #8
Closed
curnane-lab wants to merge 3 commits into
Closed
curnane-lab wants to merge 3 commits into
curnane-lab wants to merge 3 commits into
Conversation
ensure_flash_attention.sh built the patched flash-attn wheel straight into the persistent per-build_key cache, before installing it or running the ABI check. A wheel that failed the check, or a truncated one left by a timed-out or cancelled bdist_wheel, stayed cached, and every later run skipped the build and reinstalled it. - Build into the temporary source directory and publish to the cache by an atomic rename only after the wheel installs and passes the ABI check. Drop a cached wheel that fails to install or fails the check, so the next run rebuilds it. - Verify the sdist against the sha256 in PyPI's JSON before extracting and patching it; uv used to do this when it fetched the sdist itself. Tests: the curl stub now serves a byte-stable tarball with its sha256, and its setup.py asserts FLASH_ATTENTION_FORCE_BUILD=TRUE, which no test covered after the build moved out of uv. New cases cover the failed-check, dropped-cache and checksum-mismatch paths. Co-authored-by: curnane-lab <curnane-lab@users.noreply.github.com>
HOME (/github/home) persists across jobs on the self-hosted runner, which is what keeps SPECFORGE_CI_CACHE alive, so `git config --global --add safe.directory` appended another identical entry on every unit-test run. Skip the add when the exact path is already trusted. Co-authored-by: curnane-lab <curnane-lab@users.noreply.github.com>
sgl-project#1065 and sgl-project#1066 were opened against their stacked parents and retargeted to main once the parent merged. Changing the base fires only an edited event, which the default pull_request types omit, so unit-test never ran on either before merge and the broken pinned-SGLang toolchain reached main. Add edited to the trigger types and run the GPU jobs for it only when the base changed. Title and body edits also fire edited; give those runs their own concurrency group so they do not cancel or replace a running or queued test, and skip their jobs. Co-authored-by: curnane-lab <curnane-lab@users.noreply.github.com>
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.
Motivation
Follow-up to sgl-project#1078. These are gaps in the unit-test pipeline, each reproduced or traced before the fix:
mainafter the parent merged. Changing the base only firespull_request: edited, which the default trigger types leave out, so both PRs show onlylintand merged with the broken pinned-SGLang toolchain that ci: unbreak unit-test with the pinned SGLang toolchain sgl-project/SpecForge#1078 then had to fix.ensure_flash_attention.shbuilt the patched wheel straight into the persistentSPECFORGE_CI_CACHE/flash-attn/<build_key>/wheels/, before installing it or running the ABI check. A wheel that fails the check, or a truncated one left whenbdist_wheelis killed by the 30-minute step timeout, is reused by every later run with no rebuild, until someone deletes it on the runner. Reproduced with the existing stubs: after an ABI failure the wheel was still cached and the next run downloaded nothing.digests.sha256from the same JSON. uv checked it when it fetched the sdist itself.FLASH_ATTENTION_FORCE_BUILDis no longer covered by tests. Before ci: unbreak unit-test with the pinned SGLang toolchain sgl-project/SpecForge#1078 a test asserted it on the uv command. Now it is only passed tosetup.pyand nothing checks it. Without it, flash-attn'ssetup.pydownloads a prebuilt wheel built against another torch, and the tests still pass.safe.directorygrows every run./github/homepersists across jobs on this runner (that is what keepsSPECFORGE_CI_CACHEalive), sogit config --global --addappends another identical entry on every unit-test run.Modifications
test.yamltypes: [opened, synchronize, reopened, edited]. The GPU jobs run foreditedonly whengithub.event.changes.baseis set.edited. Those runs get their own concurrency group (keyed byrun_id), so they neither cancel a running test nor replace a queued one under the workflow'scancel-in-progress.safe.directoryentry only whengit config --fixed-value --getdoes not already find it (the container has git 2.43).ensure_flash_attention.shcheck_flash_attention, by copying to a hidden staging file in the cache directory and renaming it atomically.sha256sum -cbefore extracting.test_ci_dependencies.pysetup.pyassertsFLASH_ATTENTION_FORCE_BUILD=TRUE.Related Issues
Follow-up to sgl-project#1078. The stacked-PR gap let sgl-project#1065 and sgl-project#1066 merge without unit-test.
Accuracy Test
Not applicable. CI scripts and tests only.
Benchmark & Profiling
Not applicable. CI scripts and tests only.
Testing
python -m unittest tests.test_scripts.test_ci_dependencies: 14 tests pass.safe.directorycases fail.FLASH_ATTENTION_FORCE_BUILD=TRUEremoved from the script, 6 cases fail.pre-commit runon the changed files passes.actionlintreports only the existingqueue: maxkeys, which it does not know yet.build_key, so unit-test reuses it until torch or flash-attn changes.Checklist