Skip to content

Track and plot per-token off-policy in RL - #3515

Merged
tdene merged 17 commits into
NVIDIA:mainfrom
tdene:tde/rl_set_offpolicy
Feb 26, 2026
Merged

tdene merged 17 commits into
NVIDIA:mainfrom
tdene:tde/rl_set_offpolicy

Conversation

@tdene

@tdene tdene commented Feb 20, 2026

Copy link
Copy Markdown
Contributor

What does this PR do ?

policy_staleness describes how many checkpoints off-policy a request's tokens are. kv_cache_staleness describes how many checkpoints off-policy a request's KV$ is. policy_staleness >= kv_cache_staleness. Both are tracked on a per-token basis.

When not using recompute, the two metrics are expected to overlap - with one exception. If the KV cache buffer is full and needs to evict paused requests to make room, this will null kv_cache_staleness while not affecting policy_staleness. num_evictions is a per-request metric that tracks such "natural" prefill recompute.

Request staleness is tracked inside the engine. Once a request finishes, it leaves the engine. Since a request may not be consumed until much later than it finishes, RL has to manually increment staleness. This is represented by completion_gap and is graphed as mean_completion_gap.

Contribution process

flowchart LR
    A[Pre-checks] --> B[PR Tests]
    subgraph Code Review/Approval
        C1[Expert Review] --> C2[Final Review]
    end
    B --> C1
    C2 --> D[Merge]
Loading

Pre-checks

  • I want this PR in a versioned release and have added the appropriate Milestone (e.g., Core 0.8)
  • I have added relevant unit tests
  • I have added relevant functional tests
  • I have added proper typing to my code Typing guidelines
  • I have added relevant documentation
  • I have run the autoformatter.sh on my PR

Code review

The following process is enforced via the CODEOWNERS file for changes into megatron/core. For changes outside of megatron/core, it is up to the PR author whether or not to tag the Final Reviewer team.

For MRs into `main` branch

Feel free to message or comment the @mcore-oncall to help accelerate your merge into main. The less complex your PR is, the faster it will be approved and merged!

(Step 1): Add PR label Expert Review

(Step 2): Collect the expert reviewers reviews

  1. Attach the Expert Review label when your PR is ready for review.
  2. GitHub auto-assigns expert reviewers based on your changes. They will get notified and pick up your PR soon.

⚠️ Only proceed to the next step once all reviewers have approved, merge-conflict are resolved and the CI is passing.
Final Review might get declined if these requirements are not fulfilled.

(Step 3): Final Review

  1. Add Final Review label
  2. GitHub auto-assigns final reviewers based on your changes. They will get notified and pick up your PR soon.

(Optional Step 4): Cherry-pick into release branch

If this PR also needs to be merged into core_r* release branches, after this PR has been merged, select Cherry-pick to open a new PR into the release branch.

For MRs into `dev` branch The proposed review process for `dev` branch is under active discussion.

MRs are mergable after one approval by either eharper@nvidia.com or zijiey@nvidia.com.

Merging your PR

Any member of core-adlr and core-nemo will be able to merge your PR.

Comment thread megatron/rl/inference/megatron.py Outdated
Comment thread megatron/rl/inference/megatron.py Outdated
Comment thread megatron/rl/inference/megatron.py Outdated
Comment thread megatron/rl/inference/megatron.py Outdated
Comment thread megatron/rl/server/api.py Outdated
Comment thread megatron/rl/rl_utils.py Outdated
Comment thread megatron/rl/rl_utils.py Outdated
Comment thread megatron/rl/rl_utils.py Outdated
for group in rollouts:
for rollout in group:
gap = args.curr_iteration - rollout.completed_at_step
if gap > 0:

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What if gap is 0?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Then we're doing a list comprehension with [x+0 for x in l], and that seemed uglier to me.

Comment thread megatron/rl/agent/api.py Outdated
Comment thread megatron/rl/rl_utils.py Outdated
Comment thread megatron/rl/rl_utils.py Outdated
Comment thread megatron/rl/rl_utils.py
Comment thread megatron/rl/rl_utils.py Outdated
Comment thread megatron/rl/agent/reward_only_agent.py

@yobibyte yobibyte left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! Thanks for addressing my comments.

@tdene
tdene added this pull request to the merge queue Feb 26, 2026
@svcnvidia-nemo-ci

Copy link
Copy Markdown
Contributor

🔄 Merge queue validation started!

You can track the progress here: https://github.com/NVIDIA/Megatron-LM/actions/runs/22455951465

@svcnvidia-nemo-ci

Copy link
Copy Markdown
Contributor

🔄 Merge queue validation started!

You can track the progress here: https://github.com/NVIDIA/Megatron-LM/actions/runs/22456323984

Merged via the queue into NVIDIA:main with commit 6c0b9c6 Feb 26, 2026
55 of 57 checks passed
@tdene
tdene deleted the tde/rl_set_offpolicy branch February 26, 2026 20:58
BoxiangW pushed a commit to BoxiangW/Megatron-LM that referenced this pull request Mar 4, 2026
yangbofun pushed a commit to xlm-research/Megatron-LM that referenced this pull request May 22, 2026
terminator123 pushed a commit to 021ai/Megatron-LM that referenced this pull request Aug 3, 2026

This branch was previously deployed

1 inactive deployment
test — 88e2bd5f Deployed Feb 26, 2026 by copy-pr-bot[bot] via cicd-wait-in-queue #8763
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

complexity: medium Expert Review [deprecated] Apply this label to indicate that your PR is ready for expert review.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants