Skip to content

Commit 5e7ff4e

Browse files
eaglstunclaude
andcommitted
Retire the lion weight-decay strict-xfail now that the divergence is fixed
test_lion_weight_decay_backend_divergence was a strict xfail pinning a real cross-backend bug: the `default` optimizer kernel (used on mps) applied COUPLED weight decay for Lion while the cpu/CUDA kernels applied DECOUPLED decay. That bug was fixed upstream (bitsandbytes-foundation#1992 / bitsandbytes-foundation#1993) -- the default backend now excludes LION from the coupled fold and shrinks the param directly -- so mps and the cpu oracle agree and the strict xfail was XPASSing (counting as a suite failure). - Remove the xfail; rename to test_lion_weight_decay_decoupled_parity and document it as the regression guard for the fix. - Update the stale "backends disagree" note in test_optimizer_update_32bit. - MPS_STATUS.md: mark the §5 finding resolved, fix the §2 matrix row, update the harness header (329 passed, 0 xfailed). Full parity suite: 329 passed, 0 xfailed under BNB_MPS_REQUIRE_NATIVE=1 (native lib built in this checkout). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent ed2ea2e commit 5e7ff4e

2 files changed

Lines changed: 28 additions & 25 deletions

File tree

docs/apple_silicon/MPS_STATUS.md

Lines changed: 15 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -7,9 +7,10 @@
77
**Harness:** `tests/test_mps_parity.py` — Phase-1 baseline (no native build): **183 passed, 1 xfailed
88
(strict), 0 skipped**. Phase-3 source build (`-DCOMPUTE_BACKEND=mps`, `BNB_MPS_REQUIRE_NATIVE=1`):
99
**293 passed, 1 xfailed**, with `quantize_blockwise`, `dequantize_blockwise`, `dequantize_4bit`, and
10-
`quantize_4bit` all running through hand-written Metal and bit-exact vs the CPU oracle. Phase-M4
11-
build: **328 passed** plus the known, unrelated lion strict-xfail XPASS (a separate optimizer track;
12-
see §5), with the 4-bit matmuls native per §11.
10+
`quantize_4bit` all running through hand-written Metal and bit-exact vs the CPU oracle. Post-M4
11+
build (`-DCOMPUTE_BACKEND=mps`, `BNB_MPS_REQUIRE_NATIVE=1`): **329 passed, 0 xfailed** — the lion
12+
weight-decay divergence that was a strict `xfail` is now fixed upstream and is a passing regression
13+
test (§5), and the 4-bit matmuls are native per §11.
1314

1415
This is the Phase-1 deliverable (§3 audit) plus the Phase-2 result (first native kernel end to end).
1516
Re-verify against `bitsandbytes/_ops.py` and `bitsandbytes/backends/mps/ops.py` before trusting this
@@ -76,7 +77,7 @@ Parity = max deviation vs the CPU oracle with seeded inputs (see §3 for toleran
7677
| `int8_scaled_mm` | ❌ → `default` | composition of the above | exact |
7778
| `int8_mixed_scaled_mm` | ❌ → `default` | composition of the above | covered via components |
7879
| `int8_double_quant` | ❌ (cuda only, no default) | **`NotImplementedError`** | — (gap; also unavailable on cpu) |
79-
| `optimizer_update_32bit` | ❌ → `default` | pure-torch on MPS | exact, **except lion + weight_decay (§5)** |
80+
| `optimizer_update_32bit` | ❌ → `default` | pure-torch on MPS | exact (incl. lion + weight_decay since #1992; §5) |
8081
| `optimizer_update_8bit_blockwise` | ❌ (cpu/cuda/xpu, no default) | **`NotImplementedError`** | — (gap: 8-bit optimizers unusable on mps) |
8182

8283
**Round-trip reconstruction** (quantize→dequantize on MPS vs same on CPU, seeded randn,
@@ -143,16 +144,16 @@ Verified against the plan's §1 claims, at `777c145`:
143144

144145
## 5. Findings / divergences
145146

146-
1. **Lion weight-decay semantics differ between backends** (caught by the harness;
147-
encoded as a strict `xfail`, `test_lion_weight_decay_backend_divergence`):
148-
- `default` kernel (used on **mps**): **coupled** decay `g += p * weight_decay`
149-
(`backends/default/ops.py`, LION included in `optimizer_id in [0, 1, 2, 4]`).
150-
- `cpu` kernel (`backends/cpu/ops.py::_optimizer_update_32bit_cpu`) and the **CUDA
151-
kernel** (`csrc/kernels.cu`, `case LION: p_vals[j] *= (1.0f - lr*weight_decay)`):
152-
**decoupled** decay, matching the Lion paper.
153-
- The `default` backend is the outlier ⇒ candidate upstream bug affecting every
154-
device that relies on the default optimizer path (mps included). Out of Phase-1
155-
scope to fix; flagged for Eric.
147+
1. **Lion weight-decay semantics — RESOLVED (was a cross-backend divergence).** The
148+
harness caught this in Phase 1 (originally a strict `xfail`): the `default` kernel
149+
used on **mps** applied **coupled** decay for lion (`g += p * weight_decay`, LION
150+
wrongly included in the coupled group), while the `cpu` and CUDA kernels applied
151+
**decoupled** decay (`p *= 1 - lr*weight_decay`) per the Lion paper — the `default`
152+
backend was the outlier, a real upstream bug affecting every device on the default
153+
optimizer path (mps included). **Fixed upstream (#1992 / #1993):** the default
154+
backend now excludes LION from the coupled fold and applies decoupled decay, so mps
155+
and the cpu oracle agree. The former xfail is now the passing regression test
156+
`test_lion_weight_decay_decoupled_parity`.
156157
2. **8-bit optimizers are unusable on mps**`optimizer_update_8bit_blockwise` has no
157158
mps/default registration and raises `NotImplementedError`.
158159
3. **`int8_double_quant` is CUDA-only** — raises on mps _and_ on cpu.

tests/test_mps_parity.py

Lines changed: 13 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -929,9 +929,9 @@ class TestOptimizerParity:
929929
def test_optimizer_update_32bit(self, optimizer_name):
930930
# On mps this resolves to the "default" (pure-torch) kernel; the cpu oracle
931931
# runs the dedicated "cpu" kernel from backends/cpu/ops.py.
932-
# NOTE: lion is tested with weight_decay=0.0 because the two backends
933-
# disagree on lion weight-decay semantics -- see
934-
# test_lion_weight_decay_backend_divergence below.
932+
# lion + weight_decay>0 parity is covered separately by
933+
# test_lion_weight_decay_decoupled_parity below; keep wd=0 here to avoid
934+
# redundant coverage.
935935
weight_decay = 0.0 if optimizer_name == "lion" else 0.01
936936
(p, state1, state2), (p_mps, state1_mps, state2_mps) = _run_optimizer_32bit_both_devices(
937937
optimizer_name, weight_decay
@@ -942,14 +942,16 @@ def test_optimizer_update_32bit(self, optimizer_name):
942942
if state2 is not None:
943943
assert_parity(state2_mps, state2, torch.float32)
944944

945-
@pytest.mark.xfail(
946-
strict=True,
947-
reason="Known cross-backend divergence: the 'default' kernel (used on mps) applies COUPLED "
948-
"weight decay for lion (g += p*wd, backends/default/ops.py optimizer_id in [0,1,2,4]), while "
949-
"the 'cpu' kernel and the CUDA kernel apply DECOUPLED weight decay (p *= 1 - lr*wd), matching "
950-
"the Lion paper. The default backend is the outlier. See docs/apple_silicon/MPS_STATUS.md.",
951-
)
952-
def test_lion_weight_decay_backend_divergence(self):
945+
def test_lion_weight_decay_decoupled_parity(self):
946+
"""Regression: lion + weight_decay agrees across backends on mps.
947+
948+
This was previously a strict-xfail documenting a real divergence -- the
949+
'default' kernel (used on mps) applied COUPLED weight decay for lion
950+
(g += p*wd), while the 'cpu' and CUDA kernels applied DECOUPLED decay
951+
(p *= 1 - lr*wd), matching the Lion paper. The default backend was the
952+
outlier; it was fixed to be decoupled (#1992 / #1993), so mps and the cpu
953+
oracle now match. See docs/apple_silicon/MPS_STATUS.md.
954+
"""
953955
(p, state1, _), (p_mps, state1_mps, _) = _run_optimizer_32bit_both_devices("lion", weight_decay=0.01)
954956

955957
assert_parity(p_mps, p, torch.float32)

0 commit comments

Comments
 (0)