[bugfix]: get the full CI suite green on main (pytest 9 flag teardown, Qwen golden env) (#2059)
Co-authored-by: Aryan Kumar <aryan5v@users.noreply.github.com>
This commit is contained in:
co-authored by
Aryan Kumar
parent
de873eae2d
commit
3fe9c0c70d
@@ -0,0 +1,37 @@
|
||||
---
|
||||
date: 2026-11-02
|
||||
experiment: FastVideo CI repair on main (Transformer and Unit lanes)
|
||||
category: infrastructure
|
||||
severity: important
|
||||
---
|
||||
|
||||
# pytest 9 monkeypatch cannot restore torch flag proxies
|
||||
|
||||
## What Happened
|
||||
|
||||
After the CI image picked up pytest 9, the Transformer lane reported 22
|
||||
teardown errors (`'cuBLASModule' object has no attribute 'allow_tf32'`) and
|
||||
the Unit lane reported 28 setup errors in `test_lora_side_branch.py`
|
||||
(`torch._dynamo.config ... has no attribute 'disable'`). The test bodies
|
||||
still passed.
|
||||
|
||||
## Root Cause
|
||||
|
||||
pytest 9's `monkeypatch.setattr` reads the previous value from the target's
|
||||
instance `__dict__`. `torch.backends.cuda.matmul` and `torch._dynamo.config`
|
||||
serve their flags through `__getattr__`, so pytest records the value as
|
||||
missing. Teardown then calls `delattr`: on cuBLAS that raises and leaves TF32
|
||||
changed, and on the dynamo config it deletes the `disable` entry, so every
|
||||
later `setattr` on it fails. pytest 8.4 restores both correctly.
|
||||
|
||||
## Fix / Workaround
|
||||
|
||||
Use the owner's own scoping instead of monkeypatch:
|
||||
`torch._dynamo.config.patch(name=value)` for dynamo/inductor config, and the
|
||||
`tf32_disabled` fixture in `fastvideo/tests/conftest.py` for the TF32 flags.
|
||||
|
||||
## Prevention
|
||||
|
||||
Do not `monkeypatch.setattr` attributes of `torch.backends.cuda.matmul`,
|
||||
`torch._dynamo.config`, `torch._inductor.config` or other torch config
|
||||
proxies. Module functions such as `torch.compiler.is_compiling` are safe.
|
||||
@@ -22,8 +22,9 @@ jobs:
|
||||
trigger:
|
||||
if: >-
|
||||
inputs.pr_number > 0
|
||||
|| (github.event.action == 'labeled' && github.event.label.name == 'ready')
|
||||
|| github.event.action == 'synchronize'
|
||||
|| (github.event.pull_request.base.ref == github.event.repository.default_branch
|
||||
&& ((github.event.action == 'labeled' && github.event.label.name == 'ready')
|
||||
|| github.event.action == 'synchronize'))
|
||||
runs-on: ubuntu-latest
|
||||
# Job-level concurrency: only this guarded job acquires the group, so an
|
||||
# unrelated `labeled` event (which skips the job) cannot cancel an in-flight
|
||||
|
||||
@@ -12,10 +12,52 @@ permissions:
|
||||
issues: write
|
||||
|
||||
jobs:
|
||||
publish-node:
|
||||
name: Publish Custom Node to registry
|
||||
# Any pyproject.toml edit triggers this workflow, but the registry rejects a
|
||||
# version it already has ("The node version already exists"). Publish only
|
||||
# when [project].version changed, as the PyPI workflow does.
|
||||
check-version-change:
|
||||
runs-on: ubuntu-latest
|
||||
if: ${{ github.repository_owner == 'hao-ai-lab' }}
|
||||
outputs:
|
||||
changed: ${{ steps.check.outputs.changed }}
|
||||
steps:
|
||||
- name: Check out code
|
||||
uses: actions/checkout@v4
|
||||
with:
|
||||
fetch-depth: 2
|
||||
- name: Compare [project].version with the version before this push
|
||||
id: check
|
||||
shell: python
|
||||
env:
|
||||
BEFORE_SHA: ${{ github.event.before }}
|
||||
run: |
|
||||
import os
|
||||
import subprocess
|
||||
import tomllib
|
||||
|
||||
def version(text: str) -> str | None:
|
||||
return tomllib.loads(text).get("project", {}).get("version")
|
||||
|
||||
def git(*args: str) -> subprocess.CompletedProcess:
|
||||
return subprocess.run(["git", *args], capture_output=True, text=True)
|
||||
|
||||
# A push can carry several commits; compare with the tip before the push, not HEAD~1.
|
||||
base = os.environ.get("BEFORE_SHA", "").strip()
|
||||
if not base or set(base) == {"0"} or git("fetch", "--no-tags", "--depth=1", "origin", base).returncode:
|
||||
base = "HEAD~1"
|
||||
new = version(open("pyproject.toml", encoding="utf-8").read())
|
||||
previous = git("show", f"{base}:pyproject.toml")
|
||||
old = version(previous.stdout) if previous.returncode == 0 else None
|
||||
changed = new is not None and new != old
|
||||
print(f"base: {base}, previous version: {old}, current version: {new}, changed: {changed}")
|
||||
with open(os.environ["GITHUB_OUTPUT"], "a", encoding="utf-8") as output:
|
||||
output.write(f"changed={str(changed).lower()}\n")
|
||||
|
||||
publish-node:
|
||||
name: Publish Custom Node to registry
|
||||
needs: check-version-change
|
||||
runs-on: ubuntu-latest
|
||||
if: ${{ github.repository_owner == 'hao-ai-lab' && (needs.check-version-change.outputs.changed == 'true' || github.event_name == 'workflow_dispatch') }}
|
||||
steps:
|
||||
- name: Check out code
|
||||
uses: actions/checkout@v4
|
||||
@@ -25,4 +67,4 @@ jobs:
|
||||
uses: Comfy-Org/publish-node-action@v1
|
||||
with:
|
||||
## Add your own personal access token to your Github Repository secrets and reference it here.
|
||||
personal_access_token: ${{ secrets.REGISTRY_ACCESS_TOKEN }}
|
||||
personal_access_token: ${{ secrets.REGISTRY_ACCESS_TOKEN }}
|
||||
|
||||
@@ -28,14 +28,34 @@ def fa4_env_fingerprint(environ: Mapping[str, str] | None = None, ) -> tuple[tup
|
||||
return tuple(pairs)
|
||||
|
||||
|
||||
def _cached_entries(cache: object) -> dict | None:
|
||||
"""The in-memory entries of an FA4 JIT cache, or None if ``cache`` is not one.
|
||||
|
||||
Older FA4 builds keep a plain dict; the pinned ref wraps it in ``cache_utils.JITCache``
|
||||
(entries under ``.cache``), which is not a dict, so checking only for dicts cleared nothing.
|
||||
"""
|
||||
if isinstance(cache, dict):
|
||||
return cache
|
||||
entries = getattr(cache, "cache", None)
|
||||
if isinstance(entries, dict) and callable(getattr(cache, "clear", None)):
|
||||
return entries
|
||||
return None
|
||||
|
||||
|
||||
def clear_fa4_jit_caches(kernel_objects: Iterable[object]) -> int:
|
||||
"""Drop every FA4 JIT cache on ``kernel_objects``; return entries dropped."""
|
||||
"""Drop every FA4 JIT cache on ``kernel_objects``; return entries dropped.
|
||||
|
||||
Clearing goes through the cache's own ``clear()``. For FA4's opt-in persistent cache
|
||||
(FLASH_ATTENTION_CUTE_DSL_CACHE_ENABLED=1) that also purges its on-disk kernels, whose keys
|
||||
omit the tuning env as well; it only happens when compiled kernels are held in memory.
|
||||
"""
|
||||
cleared = 0
|
||||
for kernel_object in kernel_objects:
|
||||
for attribute in _JIT_CACHE_ATTRS:
|
||||
cache = getattr(kernel_object, attribute, None)
|
||||
if isinstance(cache, dict) and cache:
|
||||
cleared += len(cache)
|
||||
entries = _cached_entries(cache)
|
||||
if entries:
|
||||
cleared += len(entries)
|
||||
cache.clear()
|
||||
return cleared
|
||||
|
||||
|
||||
@@ -49,14 +49,11 @@ def test_cudnn_route_presents_bhsd_strided_views():
|
||||
def test_cudnn_route_is_bit_identical_to_cudnn_dispatched_sdpa():
|
||||
query, key, value = _qkv(2048)
|
||||
scale = 1.0 / math.sqrt(query.shape[-1])
|
||||
previous = torch.backends.cuda.cudnn_sdp_enabled()
|
||||
torch.backends.cuda.enable_cudnn_sdp(True)
|
||||
try:
|
||||
with torch.no_grad():
|
||||
reference = torch.nn.functional.scaled_dot_product_attention(
|
||||
query.transpose(1, 2), key.transpose(1, 2), value.transpose(1, 2), scale=scale).transpose(1, 2)
|
||||
finally:
|
||||
torch.backends.cuda.enable_cudnn_sdp(previous)
|
||||
# Pin the reference to cuDNN: enabling it only makes it eligible, and the dispatcher
|
||||
# prefers another backend on some GPUs (GB10), which is not bit-identical.
|
||||
with torch.no_grad(), torch.nn.attention.sdpa_kernel(torch.nn.attention.SDPBackend.CUDNN_ATTENTION):
|
||||
reference = torch.nn.functional.scaled_dot_product_attention(
|
||||
query.transpose(1, 2), key.transpose(1, 2), value.transpose(1, 2), scale=scale).transpose(1, 2)
|
||||
|
||||
with torch.no_grad():
|
||||
output = cudnn_sdpa_bshd(query, key, value, scale=scale)
|
||||
|
||||
@@ -98,6 +98,35 @@ def test_unrelated_environment_changes_do_not_drop_kernels() -> None:
|
||||
assert list(fwd.compile_cache) == ["a"]
|
||||
|
||||
|
||||
class _JITCacheLike:
|
||||
"""Shape of ``flash_attn.cute.cache_utils.JITCache`` at the pinned FA4 ref: not a dict."""
|
||||
|
||||
def __init__(self, entries: dict) -> None:
|
||||
self.cache = dict(entries)
|
||||
|
||||
def __contains__(self, key) -> bool:
|
||||
return key in self.cache
|
||||
|
||||
def clear(self) -> None:
|
||||
self.cache.clear()
|
||||
|
||||
|
||||
def test_jit_cache_objects_are_cleared_through_their_own_clear() -> None:
|
||||
"""The pinned FA4 wraps its compile cache in a JITCache; a dict-only check cleared nothing."""
|
||||
fwd = SimpleNamespace(compile_cache=_JITCacheLike({"a": object(), "b": object()}))
|
||||
bwd = SimpleNamespace(compile_cache=_JITCacheLike({}), compile_cache_pre=_JITCacheLike({"c": object()}))
|
||||
|
||||
assert fa4_env_cache.clear_fa4_jit_caches((fwd, bwd)) == 3
|
||||
assert fwd.compile_cache.cache == {}
|
||||
assert bwd.compile_cache_pre.cache == {}
|
||||
|
||||
fwd.compile_cache.cache["a"] = object()
|
||||
assert fa4_env_cache.invalidate_fa4_jit_caches_on_env_change((fwd, ), {"FA_X": "1"}) is True
|
||||
fwd.compile_cache.cache["a"] = object()
|
||||
assert fa4_env_cache.invalidate_fa4_jit_caches_on_env_change((fwd, ), {"FA_X": "0"}) is True
|
||||
assert "a" not in fwd.compile_cache
|
||||
|
||||
|
||||
def test_missing_or_non_dict_caches_are_ignored() -> None:
|
||||
bare = SimpleNamespace()
|
||||
non_dict = SimpleNamespace(compile_cache=["not", "a", "dict"])
|
||||
|
||||
@@ -45,3 +45,22 @@ def env_overrides():
|
||||
"""
|
||||
with contextlib.ExitStack() as stack:
|
||||
yield stack
|
||||
|
||||
|
||||
@pytest.fixture
|
||||
def tf32_disabled():
|
||||
"""Disable TF32 for CUDA matmuls and cuDNN during the test, then restore the previous flags.
|
||||
|
||||
``torch.backends.cuda.matmul`` serves ``allow_tf32`` through ``__getattr__``. pytest 9's
|
||||
``monkeypatch.setattr`` reads the old value from the instance ``__dict__``, finds nothing, and
|
||||
its teardown ``delattr`` raises instead of restoring the flag. Restore through the same legacy
|
||||
flags: mixing them with the per-backend ``fp32_precision`` API makes later ``allow_tf32`` reads raise.
|
||||
"""
|
||||
matmul, cudnn = torch.backends.cuda.matmul, torch.backends.cudnn
|
||||
previous = (matmul.allow_tf32, cudnn.allow_tf32)
|
||||
matmul.allow_tf32 = False
|
||||
cudnn.allow_tf32 = False
|
||||
try:
|
||||
yield
|
||||
finally:
|
||||
matmul.allow_tf32, cudnn.allow_tf32 = previous
|
||||
|
||||
@@ -187,6 +187,9 @@ def test_merge_comment_has_one_change_aware_trigger_path():
|
||||
assert "pr.state !== 'open'" in ready_workflow
|
||||
assert "pr.base.repo.full_name !== context.payload.repository.full_name" in ready_workflow
|
||||
assert "pr.base.ref !== context.payload.repository.default_branch" in ready_workflow
|
||||
trigger_if = yaml.safe_load(ready_workflow)["jobs"]["trigger"]["if"]
|
||||
assert "github.event.pull_request.base.ref == github.event.repository.default_branch" in trigger_if
|
||||
assert trigger_if.startswith("inputs.pr_number > 0 ||")
|
||||
assert "core.setOutput('head_sha', pr.head.sha)" in ready_workflow
|
||||
assert "core.setOutput('base_sha', pr.base.sha)" in ready_workflow
|
||||
assert "api.buildkite.com" in ready_workflow
|
||||
|
||||
@@ -6,10 +6,12 @@ the target image. Causal text segments use masked SDPA; reference and target
|
||||
segments use FlashAttention, including their unequal query/key lengths. Complex RoPE and
|
||||
the two modulation rows match the causal-conditioning block interface.
|
||||
|
||||
Requires a BF16-capable CUDA GPU and FlashAttention. Select FA4 with
|
||||
FASTVIDEO_FA4=1 before starting Python. The shared harness downloads
|
||||
only checkpoint shards containing block 0 and compares a device/runtime-matched
|
||||
golden exactly; it never loads the text encoder or VAE.
|
||||
Requires a BF16-capable CUDA GPU and FlashAttention. Like the other block
|
||||
goldens, the published references match the golden-gate CI lane: FA2 with
|
||||
FASTVIDEO_FA4 off. Running with FASTVIDEO_FA4=1 reports an environment
|
||||
mismatch rather than drift. The shared harness downloads only checkpoint
|
||||
shards containing block 0 and compares a device/runtime-matched golden
|
||||
exactly; it never loads the text encoder or VAE.
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
@@ -15,8 +15,10 @@ IN, OUT, RANK = 32, 24, 16
|
||||
|
||||
@pytest.fixture(autouse=True)
|
||||
def _cpu_merge(monkeypatch: pytest.MonkeyPatch):
|
||||
monkeypatch.setattr(torch._dynamo.config, "disable", True)
|
||||
# config.patch restores the flag; a monkeypatch.setattr teardown would delete it (pytest 9).
|
||||
monkeypatch.setattr(lora_linear, "get_local_torch_device", lambda: torch.device("cpu"))
|
||||
with torch._dynamo.config.patch(disable=True):
|
||||
yield
|
||||
|
||||
|
||||
def _merged(adapter_dtype: torch.dtype) -> tuple[torch.Tensor, torch.Tensor, torch.Tensor, torch.Tensor]:
|
||||
|
||||
@@ -23,9 +23,21 @@ IN, OUT, RANK = 16, 12, 4
|
||||
|
||||
@pytest.fixture(autouse=True)
|
||||
def _eager_forward(monkeypatch: pytest.MonkeyPatch):
|
||||
"""``BaseLayerWithLoRA.forward`` is ``torch.compile``d; run it eagerly, and merge on CPU even on MPS hosts."""
|
||||
monkeypatch.setattr(torch._dynamo.config, "disable", True)
|
||||
"""``BaseLayerWithLoRA.forward`` is ``torch.compile``d; run it eagerly, and merge on CPU even on MPS hosts.
|
||||
|
||||
Dynamo flags go through ``config.patch``: its module serves them via ``__getattr__``, so a
|
||||
``monkeypatch.setattr`` teardown deletes the flag instead of restoring it (pytest 9).
|
||||
"""
|
||||
monkeypatch.setattr(lora_linear, "get_local_torch_device", lambda: torch.device("cpu"))
|
||||
with torch._dynamo.config.patch(disable=True):
|
||||
yield
|
||||
|
||||
|
||||
@pytest.fixture
|
||||
def dynamo_enabled():
|
||||
"""Re-enable dynamo inside the autouse eager scope for tests that compile on purpose."""
|
||||
with torch._dynamo.config.patch(disable=False):
|
||||
yield
|
||||
|
||||
|
||||
class _PackedMethod:
|
||||
@@ -84,14 +96,12 @@ def test_wrapped_layer_at_strength_zero_is_bitwise_the_base_layer(packed: bool)
|
||||
|
||||
@pytest.mark.parametrize("backend", ["eager", "inductor"])
|
||||
@pytest.mark.parametrize("adapter", [False, True])
|
||||
def test_wrapping_does_not_change_a_compiled_block(monkeypatch: pytest.MonkeyPatch, backend: str,
|
||||
adapter: bool) -> None:
|
||||
def test_wrapping_does_not_change_a_compiled_block(dynamo_enabled, backend: str, adapter: bool) -> None:
|
||||
"""Under a caller's compile the base layer is traced with the block, as it is unwrapped; the wrapper adds no
|
||||
region of its own, so the compiled block is unchanged with no adapter or at strength 0."""
|
||||
if backend == "inductor" and not torch.cuda.is_available():
|
||||
pytest.skip("Inductor execution requires a CUDA GPU for this regression")
|
||||
device = torch.device("cuda" if backend == "inductor" else "cpu")
|
||||
monkeypatch.setattr(torch._dynamo.config, "disable", False)
|
||||
torch._dynamo.reset()
|
||||
|
||||
class Block(torch.nn.Module):
|
||||
@@ -117,9 +127,9 @@ def test_wrapping_does_not_change_a_compiled_block(monkeypatch: pytest.MonkeyPat
|
||||
torch._dynamo.reset()
|
||||
|
||||
|
||||
def test_wrapper_adds_no_compiled_region_around_the_base_layer(monkeypatch: pytest.MonkeyPatch) -> None:
|
||||
def test_wrapper_adds_no_compiled_region_around_the_base_layer(monkeypatch: pytest.MonkeyPatch,
|
||||
dynamo_enabled) -> None:
|
||||
"""Compiling a packed base layer on its own changes its numerics, so an uncompiled caller runs it eagerly."""
|
||||
monkeypatch.setattr(torch._dynamo.config, "disable", False)
|
||||
torch._dynamo.reset()
|
||||
base = _packed_linear()
|
||||
layer = get_lora_layer(base)
|
||||
@@ -179,11 +189,11 @@ def test_side_branch_chunked_update_matches_the_full_product(monkeypatch: pytest
|
||||
|
||||
|
||||
@pytest.mark.parametrize("backend", ["eager", "inductor"])
|
||||
def test_side_branch_chunked_update_compiles_fullgraph(monkeypatch: pytest.MonkeyPatch, backend: str) -> None:
|
||||
def test_side_branch_chunked_update_compiles_fullgraph(monkeypatch: pytest.MonkeyPatch, dynamo_enabled,
|
||||
backend: str) -> None:
|
||||
if backend == "inductor" and not torch.cuda.is_available():
|
||||
pytest.skip("Inductor execution requires a CUDA GPU for this regression")
|
||||
device = torch.device("cuda" if backend == "inductor" else "cpu")
|
||||
monkeypatch.setattr(torch._dynamo.config, "disable", False)
|
||||
monkeypatch.setattr(lora_linear, "_SIDE_BRANCH_CHUNK_BYTES", 64)
|
||||
torch._dynamo.reset()
|
||||
base = _packed_linear().to(device)
|
||||
@@ -221,9 +231,8 @@ def test_side_branch_chunked_update_compiles_fullgraph(monkeypatch: pytest.Monke
|
||||
torch._dynamo.reset()
|
||||
|
||||
|
||||
def test_side_branch_strength_changes_do_not_recompile(monkeypatch: pytest.MonkeyPatch) -> None:
|
||||
def test_side_branch_strength_changes_do_not_recompile(dynamo_enabled) -> None:
|
||||
"""A float strength is guarded by value, so fullgraph would fail after ``recompile_limit`` distinct strengths."""
|
||||
monkeypatch.setattr(torch._dynamo.config, "disable", False)
|
||||
torch._dynamo.reset()
|
||||
base = _packed_linear()
|
||||
layer = get_lora_layer(base)
|
||||
|
||||
@@ -25,7 +25,7 @@ pytestmark = pytest.mark.skipif(not torch.cuda.is_available(), reason="Requires
|
||||
|
||||
|
||||
@pytest.fixture
|
||||
def hunyuan_setup(monkeypatch, request):
|
||||
def hunyuan_setup(request):
|
||||
# Environment writes go through the registry helpers (docs/contributing/env_vars.md).
|
||||
with contextlib.ExitStack() as stack:
|
||||
# Respect the port reserved by the CI runner when sharing a GPU host.
|
||||
@@ -37,8 +37,7 @@ def hunyuan_setup(monkeypatch, request):
|
||||
if "MASTER_ADDR" not in os.environ:
|
||||
stack.enter_context(envs.override_external("MASTER_ADDR", "127.0.0.1"))
|
||||
stack.enter_context(envs.FASTVIDEO_ATTENTION_BACKEND.override("TORCH_SDPA"))
|
||||
monkeypatch.setattr(torch.backends.cuda.matmul, "allow_tf32", False)
|
||||
monkeypatch.setattr(torch.backends.cudnn, "allow_tf32", False)
|
||||
request.getfixturevalue("tf32_disabled")
|
||||
request.getfixturevalue("distributed_setup")
|
||||
yield
|
||||
|
||||
|
||||
@@ -24,12 +24,11 @@ from fastvideo.layers.linear import ReplicatedLinear
|
||||
from fastvideo.platforms import AttentionBackendEnum
|
||||
|
||||
@pytest.fixture
|
||||
def qwen_setup(env_overrides, monkeypatch, request):
|
||||
def qwen_setup(env_overrides, request):
|
||||
if not torch.cuda.is_available():
|
||||
pytest.skip("Requires one CUDA GPU")
|
||||
env_overrides.enter_context(envs.FASTVIDEO_ATTENTION_BACKEND.override("TORCH_SDPA"))
|
||||
monkeypatch.setattr(torch.backends.cuda.matmul, "allow_tf32", False)
|
||||
monkeypatch.setattr(torch.backends.cudnn, "allow_tf32", False)
|
||||
request.getfixturevalue("tf32_disabled")
|
||||
request.getfixturevalue("distributed_setup")
|
||||
with torch.no_grad(), sdpa_kernel(SDPBackend.MATH):
|
||||
yield torch.device("cuda")
|
||||
|
||||
Reference in New Issue
Block a user