Skip to content

Commit dca8f8f

Browse files
authored
[fix] Fix num_padding_microbatches metric for Megatron backend (#1856)
# What does this PR do? Fixes `num_padding_microbatches` metric calculation for the Megatron backend when there are real sequences with all zero loss masks (example with `apply_overlong_filtering=true`) The number of padding microbatches should not count all sequences with zero loss mask because some of these are real sequences but masked out for loss calculation. The fix is to read the `num_padding_microbatches` attribute from the `microbatch_iterator` when available. Signed-off-by: SumanthRH <sumanthrh@anyscale.com>
1 parent 06785af commit dca8f8f

1 file changed

Lines changed: 10 additions & 6 deletions

File tree

skyrl/backends/skyrl_train/workers/megatron/megatron_worker.py

Lines changed: 10 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1034,11 +1034,15 @@ def forward_backward(
10341034
)
10351035

10361036
# Count microbatches that carry real (non-padding) samples. Token-based batching
1037-
# appends fully-padding microbatches (loss_mask all zero) so every DP rank runs the
1038-
# same number of forward passes; those contribute 0 to KL/entropy and to mean metrics
1039-
# but would otherwise inflate the denominators. `num_real_microbatches` lets the loss
1040-
# normalize KL/entropy over real microbatches only.
1041-
num_real_microbatches = sum(1 for m in micro_buffer if m["loss_mask"].sum().item() > 0)
1037+
# appends fully-padding microbatches so every DP rank runs the same number of
1038+
# forward passes; those contribute 0 to KL/entropy and to mean metrics but would
1039+
# otherwise inflate the denominators. A real microbatch can still have an all-zero
1040+
# loss_mask (for example, DAPO overlong filtering), so use the iterator's padding
1041+
# count rather than inferring from loss_mask.
1042+
num_padding_microbatches = (
1043+
getattr(microbatch_iterator, "num_padding_microbatches", 0) if microbatch_iterator is not None else 0
1044+
)
1045+
num_real_microbatches = len(micro_buffer) - num_padding_microbatches
10421046
for m_batch in micro_buffer:
10431047
m_batch["num_microbatches"] = len(micro_buffer)
10441048
m_batch["num_real_microbatches"] = num_real_microbatches
@@ -1118,7 +1122,7 @@ def forward_backward(
11181122
# identical on every rank; num_padding_microbatches reports the per-rank average).
11191123
if use_token_batching:
11201124
status["num_microbatches"] = float(len(micro_buffer))
1121-
status["num_padding_microbatches"] = float(len(micro_buffer) - num_real_microbatches)
1125+
status["num_padding_microbatches"] = float(num_padding_microbatches)
11221126

11231127
group = mpu.get_data_parallel_group(with_context_parallel=False)
11241128
status = all_reduce_metrics(status, self.strategy, group=group, sum_loss_metrics=sum_loss_metrics)

0 commit comments

Comments
 (0)