Guard RoPE scaling against the transformers v5 buffer blank; honor extended RoPE factor (#6925)
* Guard RoPE scaling against the transformers v5 buffer blank; honor extended factor Add a family-agnostic guard that builds each rotary from a scaled config, blanks its non-persistent buffers (what transformers v5 does on load), runs loader._fix_rope_inv_freq, and asserts every buffer is restored to its scaled value (llama3 and longrope). This catches the whole bug class, not just the one call site, and is validated to fail on the pre-fix repair. Also make LlamaExtendedRotaryEmbedding read the llama3 factor from the config instead of hardcoding 8 (wrong for Llama-3.2, factor 32), falling back to the Llama-3.1 defaults when built without a config. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Pass config into extended rotary codegen; skip v5 round-trip on transformers 4.x - patch_llama_rope_scaling now builds the llama3 extended rotary with config=self.config so it reads the real factor (32 for Llama-3.2) instead of falling back to 8; the template already references self.config. - test_v5_blank_repair_roundtrip now skips when loader._NEEDS_ROPE_FIX is False, since _fix_rope_inv_freq is a no-op on transformers 4.x and cannot restore the blanked buffers there. * Raise stream deadlock-guard timeouts from 0.2s to 5.0s in passthrough tests These asyncio.wait_for guards bound test setup and cross-task event signaling that complete near-instantly on success; the 0.2s budget is a latency assertion in disguise and times out under CI scheduling load (seen on the 3.11 matrix leg while 3.10/3.12/3.13 pass the same commit). 5.0s matches the timeout used elsewhere in the suite and still fails fast on a real hang. No test relies on the guard expiring. * Extended rotary reads rope_parameters as well as rope_scaling transformers v5 stores llama3 scaling under config.rope_parameters and exposes rope_scaling only as a back-compat property. Reading that property works on 5.0-5.13 (verified: factor resolves to 32 for Llama-3.2), but a future release may drop the shim, after which the subclass path would fall back to factor 8. Read either field so the factor survives the rename. Adds test_extended_rotary_reads_rope_parameters_v5 (fails on the old single-field read: rope_parameters-only config resolves to 8, not 32). * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --------- Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
This commit is contained in:
parent
296cacb5a1
commit
bdb958e052
4 changed files with 160 additions and 14 deletions
|
|
@ -1900,7 +1900,7 @@ class TestApiMonitorProviderAndCompletionStreams:
|
|||
"chatcmpl-test",
|
||||
monitor_id = monitor_id,
|
||||
),
|
||||
timeout = 0.2,
|
||||
timeout = 5.0,
|
||||
)
|
||||
assert isinstance(response, _SameTaskStreamingResponse)
|
||||
|
||||
|
|
@ -2044,7 +2044,7 @@ class TestApiMonitorProviderAndCompletionStreams:
|
|||
"chatcmpl-test",
|
||||
monitor_id = monitor_id,
|
||||
),
|
||||
timeout = 0.2,
|
||||
timeout = 5.0,
|
||||
)
|
||||
assert isinstance(response, _SameTaskStreamingResponse)
|
||||
gate.set()
|
||||
|
|
@ -2107,7 +2107,7 @@ class TestApiMonitorProviderAndCompletionStreams:
|
|||
"chatcmpl-test",
|
||||
monitor_id = monitor_id,
|
||||
),
|
||||
timeout = 0.2,
|
||||
timeout = 5.0,
|
||||
)
|
||||
assert isinstance(response, _SameTaskStreamingResponse)
|
||||
|
||||
|
|
@ -2190,7 +2190,7 @@ class TestApiMonitorProviderAndCompletionStreams:
|
|||
"chatcmpl-test",
|
||||
monitor_id = monitor_id,
|
||||
),
|
||||
timeout = 0.2,
|
||||
timeout = 5.0,
|
||||
)
|
||||
assert isinstance(response, _SameTaskStreamingResponse)
|
||||
|
||||
|
|
@ -2252,7 +2252,7 @@ class TestApiMonitorProviderAndCompletionStreams:
|
|||
"chatcmpl-test",
|
||||
monitor_id = monitor_id,
|
||||
),
|
||||
timeout = 0.2,
|
||||
timeout = 5.0,
|
||||
)
|
||||
assert isinstance(response, _SameTaskStreamingResponse)
|
||||
assert cancel_id in inf_mod._CANCEL_REGISTRY
|
||||
|
|
@ -2323,13 +2323,13 @@ class TestApiMonitorProviderAndCompletionStreams:
|
|||
monitor_id = monitor_id,
|
||||
)
|
||||
)
|
||||
await asyncio.wait_for(entered.wait(), timeout = 0.2)
|
||||
await asyncio.wait_for(entered.wait(), timeout = 5.0)
|
||||
assert cancel_id in inf_mod._CANCEL_REGISTRY
|
||||
|
||||
task.cancel()
|
||||
with pytest.raises(asyncio.CancelledError):
|
||||
await task
|
||||
await asyncio.wait_for(cancelled.wait(), timeout = 0.2)
|
||||
await asyncio.wait_for(cancelled.wait(), timeout = 5.0)
|
||||
assert cancel_id not in inf_mod._CANCEL_REGISTRY
|
||||
|
||||
asyncio.run(_run())
|
||||
|
|
@ -2389,13 +2389,13 @@ class TestApiMonitorProviderAndCompletionStreams:
|
|||
"chatcmpl-test",
|
||||
monitor_id = monitor_id,
|
||||
),
|
||||
timeout = 0.2,
|
||||
timeout = 5.0,
|
||||
)
|
||||
assert isinstance(response, _SameTaskStreamingResponse)
|
||||
assert cancel_id in inf_mod._CANCEL_REGISTRY
|
||||
|
||||
gate.set()
|
||||
await asyncio.wait_for(returned.wait(), timeout = 0.2)
|
||||
await asyncio.wait_for(returned.wait(), timeout = 5.0)
|
||||
await asyncio.sleep(0)
|
||||
await response._unstarted_cleanup()
|
||||
assert upstream_response.is_closed
|
||||
|
|
|
|||
|
|
@ -257,6 +257,63 @@ def test_recompute_helper_scales_on_cpu():
|
|||
), "_unsloth_recompute_inv_freq must return vanilla inv_freq when unscaled."
|
||||
|
||||
|
||||
def test_extended_rotary_reads_config_factor():
|
||||
# LlamaExtendedRotaryEmbedding must honor the config factor, not hardcode 8
|
||||
# (Llama-3.2 uses 32); otherwise the subclass path re-drops scaling (#2405).
|
||||
from types import SimpleNamespace
|
||||
|
||||
from unsloth.models.llama import LlamaExtendedRotaryEmbedding
|
||||
|
||||
rot = object.__new__(LlamaExtendedRotaryEmbedding)
|
||||
rot.base = ROPE_THETA
|
||||
rot.dim = HEAD_DIM
|
||||
rot._unsloth_rope_config = SimpleNamespace(
|
||||
rope_scaling = {
|
||||
"rope_type": "llama3",
|
||||
"factor": 32.0,
|
||||
"low_freq_factor": 1.0,
|
||||
"high_freq_factor": 4.0,
|
||||
"original_max_position_embeddings": 8192,
|
||||
}
|
||||
)
|
||||
vanilla = _vanilla_inv_freq()
|
||||
scaled = rot._apply_inv_freq_scaling(vanilla).reshape(-1)
|
||||
ratio = float(vanilla[-1]) / float(scaled[-1])
|
||||
assert abs(ratio - 32.0) < 1e-3, (
|
||||
f"LlamaExtendedRotaryEmbedding ignored config factor 32 (ratio {ratio}); the "
|
||||
"low-frequency band must be divided by the config factor (issue #2405)."
|
||||
)
|
||||
|
||||
|
||||
def test_extended_rotary_reads_rope_parameters_v5():
|
||||
# transformers v5 stores scaling under rope_parameters (rope_scaling is a
|
||||
# back-compat shim that may be removed); the factor must still be read.
|
||||
from types import SimpleNamespace
|
||||
|
||||
from unsloth.models.llama import LlamaExtendedRotaryEmbedding
|
||||
|
||||
rot = object.__new__(LlamaExtendedRotaryEmbedding)
|
||||
rot.base = ROPE_THETA
|
||||
rot.dim = HEAD_DIM
|
||||
rot._unsloth_rope_config = SimpleNamespace(
|
||||
rope_scaling = None,
|
||||
rope_parameters = {
|
||||
"rope_type": "llama3",
|
||||
"factor": 32.0,
|
||||
"low_freq_factor": 1.0,
|
||||
"high_freq_factor": 4.0,
|
||||
"original_max_position_embeddings": 8192,
|
||||
},
|
||||
)
|
||||
vanilla = _vanilla_inv_freq()
|
||||
scaled = rot._apply_inv_freq_scaling(vanilla).reshape(-1)
|
||||
ratio = float(vanilla[-1]) / float(scaled[-1])
|
||||
assert abs(ratio - 32.0) < 1e-3, (
|
||||
f"Extended rotary ignored rope_parameters factor 32 (ratio {ratio}); v5 "
|
||||
"keeps the factor under rope_parameters, not rope_scaling."
|
||||
)
|
||||
|
||||
|
||||
def _cos_at_position(rot, position):
|
||||
"""cos row at one position, built like _set_cos_sin_cache but CPU-only."""
|
||||
inv_freq = rot.inv_freq.float().cpu()
|
||||
|
|
@ -324,6 +381,87 @@ def test_extended_cache_keeps_scaling_after_growth():
|
|||
)
|
||||
|
||||
|
||||
def _blank_nonpersistent_buffers(module):
|
||||
"""Mimic transformers v5 meta-load: overwrite non-persistent buffers with garbage."""
|
||||
for name, buf in list(module.named_buffers()):
|
||||
leaf = module
|
||||
*parents, attr = name.split(".")
|
||||
for part in parents:
|
||||
leaf = getattr(leaf, part)
|
||||
if attr in getattr(leaf, "_non_persistent_buffers_set", set()):
|
||||
setattr(leaf, attr, torch.rand_like(buf))
|
||||
|
||||
|
||||
def _build_llama3_rotary():
|
||||
from unsloth.models import llama as llama_mod
|
||||
config = _make_config(LLAMA3_ROPE_SCALING)
|
||||
return llama_mod.LlamaRotaryEmbedding(config = config), config
|
||||
|
||||
|
||||
def _build_longrope_rotary():
|
||||
from types import SimpleNamespace
|
||||
|
||||
from unsloth.models import llama as llama_mod
|
||||
|
||||
short_factor, long_factor = [1.05] * 48, [1.3] * 48
|
||||
rot = llama_mod.LongRopeRotaryEmbedding(
|
||||
dim = 96,
|
||||
max_position_embeddings = 131072,
|
||||
original_max_position_embeddings = 4096,
|
||||
base = ROPE_THETA,
|
||||
short_factor = short_factor,
|
||||
long_factor = long_factor,
|
||||
)
|
||||
config = SimpleNamespace(
|
||||
rope_scaling = {
|
||||
"rope_type": "longrope",
|
||||
"short_factor": short_factor,
|
||||
"long_factor": long_factor,
|
||||
"original_max_position_embeddings": 4096,
|
||||
}
|
||||
)
|
||||
return rot, config
|
||||
|
||||
|
||||
@requires_cuda
|
||||
@pytest.mark.parametrize(
|
||||
"build", [_build_llama3_rotary, _build_longrope_rotary], ids = ["llama3", "longrope"]
|
||||
)
|
||||
def test_v5_blank_repair_roundtrip(build):
|
||||
# Build scaled -> blank non-persistent buffers (what transformers v5 does on
|
||||
# load) -> run the repair -> every buffer must return to its scaled value.
|
||||
# Family-agnostic: encodes no scaling math, so it guards any rotary that
|
||||
# keeps scaling in a buffer (issue #2405 / PR #6907).
|
||||
from unsloth.models import loader
|
||||
|
||||
# The repair only runs on transformers v5 (it is what blanks the buffers);
|
||||
# on v4 _fix_rope_inv_freq is a no-op, so the round-trip cannot restore.
|
||||
if not loader._NEEDS_ROPE_FIX:
|
||||
pytest.skip("transformers < 5 does not blank rope buffers; repair is a no-op")
|
||||
|
||||
rot, config = build()
|
||||
snapshot = {name: buf.detach().clone() for name, buf in rot.named_buffers()}
|
||||
assert snapshot, "rotary registers no buffers; nothing to guard"
|
||||
|
||||
_blank_nonpersistent_buffers(rot)
|
||||
assert any(
|
||||
not torch.equal(rot.get_buffer(name), snapshot[name]) for name in snapshot
|
||||
), "blanking changed no buffer; the round-trip would be vacuous"
|
||||
|
||||
wrapper = torch.nn.Module()
|
||||
wrapper.add_module("rotary_emb", rot)
|
||||
wrapper.config = config
|
||||
loader._fix_rope_inv_freq(wrapper)
|
||||
|
||||
for name in snapshot:
|
||||
assert torch.allclose(
|
||||
rot.get_buffer(name).cpu(), snapshot[name].cpu(), rtol = 1e-4, atol = 1e-6
|
||||
), (
|
||||
f"{name} was not restored to its scaled value by loader._fix_rope_inv_freq "
|
||||
"after the transformers v5 buffer blank (issue #2405 / PR #6907)."
|
||||
)
|
||||
|
||||
|
||||
def test_object_style_rope_scaling_does_not_crash():
|
||||
# Object-style rope_scaling must be normalized, not .get()'d directly.
|
||||
from dataclasses import dataclass
|
||||
|
|
|
|||
|
|
@ -2834,6 +2834,7 @@ def patch_llama_rope_scaling(
|
|||
dim = self.head_dim,
|
||||
max_position_embeddings=self.max_position_embeddings,
|
||||
base=self.rope_theta,
|
||||
config=self.config,
|
||||
)
|
||||
elif scaling_type == "longrope":
|
||||
self.rotary_emb = {longrope_rope_function}(
|
||||
|
|
|
|||
|
|
@ -1930,11 +1930,18 @@ class LlamaExtendedRotaryEmbedding(LlamaRotaryEmbedding):
|
|||
|
||||
# From https://github.com/meta-llama/llama-models/blob/main/models/llama3_1/api/model.py#L41
|
||||
def _apply_inv_freq_scaling(self, freqs: torch.Tensor):
|
||||
# Values obtained from grid search
|
||||
scale_factor = 8
|
||||
low_freq_factor = 1
|
||||
high_freq_factor = 4
|
||||
old_context_len = 8192 # original llama3 length
|
||||
# llama3 factors from config; Llama-3.1 defaults when built without one
|
||||
# (legacy codegen path). Hardcoding 8 is wrong for e.g. Llama-3.2 (32).
|
||||
# v5 renames rope_scaling -> rope_parameters; read either so the factor
|
||||
# survives even if the rope_scaling back-compat shim is dropped.
|
||||
config = getattr(self, "_unsloth_rope_config", None)
|
||||
rope_scaling = _rope_scaling_as_dict(
|
||||
getattr(config, "rope_scaling", None) or getattr(config, "rope_parameters", None) or {}
|
||||
)
|
||||
scale_factor = rope_scaling.get("factor", 8)
|
||||
low_freq_factor = rope_scaling.get("low_freq_factor", 1)
|
||||
high_freq_factor = rope_scaling.get("high_freq_factor", 4)
|
||||
old_context_len = rope_scaling.get("original_max_position_embeddings", 8192)
|
||||
|
||||
low_freq_wavelen = old_context_len / low_freq_factor
|
||||
high_freq_wavelen = old_context_len / high_freq_factor
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue