Skip to content

Commit 05763b9

Browse files
HJSangclaude
andcommitted
Give attention the longest real document as max_length
get_batch's max_seqlen also counts the trailing pad segment, which HF's own position_ids path never does, and FlashAttention picks its launch plan from max_seqlen. batch["max_seqlen"] itself is unchanged (Megatron reads it). Found by an adversarial review of this PR. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
1 parent d93a05c commit 05763b9

5 files changed

Lines changed: 20 additions & 1 deletion

File tree

‎miles/backends/fsdp_utils/actor.py‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@
2222
from miles.utils.workers.rpc.common.wire_types import Pickled
2323

2424
from .adaptations.config_checks import validate_hf_config
25+
from .adaptations.packing import real_max_seqlen
2526
from .adaptations.precision import apply_fp32_master, default_precision_policy, precision_forward_context
2627
from .adaptations.specs import resolve_arch_adapter
2728
from .hf_weight_iterator import FSDPHfWeightIterator
@@ -406,7 +407,7 @@ def _get_model_inputs_args(self, batch: dict) -> dict:
406407
self.arch_adapter.packing_kwargs(
407408
cu_seqlens=batch["cu_seqlens"],
408409
cu_seqlens_host=batch["cu_seqlens_host"],
409-
max_seqlen=batch["max_seqlen"],
410+
max_seqlen=real_max_seqlen(batch["cu_seqlens_host"], len(batch["unconcat_tokens"])),
410411
attn_implementation=self.args.attn_implementation,
411412
)
412413
)

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

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
"""
99

1010
import functools
11+
import itertools
1112
from dataclasses import dataclass
1213

1314
import torch
@@ -40,6 +41,13 @@ def packed_seq_context(position_ids: torch.Tensor | None) -> PackedSeqContext |
4041
HF_PACKING_KWARG_NAMES = frozenset({"cu_seq_lens_q", "cu_seq_lens_k", "max_length_q", "max_length_k", "seq_idx"})
4142

4243

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+
50+
4351
def hf_attention_packing_kwargs(*, cu_seqlens: torch.Tensor, max_seqlen: int) -> dict[str, object]:
4452
"""The varlen boundaries HF attention reads; ``max_length`` stays an int, which FlashAttention-4 needs."""
4553
return {

‎tests/fast-gpu/test_fsdp_gated_deltanet_packing.py‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -116,6 +116,7 @@ def _packed_batch(docs, padded_length):
116116
cu_seqlens=torch.tensor(cu_seqlens_host, dtype=torch.int32, device="cuda"),
117117
cu_seqlens_host=cu_seqlens_host,
118118
max_seqlen=max(end - start for start, end in itertools.pairwise(cu_seqlens_host)),
119+
unconcat_tokens=docs,
119120
)
120121

121122

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -99,6 +99,7 @@ def test_packing_kwargs_follow_the_batch_boundaries_so_padding_stays_one_segment
9999
"cu_seqlens": torch.tensor(cu_seqlens_host, dtype=torch.int32),
100100
"cu_seqlens_host": cu_seqlens_host,
101101
"max_seqlen": 3,
102+
"unconcat_tokens": [torch.zeros(3, dtype=torch.long), torch.zeros(2, dtype=torch.long)],
102103
}
103104

104105
model_args = actor._get_model_inputs_args(batch)

‎tests/fast/backends/test_fsdp_hf_compat.py‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -349,6 +349,14 @@ def test_hf_packing_kwargs_match_the_padding_free_collator():
349349
assert value == expected, name
350350

351351

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+
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
358+
359+
352360
def test_each_arch_passes_the_packing_kwargs_its_modeling_reads():
353361
boundaries = _packed_boundaries(2, 3)
354362
for model_type in ("qwen3_5", "qwen3_5_text", "qwen3_5_moe", "qwen3_5_moe_text", "qwen3_next"):

0 commit comments

Comments
 (0)