Skip to content

Commit cecb2b4

Browse files
HJSangclaude
andcommitted
Give stock HF attention the boundaries HF derives: each pad token on its own
real_max_seqlen dropped the pad segment from max_length while cu_seqlens still carried it as one segment of up to 127 tokens; FlashAttention leaves rows past max_length unwritten, so pad rows could get uninitialized outputs and gradients. hf_attention_packing_kwargs now passes the documents, then each pad token as a 1-token segment, exactly what HF's position_ids path derives, so the longest real document covers every segment. It is built on the device from host lengths, without a sync. Qwen3.5 keeps get_batch's boundaries and max_seqlen as on main. Reported by claude[bot] on #4316. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
1 parent 6441c3a commit cecb2b4

7 files changed

Lines changed: 92 additions & 33 deletions

File tree

‎miles/backends/fsdp_utils/actor.py‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,6 @@
2626
from miles.utils.workers.rpc.common.wire_types import Pickled
2727

2828
from .adaptations.config_checks import validate_hf_config
29-
from .adaptations.packing import real_max_seqlen
3029
from .adaptations.precision import apply_fp32_master, default_precision_policy, precision_forward_context
3130
from .adaptations.specs import resolve_arch_adapter
3231
from .hf_weight_iterator import FSDPHfWeightIterator
@@ -430,7 +429,8 @@ def _get_model_inputs_args(self, batch: dict) -> dict:
430429
self.arch_adapter.packing_kwargs(
431430
cu_seqlens=batch["cu_seqlens"],
432431
cu_seqlens_host=batch["cu_seqlens_host"],
433-
max_seqlen=real_max_seqlen(batch["cu_seqlens_host"], len(batch["unconcat_tokens"])),
432+
max_seqlen=batch["max_seqlen"],
433+
num_documents=len(batch["unconcat_tokens"]),
434434
)
435435
)
436436

‎miles/backends/fsdp_utils/adaptations/arch_adapter.py‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -41,13 +41,15 @@ def patch_model(self, model: nn.Module, args: Namespace) -> None:
4141
and the ref model, so it must be idempotent."""
4242

4343
def packing_kwargs(
44-
self, *, cu_seqlens: torch.Tensor, cu_seqlens_host: tuple[int, ...], max_seqlen: int
44+
self, *, cu_seqlens: torch.Tensor, cu_seqlens_host: tuple[int, ...], max_seqlen: int, num_documents: int
4545
) -> dict[str, object]:
4646
"""Extra forward kwargs that tell this architecture's modeling where the packed documents that
4747
``get_batch`` recorded start."""
4848
if not self.passes_attention_packing_kwargs:
4949
return {}
50-
return hf_attention_packing_kwargs(cu_seqlens=cu_seqlens, max_seqlen=max_seqlen)
50+
return hf_attention_packing_kwargs(
51+
cu_seqlens=cu_seqlens, cu_seqlens_host=cu_seqlens_host, num_documents=num_documents
52+
)
5153

5254
def param_transform(self, name: str, param: torch.Tensor) -> ParamExpand | None:
5355
"""How to rewrite ``param`` for the rollout engine at weight sync, or None to stream it unchanged."""

‎miles/backends/fsdp_utils/adaptations/packing.py‎

Lines changed: 21 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -41,20 +41,25 @@ def packed_seq_context(position_ids: torch.Tensor | None) -> PackedSeqContext |
4141
HF_PACKING_KWARG_NAMES = frozenset({"cu_seq_lens_q", "cu_seq_lens_k", "max_length_q", "max_length_k", "seq_idx"})
4242

4343

44-
def real_max_seqlen(cu_seqlens_host: tuple[int, ...], num_documents: int) -> int:
45-
"""The longest real document; ``get_batch``'s ``max_seqlen`` also counts the trailing pad segment, which
46-
HF never does (each pad token is its own document there) and which changes the kernel's launch plan."""
47-
real = cu_seqlens_host[: num_documents + 1]
48-
return max(end - start for start, end in itertools.pairwise(real))
49-
44+
def hf_attention_packing_kwargs(
45+
*, cu_seqlens: torch.Tensor, cu_seqlens_host: tuple[int, ...], num_documents: int
46+
) -> dict[str, object]:
47+
"""The boundaries HF derives from ``position_ids``: the documents, then each pad token on its own.
5048
51-
def hf_attention_packing_kwargs(*, cu_seqlens: torch.Tensor, max_seqlen: int) -> dict[str, object]:
52-
"""The varlen boundaries HF attention reads; ``max_length`` stays an int, which FlashAttention-4 needs."""
49+
``max_length`` is an int, since FlashAttention-4 rejects HF's tensor, and it covers every segment, since
50+
the kernel leaves rows past it unwritten. Keeping the pad tail as one segment would raise it, and the
51+
kernel's launch plan with it, away from what HF (and true-on-policy's bitwise match) uses.
52+
"""
53+
pad = cu_seqlens_host[-1] - cu_seqlens_host[num_documents]
54+
# on the device from host lengths, so building it never syncs
55+
pad_ends = cu_seqlens[num_documents] + torch.arange(1, pad + 1, dtype=cu_seqlens.dtype, device=cu_seqlens.device)
56+
boundaries = torch.cat([cu_seqlens[: num_documents + 1], pad_ends])
57+
max_length = max(end - start for start, end in itertools.pairwise(cu_seqlens_host[: num_documents + 1]))
5358
return {
54-
"cu_seq_lens_q": cu_seqlens,
55-
"cu_seq_lens_k": cu_seqlens,
56-
"max_length_q": max_seqlen,
57-
"max_length_k": max_seqlen,
59+
"cu_seq_lens_q": boundaries,
60+
"cu_seq_lens_k": boundaries,
61+
"max_length_q": max_length,
62+
"max_length_k": max_length,
5863
}
5964

