Skip to content

fix(metrics): align packed sequence accuracy with ring padding - #10182

Open
Excelius-Wang wants to merge 1 commit into
modelscope:mainfrom
Excelius-Wang:fix/ring-seq-accuracy
Open

Excelius-Wang wants to merge 1 commit into
modelscope:mainfrom
Excelius-Wang:fix/ring-seq-accuracy

Conversation

@Excelius-Wang

Copy link
Copy Markdown
Contributor

Ring sequence parallelism gathers predictions and shifted labels in a per-sequence padded layout. _compute_acc still groups them with the original unpadded cu_seqlens, so sequence accuracy can attribute errors to the wrong sample or miss them entirely. For example, one correct and one incorrect packed sequence can be reported as 100% sequence accuracy.

Derive the metric boundaries from the already-padded position IDs using get_cu_seqlens_from_position_ids. This keeps the boundaries aligned with gathered tensors without changing communication, labels, or token accuracy.

Validation:

  • CPU Gloo tests call the actual trainer metric method across Ring, Ring + Ulysses, Ulysses-only, and disabled-SP configurations. They compare both sequence and token accuracy with the unsharded reference across four layouts, including aligned, single-sequence, and fully masked one-token samples. Check all-correct predictions and errors at every supervised token (152 metric comparisons), using the trainer boundary helper and the normal token-metric call shape.
  • Before: both Ring configurations fail; Ulysses and no-SP controls pass. After: all configurations pass.
  • Related tests: 6 passed.
  • pre-commit run --all-files: passed.

No full GPU training run was performed; this fixes metric computation, not training loss.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant