Commit b0f74ac
authored
fix(megatron): isolate Mamba state across packed sequences (#2056)
## Summary
Megatron Core derives PackedSeqParams.seq_idx only when total_tokens is
set. Mamba uses those labels to reset recurrent state between packed
documents; without them, state can flow across document boundaries in a
packed THD row.
Pass the global padded token count when constructing PackedSeqParams.
Using the global count keeps the labels aligned with cu_seqlens_q_padded
under context parallelism.
## Testing
- uv run --no-sync pytest -q
tests/backends/skyrl_train/distributed/test_preprocess_packed_seqs_multiseq.py::TestSubSeqLengths::test_multiseq_row_emits_padded_cu_seqlens_entries
tests/backends/skyrl_train/distributed/test_preprocess_packed_seqs_multiseq.py::TestMultiSeqCPLayout::test_roundtrip_recovers_full_layout_cp2
<!-- CURSOR_SUMMARY -->
---
> [!NOTE]
> **Medium Risk**
> Fixes correctness of Mamba state in packed training (wrong labels can
silently corrupt gradients); change is small and localized to packed-seq
metadata construction.
>
> **Overview**
> **Sets `total_tokens` on `PackedSeqParams`** when building packed THD
inputs in `preprocess_packed_seqs`, using the global padded token count
(`cu_seqlens_padded_cpu[-1]`). Megatron Core only derives per-token
`seq_idx` when `total_tokens` is present; Mamba layers use those labels
to reset recurrent state at document boundaries.
>
> Without this, **recurrent state can carry across packed
sub-sequences** in the same THD row. Using the **global** padded total
keeps `seq_idx` aligned with `cu_seqlens_q_padded` under context
parallelism.
>
> Tests extend megatron stubs and assert `total_tokens` matches the
padded cumulative length in multi-subseq and CP round-trip cases.
>
> <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit
6558f94. Bugbot is set up for automated
code reviews on this repo. Configure
[here](https://www.cursor.com/dashboard/bugbot).</sup>
<!-- /CURSOR_SUMMARY -->1 parent 6ecd004 commit b0f74ac
4 files changed
Lines changed: 8 additions & 0 deletions
File tree
- skyrl/backends/skyrl_train/distributed/megatron
- tests
- backends/skyrl_train/distributed
- train
Lines changed: 2 additions & 0 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
525 | 525 | | |
526 | 526 | | |
527 | 527 | | |
| 528 | + | |
528 | 529 | | |
529 | 530 | | |
530 | 531 | | |
| |||
533 | 534 | | |
534 | 535 | | |
535 | 536 | | |
| 537 | + | |
536 | 538 | | |
537 | 539 | | |
538 | 540 | | |
| |||
Lines changed: 1 addition & 0 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
34 | 34 | | |
35 | 35 | | |
36 | 36 | | |
| 37 | + | |
37 | 38 | | |
38 | 39 | | |
39 | 40 | | |
| |||
Lines changed: 4 additions & 0 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
28 | 28 | | |
29 | 29 | | |
30 | 30 | | |
| 31 | + | |
31 | 32 | | |
32 | 33 | | |
33 | 34 | | |
| |||
175 | 176 | | |
176 | 177 | | |
177 | 178 | | |
| 179 | + | |
178 | 180 | | |
179 | 181 | | |
180 | 182 | | |
| |||
345 | 347 | | |
346 | 348 | | |
347 | 349 | | |
| 350 | + | |
| 351 | + | |
348 | 352 | | |
349 | 353 | | |
350 | 354 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
48 | 48 | | |
49 | 49 | | |
50 | 50 | | |
| 51 | + | |
51 | 52 | | |
52 | 53 | | |
53 | 54 | | |
| |||
0 commit comments