6065

@@ -73,7 +78,10 @@ def hf_packing_kwargs(
7378
output_size=cu_seqlens_host[-1],
7479
)
7580
return {
76-
**hf_attention_packing_kwargs(cu_seqlens=cu_seqlens, max_seqlen=max_seqlen),
81+
"cu_seq_lens_q": cu_seqlens,
82+
"cu_seq_lens_k": cu_seqlens,
83+
"max_length_q": max_seqlen,
84+
"max_length_k": max_seqlen,
7785
"seq_idx": seq_idx.unsqueeze(0),
7886
}
7987

‎miles/backends/fsdp_utils/adaptations/specs/qwen3_5.py‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@
1010
class Qwen35Adapter(ArchAdapter):
1111
model_types = frozenset({"qwen3_5", "qwen3_5_text", "qwen3_next"})
1212

13-
def packing_kwargs(self, *, cu_seqlens, cu_seqlens_host, max_seqlen):
13+
def packing_kwargs(self, *, cu_seqlens, cu_seqlens_host, max_seqlen, num_documents):
1414
# GatedDeltaNet reads seq_idx (causal conv) and cu_seq_lens_q (FLA chunk rule) from **kwargs.
1515
return hf_packing_kwargs(cu_seqlens=cu_seqlens, cu_seqlens_host=cu_seqlens_host, max_seqlen=max_seqlen)
1616

‎tests/fast-gpu/test_fsdp_gated_deltanet_packing.py‎

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -298,6 +298,24 @@ def test_building_packing_kwargs_never_syncs_the_device():
298298
assert inputs["cu_seq_lens_q"] is batch["cu_seqlens"] and inputs["max_length_q"] == 129
299299

300300

301+
def test_building_attention_boundaries_never_syncs_the_device():
302+
"""Stock HF attention gets the pad tail as 1-token segments, built on the device from host lengths."""
303+
actor = object.__new__(FSDPTrainRayActor)
304+
actor.arch_adapter = resolve_arch_adapter(SimpleNamespace(model_type="qwen3"))
305+
docs = [_random_tokens(length, seed) for seed, length in enumerate((37, 64, 129))]
306+
batch = _packed_batch(docs, padded_length=256)
307+
308+
previous_mode = torch.cuda.get_sync_debug_mode()
309+
torch.cuda.set_sync_debug_mode("error")
310+
try:
311+
inputs = actor._get_model_inputs_args(batch)
312+
finally:
313+
torch.cuda.set_sync_debug_mode(previous_mode)
314+
315+
assert inputs["cu_seq_lens_q"].tolist() == [0, 37, 101, 230, *range(231, 257)]
316+
assert inputs["max_length_q"] == 129
317+
318+
301319
if __name__ == "__main__":
302320
import sys
303321

‎tests/fast/backends/fsdp_utils/test_actor.py‎

Lines changed: 23 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -93,22 +93,40 @@ def test_packing_kwargs_follow_the_batch_boundaries_so_padding_stays_one_segment
9393
"""Pad tokens carry position 0; boundaries derived from `position_ids` would make each one a document."""
9494
actor = object.__new__(actor_module.FSDPTrainRayActor)
9595
actor.arch_adapter = resolve_arch_adapter(SimpleNamespace(model_type="qwen3_5_text"))
96-
cu_seqlens_host = (0, 3, 5, 9) # two documents, then the 4-token pad segment get_batch appends
96+
cu_seqlens_host = (0, 3, 5, 8) # two documents, then the 3-token pad segment get_batch appends
97+
batch = {
98+
"tokens": torch.zeros(1, 8, dtype=torch.long),
99+
"position_ids": torch.tensor([[0, 1, 2, 0, 1, 0, 0, 0]]),
100+
"cu_seqlens": torch.tensor(cu_seqlens_host, dtype=torch.int32),
101+
"cu_seqlens_host": cu_seqlens_host,
102+
"max_seqlen": 3,
103+
"unconcat_tokens": [torch.zeros(3, dtype=torch.long), torch.zeros(2, dtype=torch.long)],
104+
}
105+
106+
model_args = actor._get_model_inputs_args(batch)
107+
108+
assert model_args["cu_seq_lens_q"] is batch["cu_seqlens"]
109+
assert model_args["max_length_q"] == 3
110+
assert model_args["seq_idx"].tolist() == [[0, 0, 0, 1, 1, 2, 2, 2]]
111+
112+
def test_stock_hf_attention_gets_every_pad_token_as_its_own_segment(self) -> None:
113+
"""A 4-token pad tail kept as one segment would exceed max_length 3, leaving its rows unwritten."""
114+
actor = object.__new__(actor_module.FSDPTrainRayActor)
115+
actor.arch_adapter = resolve_arch_adapter(SimpleNamespace(model_type="qwen3"))
116+
cu_seqlens_host = (0, 3, 5, 9)
97117
batch = {
98118
"tokens": torch.zeros(1, 9, dtype=torch.long),
99119
"position_ids": torch.tensor([[0, 1, 2, 0, 1, 0, 0, 0, 0]]),
100120
"cu_seqlens": torch.tensor(cu_seqlens_host, dtype=torch.int32),
101121
"cu_seqlens_host": cu_seqlens_host,
102-
"max_seqlen": 4, # get_batch's, which counts the pad segment
122+
"max_seqlen": 4,
103123
"unconcat_tokens": [torch.zeros(3, dtype=torch.long), torch.zeros(2, dtype=torch.long)],
104124
}
105125

106126
model_args = actor._get_model_inputs_args(batch)
107127

108-
assert model_args["cu_seq_lens_q"] is batch["cu_seqlens"]
109-
# the longest real document: the pad segment changes FlashAttention's launch plan
128+
assert model_args["cu_seq_lens_q"].tolist() == [0, 3, 5, 6, 7, 8, 9]
110129
assert model_args["max_length_q"] == 3
111-
assert model_args["seq_idx"].tolist() == [[0, 0, 0, 1, 1, 2, 2, 2, 2]]
112130

113131

114132
class TestFSDPTrainParallelConfigWiring:

‎tests/fast/backends/test_fsdp_hf_compat.py‎

Lines changed: 23 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -26,13 +26,14 @@ def _adapter(model_type):
2626
return resolve_arch_adapter(SimpleNamespace(model_type=model_type))
2727

2828

29-
def _packed_boundaries(*lengths):
30-
"""The boundary fields `get_batch` records for documents of these lengths."""
31-
cu_seqlens_host = tuple(itertools.accumulate(lengths, initial=0))
29+
def _packed_boundaries(*lengths, pad=0):
30+
"""The boundary fields `get_batch` records for documents of these lengths, then its pad segment."""
31+
cu_seqlens_host = tuple(itertools.accumulate([*lengths, *([pad] if pad else [])], initial=0))
3232
return dict(
3333
cu_seqlens=torch.tensor(cu_seqlens_host, dtype=torch.int32),
3434
cu_seqlens_host=cu_seqlens_host,
35-
max_seqlen=max(lengths),
35+
max_seqlen=max(lengths + (pad,)),
36+
num_documents=len(lengths),
3637
)
3738

3839

@@ -338,7 +339,9 @@ def test_hf_packing_kwargs_match_the_padding_free_collator():
338339

339340
collator = DataCollatorWithFlattening(return_tensors="pt", return_flash_attn_kwargs=True, return_seq_idx=True)
340341
flattened = collator([{"input_ids": [1, 2, 3]}, {"input_ids": [4, 5]}, {"input_ids": [6, 7, 8, 9]}])
341-
kwargs = hf_packing_kwargs(**_packed_boundaries(3, 2, 4))
342+
boundaries = _packed_boundaries(3, 2, 4)
343+
del boundaries["num_documents"]
344+
kwargs = hf_packing_kwargs(**boundaries)
342345

343346
assert set(kwargs) == HF_PACKING_KWARG_NAMES
344347
for name, value in kwargs.items():
@@ -349,12 +352,22 @@ def test_hf_packing_kwargs_match_the_padding_free_collator():
349352
assert value == expected, name
350353

351354

352-
def test_real_max_seqlen_leaves_out_the_pad_segment():
353-
"""A 7-token pad tail must not become the max: HF never counts it, and the kernel plan follows it."""
354-
from miles.backends.fsdp_utils.adaptations.packing import real_max_seqlen
355+
@pytest.mark.parametrize("pad", [0, 1, 7])
356+
def test_attention_boundaries_are_the_ones_hf_derives_from_position_ids(pad):
357+
"""True-on-policy's bitwise match relies on HF's own boundaries; a max_length below a segment's length
358+
would leave its rows unwritten, as a 7-token pad tail kept as one segment would."""
359+
from transformers.modeling_flash_attention_utils import prepare_fa_kwargs_from_position_ids
355360

356-
assert real_max_seqlen((0, 3, 5, 12), num_documents=2) == 3
357-
assert real_max_seqlen((0, 3, 5), num_documents=2) == 3 # no pad segment
361+
from miles.backends.fsdp_utils.adaptations.packing import hf_attention_packing_kwargs
362+
363+
boundaries = _packed_boundaries(3, 2, pad=pad)
364+
del boundaries["max_seqlen"]
365+
kwargs = hf_attention_packing_kwargs(**boundaries)
366+
position_ids = torch.tensor([[0, 1, 2, 0, 1, *([0] * pad)]])
367+
(hf_cu_seqlens, _), (hf_max_length, _) = prepare_fa_kwargs_from_position_ids(position_ids)
368+
369+
assert kwargs["cu_seq_lens_q"].tolist() == hf_cu_seqlens.tolist()
370+
assert type(kwargs["max_length_q"]) is int and kwargs["max_length_q"] == int(hf_max_length) == 3
358371

359372

360373
def test_each_arch_passes_the_packing_kwargs_its_modeling_reads():

0 commit comments

Comments
 (0)