[logging] Publish rollout buffer depth and group keep-rate metrics - #1924
Merged
pcmoritz merged 12 commits intoJul 24, 2026
Conversation
Adds skyrl/train/utils/phase_metrics.py with TrainingPhaseGauge, which sets a skyrl_training_phase gauge to 1.0 for the active macro-phase and 0.0 for the rest via ray.util.metrics. Ray exports it to the same Prometheus that scrapes node GPU metrics, so phase windows join GPU utilization on one wall-clock axis without correlating the experiment tracker by hand. The async loop sets the phase at each existing Timer boundary. Best-effort: the gauge silently no-ops when Ray metrics are unavailable, so it never breaks training or tests.
Generators produce rollout batches into a buffer and the trainer pulls mini_batch_size batches per step. The buffer depth tells which side is the bottleneck. Near zero means the trainer is starving for rollouts; near the cap means generation is paused at the staleness limit. It was previously visible only in a transient tqdm postfix. Log async/gen_buffer_qsize and async/gen_buffer_maxsize to the tracker each step, and publish the same values plus skyrl_mini_batch_size and skyrl_gen_group_keep_rate as Prometheus gauges via a new ScalarGauges helper (best-effort, no-ops without Ray, like TrainingPhaseGauge). keep_rate is kept / (kept + dropped) for the last mini-batch, the zero-variance drop rate under sample_full_batch, so a deep buffer of mostly droppable groups is distinguishable from a usable one.
- Emit only the two changed series per transition instead of sweeping all phases; the full sweep runs once at construction to seed every series for PromQL selectors. - Warn and keep the current phase on an unknown phase name instead of silently zeroing everything. - Rename phases to match their paired Timer keys so wandb timing/* keys and the Prometheus phase label share one vocabulary. - Construct the gauge in __init__ behind trainer.enable_training_phase_gauge, matching the RayGpuMonitor lifecycle. - Deduplicate docstrings and comments; add unknown-phase and disabled-by-flag tests.
Merges the updated seiji/training-phase-gauge base (gauge in __init__ behind a config flag, Timer-key phase names, two-write emit) and applies this PR's own review findings: - ScalarGauges takes a description at the first set() for a name instead of a constructor descriptions dict. - ScalarGauges construction and the loop-invariant skyrl_mini_batch_size set move to __init__; the step loop reads qsize/maxsize once into locals. - keep_rate uses mini_batch_size directly (the collect loop returns exactly that many kept groups here) and is also logged to the tracker as async/keep_rate, so both stores carry both drop signals. - Comment trims.
The gauge writes two in-process values per phase transition with no thread or I/O, so there is nothing an operator needs to switch off. Construct it unconditionally; it already no-ops without Ray.
Base dropped the enable_training_phase_gauge toggle; the gauge constructs unconditionally.
ScalarGauges had no functional dependency on TrainingPhaseGauge; the stack existed only because both classes shared phase_metrics.py. Move ScalarGauges to its own module (scalar_gauges.py, one module per concern) and revert the phase-gauge content from this branch's tree, so this PR is independently mergeable and its diff is buffer-only. Forward revert, no history rewrite; NovaSky-AI#1923 still owns the phase gauge.
The gauge description strings already say what each metric is; drop the comments restating them and the dashboard interpretation. Keep the one non-obvious invariant (collect returns exactly mini_batch_size kept groups) and the set() description semantics.
eicherseiji
marked this pull request as ready for review
July 18, 2026 00:57
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Code Review
This pull request introduces a new ScalarGauges utility class to publish best-effort scalar metrics to Prometheus via ray.util.metrics, and integrates it into the FullyAsyncTrainer to track metrics like mini-batch size, generation buffer queue size, buffer capacity, and generation group keep rate. Unit tests were also added to verify the behavior of ScalarGauges. Feedback was provided to separate the float conversion from the Ray-specific metric operations to prevent a single malformed metric value from permanently disabling all Prometheus metrics.
SumanthRH
reviewed
Jul 19, 2026
SumanthRH
reviewed
Jul 19, 2026
…#1930 Review feedback from SumanthRH. The buffer capacity is a run constant: compute it once in __init__ as the single source for both the queue construction and a one-time skyrl_gen_buffer_maxsize publish, kept for dashboard panels. Drop the per-step W&B keys: NovaSky-AI#1930 already logs async/gen_buffer_qsize_at_wait_start, and this PR keeps only the step-start Prometheus gauge for the depth.
pcmoritz
reviewed
Jul 22, 2026
Publish gauges directly so a metrics error surfaces instead of silently latching gauge publishing off. Call sites pass ints.
Collaborator
|
Let's merge #1923 and then also put this into |
eicherseiji
changed the title
Keep both the phase gauge (NovaSky-AI#1923) and the buffer/keep-rate gauges in fully_async_trainer.py. Move ScalarGauges into metrics.py alongside TrainingPhaseGauge and drop the standalone scalar_gauges.py.
pcmoritz
approved these changes
Jul 24, 2026
eicherseiji
added a commit
to eicherseiji/SkyRL
that referenced
this pull request
Jul 24, 2026
Import ScalarGauges from metrics.py and drop the standalone scalar_gauges.py (now folded into metrics.py). Take main's buffer/keep-rate gauges in fully_async_trainer.py and keep only this PR's step/epoch marking hooks. Move the mirror's disable-on-failure guard into Tracking._publish_gauge, since metrics.py's ScalarGauges is intentionally unguarded.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What does this PR do?
Logs the rollout buffer state to Prometheus. Generation and training run concurrently: generators fill a buffer and the trainer pulls
mini_batch_sizebatches per step. The buffer depth tells which side is the bottleneck. Near zero means the trainer is starving for rollouts; near the cap means generation is paused at the staleness limit. Same idle-GPU symptom, opposite cause.New metrics:
skyrl_gen_buffer_qsize(Prometheus, per step): buffer depth at step start. The W&B counterpart is [chore] Log generation output buffer size at the start of a training step #1930'sasync/gen_buffer_qsize_at_wait_start.skyrl_gen_buffer_maxsize,skyrl_mini_batch_size(Prometheus, once at startup): run constants for dashboard panels, e.g. a fill ratio or capacity line.async/keep_rate(W&B) andskyrl_gen_group_keep_rate(Prometheus):mini_batch_size / (mini_batch_size + dropped)undersample_full_batch, the zero-variance drop rate for the last mini-batch, so a deep buffer of mostly droppable groups is distinguishable from a usable one.How
ScalarGauges(inskyrl/train/utils/metrics.py, alongside the phase gauge from #1923) lazily creates one Prometheus gauge per name at the firstset(). The trainer publishes step-start values directly because the tracker flushes at step end, after training and weight sync, when the buffer has already moved on.Tests
tests/train/utils/test_metrics.py(extended): gauge created once per name with its first description, values coerced to float.