CI(Core): MoE per-family coverage + GRPO patches + grouped_gemm AST
New step "MoE per-family coverage + GRPO patches + grouped_gemm AST" that hardens the matrix against the recurring MoE bug class behind unslothai/unsloth-zoo#624 / #612 / #607 / #601 and unslothai/unsloth #4934 / #3598. Five clusters of pytest cases inside one shim: 1. Per-MoE-family side-effect contract (8 parametrized cases): For each `patch_*_moe` in unsloth_zoo.temporary_patches.{qwen3_moe, qwen3_5_moe, qwen3_next_moe, qwen3_vl_moe, gemma4_moe, glm4_moe, deepseek_v3_moe, gpt_oss}, look up the transformers target classes, skip when none import on this matrix cell, run the patch fn, and assert at least one importable target now carries an unsloth "patched" marker. Accepts five marker conventions used across the codebase (_unsloth_already_patched, _unsloth_lora_patched, _unsloth_lora_extractor_fn, _original_<modeling_tail>_<cls>_forward, plain _original_forward). Surfaces silent early-returns (PR #612) that escape the registration-coverage test. gpt_oss specifically reads UNSLOTH_MODEL_NAME and only runs on transformers >= 5; the shim sets the env var via monkeypatch and skips on the 4.57.6 cell with a documented reason. 2. PR #4934 (TRL 1.0 GRPO disable_gradient_checkpointing): rebinding contract. After patch_trl_disable_gradient_checkpointing(), the no-op decorated function MUST be the symbol on trl.models.utils AND every trl.* module that imported it by reference. Skips on TRL < 1.0 (no symbol present). 3. PR #3598 (gradient_accumulation): patch_gradient_accumulation_fix on a vanilla transformers.Trainer must run cleanly without raising AND be idempotent. Catches future double-scale or import-injection regressions in the source rewriter. 4. unsloth/kernels/moe/grouped_gemm AST smoke: walks every .py under the directory (12 files) and asserts ast.parse succeeds. Triton kernels are GPU-only at runtime, but a syntax error in source surfaces as ImportError on every install. Also sanity-checks the directory layout (interface.py, kernels/forward.py, kernels/backward.py, reference/moe_block.py, reference/moe_ops.py must exist). Local verification on host TRL 0.25.1 + transformers 4.57.6: 4 pass (qwen3_moe, qwen3_vl_moe, GRPO disable-GC, grad-accum, grouped_gemm AST), 7 skip legitimately (qwen3_5/qwen3_next/gemma4/glm4/deepseek/ gpt_oss absent or version-gated). Wall-time ~10s on host; budget ~30-60s per matrix cell.
This commit is contained in:
Daniel Han
parent
7855571abb
commit
a26efef739
1 changed files with 419 additions and 0 deletions
419
.github/workflows/consolidated-tests-ci.yml
vendored
419
.github/workflows/consolidated-tests-ci.yml
vendored
|
|
@ -1531,6 +1531,425 @@ jobs:
|
|||
python -m pytest -q --tb=short -s tests/_trl_trainer_discovery_shim.py
|
||||
rm -f tests/_trl_trainer_discovery_shim.py
|
||||
|
||||
- name: MoE per-family coverage + GRPO patches + grouped_gemm AST
|
||||
# Catches the recurring class of bugs that PR #624 (gemma4 missing
|
||||
# extractor), PR #612 (gemma4 GRPO patch silently dropped), PR #607
|
||||
# (gate_up LoRA dropped from grad graph), PR #601 (qwen MoE shape
|
||||
# mismatch), unsloth#4934 (TRL disable_gradient_checkpointing
|
||||
# corrupts unsloth GC), and unsloth#3598 (gradient_accumulation
|
||||
# double-scale on accepts_loss_kwargs=False) targeted. Coverage:
|
||||
#
|
||||
# 1. Per-MoE-family side-effect contract: for every patch_*_moe
|
||||
# function in unsloth_zoo.temporary_patches, if its target
|
||||
# transformers class is importable on this matrix cell, the
|
||||
# patch must mark the class with `_unsloth_already_patched=True`
|
||||
# after running. This is exactly what unsloth_zoo's existing
|
||||
# test_moe_lora_extractor_coverage walks at the registration
|
||||
# level; here we tie each patch fn to its declared target so a
|
||||
# silent early-return (PR #612 style) surfaces as red rather
|
||||
# than a coverage skip.
|
||||
#
|
||||
# 2. PR #4934 (GRPO + TRL 1.0): patch_trl_disable_gradient_checkpointing
|
||||
# must rebind trl.models.utils.disable_gradient_checkpointing to
|
||||
# the unsloth no-op AND propagate the rebinding to every trl.*
|
||||
# module that imported the symbol by reference.
|
||||
#
|
||||
# 3. PR #3598 (gradient_accumulation): patch_gradient_accumulation_fix
|
||||
# must run cleanly on a synthetic Trainer whose training_step
|
||||
# signature carries `num_items_in_batch`. The original bug was
|
||||
# that `accepts_loss_kwargs=False` (Qwen3VL, Gemma3 in t-4.57)
|
||||
# caused double loss-scaling; here we verify the rewrite path
|
||||
# itself does not raise on a CPU-resolvable shape.
|
||||
#
|
||||
# 4. unsloth/kernels/moe/grouped_gemm AST smoke: the Triton kernels
|
||||
# are GPU-only at runtime, but a SyntaxError or stray
|
||||
# string-literal in the source still surfaces as a test-time
|
||||
# ImportError on every install. ast.parse the .py files without
|
||||
# executing.
|
||||
#
|
||||
# Wall-time per cell ~30-60s. Routed through pytest for the spoof
|
||||
# harness so unsloth_zoo.temporary_patches imports are clean.
|
||||
run: |
|
||||
set -euxo pipefail
|
||||
cat > tests/_moe_coverage_shim.py <<'PY'
|
||||
# Auto-generated by .github/workflows/consolidated-tests-ci.yml.
|
||||
import sys, pathlib, ast, importlib, importlib.util, contextlib, os
|
||||
sys.path.insert(0, str(pathlib.Path(__file__).parent))
|
||||
import _zoo_aggressive_cuda_spoof as _spoof
|
||||
_spoof.apply()
|
||||
|
||||
import pytest
|
||||
|
||||
# Map each MoE patch function to the transformers classes it is
|
||||
# contractually responsible for marking with _unsloth_already_patched
|
||||
# after a successful run. Sourced from
|
||||
# unsloth_zoo/temporary_patches/<family>_moe.py:
|
||||
# - qwen3_moe.py:382-398 patches Qwen3MoeExperts (new path) or
|
||||
# Qwen3MoeSparseMoeBlock (old path).
|
||||
# - qwen3_5_moe.py + qwen3_next_moe.py + qwen3_vl_moe.py register
|
||||
# extractors on Qwen3_5MoeExperts / Qwen3NextExperts /
|
||||
# Qwen3VLMoeTextExperts respectively.
|
||||
# - gemma4_moe.py marks Gemma4TextExperts (current) or
|
||||
# Gemma4TextMoEBlock (legacy).
|
||||
# - glm4_moe.py marks Glm4MoeLiteNaiveMoe.
|
||||
# - deepseek_v3_moe.py marks DeepseekV3NaiveMoe.
|
||||
# - gpt_oss.py:patch_gpt_oss_moe_for_lora marks GptOssExperts.
|
||||
# Each cell skips a target if the transformers version lacks it
|
||||
# (legitimate version-skew); only patches with at least one
|
||||
# importable target are exercised.
|
||||
# Each entry = ((patch_module, patch_fn), targets, env_setup,
|
||||
# version_gate). env_setup runs before the patch fn (e.g. set
|
||||
# UNSLOTH_MODEL_NAME for gpt_oss). version_gate is a callable
|
||||
# returning True when the patch SHOULD run on this transformers;
|
||||
# if False, the test skips with a documented reason.
|
||||
def _v5_or_later():
|
||||
try:
|
||||
import transformers
|
||||
major = int(transformers.__version__.split(".")[0])
|
||||
return major >= 5
|
||||
except Exception:
|
||||
return False
|
||||
|
||||
MOE_PATCHES = [
|
||||
{
|
||||
"module": "unsloth_zoo.temporary_patches.qwen3_moe",
|
||||
"fn": "patch_qwen3_moe",
|
||||
"targets": [
|
||||
("transformers.models.qwen3_moe.modeling_qwen3_moe", "Qwen3MoeExperts"),
|
||||
("transformers.models.qwen3_moe.modeling_qwen3_moe", "Qwen3MoeSparseMoeBlock"),
|
||||
],
|
||||
"env": {},
|
||||
"gate": lambda: True,
|
||||
"gate_reason": "",
|
||||
},
|
||||
{
|
||||
"module": "unsloth_zoo.temporary_patches.qwen3_5_moe",
|
||||
"fn": "patch_qwen3_5_moe",
|
||||
"targets": [
|
||||
("transformers.models.qwen3_5_moe.modeling_qwen3_5_moe", "Qwen3_5MoeExperts"),
|
||||
],
|
||||
"env": {}, "gate": lambda: True, "gate_reason": "",
|
||||
},
|
||||
{
|
||||
"module": "unsloth_zoo.temporary_patches.qwen3_next_moe",
|
||||
"fn": "patch_qwen3_next_moe",
|
||||
"targets": [
|
||||
("transformers.models.qwen3_next.modeling_qwen3_next", "Qwen3NextExperts"),
|
||||
],
|
||||
"env": {}, "gate": lambda: True, "gate_reason": "",
|
||||
},
|
||||
{
|
||||
"module": "unsloth_zoo.temporary_patches.qwen3_vl_moe",
|
||||
"fn": "patch_qwen3_vl_moe",
|
||||
"targets": [
|
||||
("transformers.models.qwen3_vl_moe.modeling_qwen3_vl_moe", "Qwen3VLMoeTextExperts"),
|
||||
],
|
||||
"env": {}, "gate": lambda: True, "gate_reason": "",
|
||||
},
|
||||
{
|
||||
"module": "unsloth_zoo.temporary_patches.gemma4_moe",
|
||||
"fn": "patch_gemma4_moe",
|
||||
"targets": [
|
||||
("transformers.models.gemma4.modeling_gemma4", "Gemma4TextExperts"),
|
||||
],
|
||||
"env": {}, "gate": lambda: True, "gate_reason": "",
|
||||
},
|
||||
{
|
||||
"module": "unsloth_zoo.temporary_patches.glm4_moe",
|
||||
"fn": "patch_glm4_moe",
|
||||
"targets": [
|
||||
("transformers.models.glm4_moe.modeling_glm4_moe", "Glm4MoeLiteNaiveMoe"),
|
||||
],
|
||||
"env": {}, "gate": lambda: True, "gate_reason": "",
|
||||
},
|
||||
{
|
||||
"module": "unsloth_zoo.temporary_patches.deepseek_v3_moe",
|
||||
"fn": "patch_deepseek_v3_moe",
|
||||
"targets": [
|
||||
("transformers.models.deepseek_v3.modeling_deepseek_v3", "DeepseekV3NaiveMoe"),
|
||||
],
|
||||
"env": {}, "gate": lambda: True, "gate_reason": "",
|
||||
},
|
||||
{
|
||||
"module": "unsloth_zoo.temporary_patches.gpt_oss",
|
||||
"fn": "patch_gpt_oss_moe_for_lora",
|
||||
"targets": [
|
||||
("transformers.models.gpt_oss.modeling_gpt_oss", "GptOssExperts"),
|
||||
],
|
||||
# The patch reads UNSLOTH_MODEL_NAME and only runs when
|
||||
# "gpt_oss" is in the normalized form. Set it explicitly
|
||||
# so the gate at gpt_oss.py:1387 passes; otherwise the
|
||||
# patch silently early-returns and the test would
|
||||
# spuriously fail.
|
||||
"env": {"UNSLOTH_MODEL_NAME": "gpt_oss"},
|
||||
# Additionally only runs on transformers >= 5
|
||||
# (gpt_oss.py:1392 `_is_transformers_v5()` gate).
|
||||
"gate": _v5_or_later,
|
||||
"gate_reason": (
|
||||
"patch_gpt_oss_moe_for_lora gates on "
|
||||
"transformers >= 5 (split-LoRA grouped_mm path)"
|
||||
),
|
||||
},
|
||||
]
|
||||
|
||||
|
||||
def _resolve_target_classes(targets):
|
||||
"""Return [(qual, cls), ...] for every importable target."""
|
||||
out = []
|
||||
for mod_path, cls_name in targets:
|
||||
try:
|
||||
mod = importlib.import_module(mod_path)
|
||||
except Exception:
|
||||
continue
|
||||
cls = getattr(mod, cls_name, None)
|
||||
if cls is None:
|
||||
continue
|
||||
out.append((f"{mod_path}.{cls_name}", cls))
|
||||
return out
|
||||
|
||||
|
||||
@pytest.mark.parametrize(
|
||||
"spec",
|
||||
MOE_PATCHES,
|
||||
ids=lambda s: s["fn"],
|
||||
)
|
||||
def test_moe_patch_marks_its_target_when_class_present(spec, monkeypatch):
|
||||
"""If at least one target class is importable AND the
|
||||
version gate passes, run the patch fn and assert at least
|
||||
one target is marked patched afterwards. Skips when the
|
||||
transformers version lacks every target or when the
|
||||
version gate blocks the patch (legitimate). Fails on
|
||||
silent patch-fn early-returns (PR #612 class of bug)."""
|
||||
targets = spec["targets"]
|
||||
patch_module = spec["module"]
|
||||
patch_name = spec["fn"]
|
||||
importable = _resolve_target_classes(targets)
|
||||
if not importable:
|
||||
pytest.skip(
|
||||
f"{patch_name}: no target class importable on this "
|
||||
f"transformers (looked for {[c for _, c in targets]})."
|
||||
)
|
||||
if not spec["gate"]():
|
||||
pytest.skip(
|
||||
f"{patch_name}: version gate blocks this cell. "
|
||||
f"Reason: {spec['gate_reason']}"
|
||||
)
|
||||
for k, v in spec["env"].items():
|
||||
monkeypatch.setenv(k, v)
|
||||
try:
|
||||
pmod = importlib.import_module(patch_module)
|
||||
except Exception as e:
|
||||
pytest.skip(
|
||||
f"{patch_module} import failed (likely optional dep): "
|
||||
f"{type(e).__name__}: {e}"
|
||||
)
|
||||
fn = getattr(pmod, patch_name, None)
|
||||
if fn is None or not callable(fn):
|
||||
pytest.skip(f"{patch_module} has no callable {patch_name}")
|
||||
try:
|
||||
fn()
|
||||
except Exception as e:
|
||||
raise AssertionError(
|
||||
f"{patch_name}() raised on a transformers that "
|
||||
f"DOES ship at least one target class ({importable}). "
|
||||
f"This is the silent-failure mode PR #612 fixed: "
|
||||
f"{type(e).__name__}: {e}"
|
||||
)
|
||||
# At least one importable target must now carry SOME marker
|
||||
# showing unsloth touched it. Accepted signals (each is set
|
||||
# by a different patch flow in unsloth_zoo):
|
||||
# - `_unsloth_already_patched=True` (gemma4, deepseek_v3, glm4)
|
||||
# - `_unsloth_lora_patched=True` (gpt_oss_moe_for_lora)
|
||||
# - `_unsloth_lora_extractor_fn` is callable (qwen3_*, glm4_moe)
|
||||
# - `_original_<modeling_tail>_<ClassName>_forward` attr
|
||||
# (set by patch_function: qwen3_moe SparseMoeBlock, etc.)
|
||||
# - `_original_forward` attribute (gpt_oss in-place patch)
|
||||
# Accept any one as "patched".
|
||||
def _is_patched(cls) -> bool:
|
||||
if getattr(cls, "_unsloth_already_patched", False) is True:
|
||||
return True
|
||||
if getattr(cls, "_unsloth_lora_patched", False) is True:
|
||||
return True
|
||||
if callable(getattr(cls, "_unsloth_lora_extractor_fn", None)):
|
||||
return True
|
||||
if "_original_forward" in dir(cls):
|
||||
return True
|
||||
cls_name = cls.__name__
|
||||
for attr in dir(cls):
|
||||
if attr.startswith("_original_") and attr.endswith(
|
||||
f"_{cls_name}_forward"
|
||||
):
|
||||
return True
|
||||
return False
|
||||
|
||||
after = _resolve_target_classes(targets)
|
||||
marked = [qual for qual, cls in after if _is_patched(cls)]
|
||||
if not marked:
|
||||
raise AssertionError(
|
||||
f"{patch_name}() ran without exception but no target "
|
||||
f"in {importable} carries any of the unsloth markers "
|
||||
"(_unsloth_already_patched / _unsloth_lora_patched / "
|
||||
"_unsloth_lora_extractor_fn / _original_*_forward). "
|
||||
"Patch silently no-op'd (PR #612 class of bug)."
|
||||
)
|
||||
print(f" {patch_name}: marked {marked}")
|
||||
|
||||
|
||||
# ---- PR #4934 (TRL 1.0+ GRPO disable_gradient_checkpointing) ----
|
||||
|
||||
def test_patch_trl_disable_gradient_checkpointing():
|
||||
"""unsloth/models/rl.py:patch_trl_disable_gradient_checkpointing
|
||||
must rebind trl.models.utils.disable_gradient_checkpointing to
|
||||
the unsloth no-op when TRL >= 1.0. Pre-1.0 TRL has no such
|
||||
symbol -> the patch returns early."""
|
||||
try:
|
||||
import trl.models.utils as _tmu
|
||||
except ImportError:
|
||||
pytest.skip("trl not installed")
|
||||
had_symbol = hasattr(_tmu, "disable_gradient_checkpointing")
|
||||
try:
|
||||
from unsloth.models.rl import patch_trl_disable_gradient_checkpointing
|
||||
except ImportError:
|
||||
pytest.skip(
|
||||
"unsloth.models.rl.patch_trl_disable_gradient_checkpointing "
|
||||
"absent (older unsloth than #4934)"
|
||||
)
|
||||
patch_trl_disable_gradient_checkpointing()
|
||||
if not had_symbol:
|
||||
# Pre-1.0 TRL: patch is a no-op early-return. Verify
|
||||
# nothing broke.
|
||||
pytest.skip(
|
||||
"TRL pre-1.0 has no disable_gradient_checkpointing; "
|
||||
"patch correctly early-returned."
|
||||
)
|
||||
fn = getattr(_tmu, "disable_gradient_checkpointing", None)
|
||||
assert fn is not None, (
|
||||
"trl.models.utils.disable_gradient_checkpointing missing "
|
||||
"after patch -- patch removed the symbol entirely?"
|
||||
)
|
||||
assert getattr(fn, "_unsloth_noop_patched", False) is True, (
|
||||
"trl.models.utils.disable_gradient_checkpointing was NOT "
|
||||
"rebound to the unsloth no-op. PR #4934 regression."
|
||||
)
|
||||
# PR #4934 also walks sys.modules to rebind trl.* modules
|
||||
# that imported the symbol by reference. Verify at least the
|
||||
# canonical trainer modules picked up the rebinding when
|
||||
# they re-export it.
|
||||
import sys
|
||||
checked = 0
|
||||
missed = []
|
||||
for mod_name, mod in list(sys.modules.items()):
|
||||
if not mod_name.startswith("trl."):
|
||||
continue
|
||||
bound = getattr(mod, "disable_gradient_checkpointing", None)
|
||||
if bound is None:
|
||||
continue
|
||||
checked += 1
|
||||
if not getattr(bound, "_unsloth_noop_patched", False):
|
||||
missed.append(mod_name)
|
||||
print(f" rebound disable_gradient_checkpointing in {checked} trl.* modules")
|
||||
assert not missed, (
|
||||
"trl.* modules that imported disable_gradient_checkpointing "
|
||||
f"by reference but did not get rebound: {missed}"
|
||||
)
|
||||
|
||||
|
||||
# ---- PR #3598 (gradient_accumulation loss-scaling rewrite) ----
|
||||
|
||||
def test_patch_gradient_accumulation_fix_runs_on_synthetic_trainer():
|
||||
"""patch_gradient_accumulation_fix rewrites a Trainer's
|
||||
`training_step` source via inspect+exec when the signature
|
||||
carries `num_items_in_batch`. PR #3598 fixed the rewrite
|
||||
path to not double-scale for trainers with
|
||||
`accepts_loss_kwargs=False`. Verify the patch fn runs
|
||||
without raising on a synthetic Trainer carrying that
|
||||
signature."""
|
||||
try:
|
||||
from unsloth.models._utils import patch_gradient_accumulation_fix
|
||||
except ImportError:
|
||||
pytest.skip(
|
||||
"unsloth.models._utils.patch_gradient_accumulation_fix absent"
|
||||
)
|
||||
try:
|
||||
from transformers import Trainer
|
||||
except ImportError:
|
||||
pytest.skip("transformers.Trainer absent")
|
||||
# The patch reads the live Trainer.training_step source. We
|
||||
# exercise the standard transformers.Trainer here -- if the
|
||||
# bug is reintroduced in the source rewriter (e.g. broken
|
||||
# exec, missing import injection), the patch fn raises.
|
||||
try:
|
||||
patch_gradient_accumulation_fix(Trainer)
|
||||
except Exception as e:
|
||||
raise AssertionError(
|
||||
"patch_gradient_accumulation_fix raised on a vanilla "
|
||||
f"transformers.Trainer: {type(e).__name__}: {e}"
|
||||
)
|
||||
# Idempotency: second call must not raise either (the rewrite
|
||||
# adds `_unsloth_training_step` marker so the second call
|
||||
# short-circuits per _utils.py:1692-1693).
|
||||
patch_gradient_accumulation_fix(Trainer)
|
||||
|
||||
|
||||
# ---- unsloth/kernels/moe/grouped_gemm AST smoke ----
|
||||
|
||||
def _walk_py_files(root: pathlib.Path):
|
||||
for p in root.rglob("*.py"):
|
||||
if "__pycache__" in p.parts:
|
||||
continue
|
||||
yield p
|
||||
|
||||
|
||||
def test_unsloth_kernels_moe_grouped_gemm_ast_parses():
|
||||
"""unsloth/kernels/moe/grouped_gemm hosts the Triton MoE
|
||||
kernels (GPU-only at runtime). A SyntaxError or stray token
|
||||
at the SOURCE level still surfaces as ImportError on every
|
||||
install, so AST-parse the .py files without executing."""
|
||||
# Locate `unsloth/kernels/moe/grouped_gemm` via the installed
|
||||
# `unsloth` package.
|
||||
import unsloth as _unsloth
|
||||
kernel_root = (
|
||||
pathlib.Path(_unsloth.__file__).parent
|
||||
/ "kernels" / "moe" / "grouped_gemm"
|
||||
)
|
||||
if not kernel_root.exists():
|
||||
pytest.skip(
|
||||
f"{kernel_root} not present in this unsloth checkout."
|
||||
)
|
||||
fail = []
|
||||
ok = 0
|
||||
for p in _walk_py_files(kernel_root):
|
||||
try:
|
||||
ast.parse(p.read_text(encoding="utf-8"), filename=str(p))
|
||||
ok += 1
|
||||
except SyntaxError as e:
|
||||
fail.append((str(p), f"SyntaxError: {e}"))
|
||||
except Exception as e:
|
||||
fail.append((str(p), f"{type(e).__name__}: {e}"))
|
||||
print(f"AST-parsed {ok} grouped_gemm files; failed={len(fail)}")
|
||||
for path, err in fail:
|
||||
print(f" AST FAIL {path}: {err}")
|
||||
assert not fail, (
|
||||
f"AST parse failed for {len(fail)} grouped_gemm files"
|
||||
)
|
||||
# Sanity: the directory MUST contain at least the interface
|
||||
# + kernels + reference subtrees as documented.
|
||||
expected = [
|
||||
"interface.py",
|
||||
"kernels/forward.py",
|
||||
"kernels/backward.py",
|
||||
"reference/moe_block.py",
|
||||
"reference/moe_ops.py",
|
||||
]
|
||||
missing = [e for e in expected if not (kernel_root / e).is_file()]
|
||||
assert not missing, (
|
||||
"grouped_gemm directory layout regressed; missing: "
|
||||
f"{missing}"
|
||||
)
|
||||
PY
|
||||
python -m pytest -q --tb=short -s tests/_moe_coverage_shim.py
|
||||
rm -f tests/_moe_coverage_shim.py
|
||||
|
||||
- name: llama.cpp install via unsloth_zoo.llama_cpp + `llama-cli --help` smoke
|
||||
# Exercise the canonical `unsloth_zoo.llama_cpp.install_llama_cpp`
|
||||
# flow that GGUF export uses at runtime: clone ggml-org/llama.cpp
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue