Skip to content

feat: Discard weight when finish generation in the main loop#2495

Merged
terrykong merged 2 commits into
NVIDIA-NeMo:mainfrom
guyueh1:discard_vllm_weight_when_offloaded
May 21, 2026
Merged

feat: Discard weight when finish generation in the main loop#2495
terrykong merged 2 commits into
NVIDIA-NeMo:mainfrom
guyueh1:discard_vllm_weight_when_offloaded

Conversation

@guyueh1

@guyueh1 guyueh1 commented May 14, 2026

Copy link
Copy Markdown
Contributor

What does this PR do ?

A feature to reduce CPU memory footprint, which allows us to use pinned memory for opt offload in GB systems and helps perf then.

Currently the CPU memory peak happens at the end of finish_generation, which is inference weights + training weights + training optimizer; with this change we will reduce it to training weights + training optimizer.

Discard Weight in vLLM offload

With this change, I was able to use TP8 instead of TP16 for inference in DeepSeek V3 GRPO and got ~20% E2E speedup (>30% generation speedup)

Screenshot 2026-05-14 at 2 58 25 PM

Issues

List issues that this PR closes (syntax):

Usage

  • You can potentially add a usage example below
# Add a code snippet demonstrating how to use this

Before your PR is "Ready for review"

Pre checks:

  • Make sure you read and followed Contributor guidelines
  • Did you write any new necessary tests?
  • Did you run the unit tests and functional tests locally? Visit our Testing Guide for how to run tests
  • Did you add or update any necessary documentation? Visit our Document Development Guide for how to write, build and test the docs.

Additional Information

  • ...

@guyueh1
guyueh1 requested review from a team as code owners May 14, 2026 17:38
@copy-pr-bot

copy-pr-bot Bot commented May 14, 2026

Copy link
Copy Markdown

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

@guyueh1 guyueh1 added the CI:Lfast Runs a fast test suite and re-use nightly `main` container (but sync dependencies to PRs version) label May 14, 2026
@guyueh1

guyueh1 commented May 14, 2026

Copy link
Copy Markdown
Contributor Author

/ok to test 5efbbf4

@guyueh1

guyueh1 commented May 15, 2026

Copy link
Copy Markdown
Contributor Author

/ok to test 8253a2a

Signed-off-by: Guyue Huang <guyueh@login-lyris01.lyris.clusters.nvidia.com>
@guyueh1
guyueh1 force-pushed the discard_vllm_weight_when_offloaded branch from 8253a2a to 6760d81 Compare May 17, 2026 22:44
@guyueh1

guyueh1 commented May 17, 2026

Copy link
Copy Markdown
Contributor Author

/ok to test 6760d81

@guyueh1 guyueh1 removed the CI:Lfast Runs a fast test suite and re-use nightly `main` container (but sync dependencies to PRs version) label May 19, 2026
@guyueh1

guyueh1 commented May 19, 2026

Copy link
Copy Markdown
Contributor Author

/ok to test 6760d81

@guyueh1

guyueh1 commented May 19, 2026

Copy link
Copy Markdown
Contributor Author

/claude review

Signed-off-by: Guyue Huang <guyueh@nvidia.com>
@guyueh1

guyueh1 commented May 19, 2026

Copy link
Copy Markdown
Contributor Author

/ok to test 8d80498

@guyueh1

guyueh1 commented May 19, 2026

Copy link
Copy Markdown
Contributor Author

/claude review

@guyueh1

guyueh1 commented May 19, 2026

Copy link
Copy Markdown
Contributor Author

CI is green, @terrykong please take a look; @youngeunkwon0405 please approve again

@terrykong terrykong left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

thanks @guyueh1 . there's also this PR which we are currently reviewing #2523

seems to almost be the same thing with the exception of some small bits like the colocated check. shall we just use @Kipok 's PR? if so, could you apply the review over on his PR?

@guyueh1

guyueh1 commented May 20, 2026

Copy link
Copy Markdown
Contributor Author

thanks @guyueh1 . there's also this PR which we are currently reviewing #2523

seems to almost be the same thing with the exception of some small bits like the colocated check. shall we just use @Kipok 's PR? if so, could you apply the review over on his PR?

@terrykong I don't like the implementation in 2523 because it makes a knob visible to user but that knob involves system knowledge and should have been managed by the framework. Also 2523 is kind of wrong because it sets all the sleep to 2, but in grpo.py the calls of policy_generation.finish_generation() after the validation shouldn't discard the weights, since the refit happened before validation and won't happen again before the next generation. If we adopt 2523 it will crash cases that has validation.

@guyueh1
guyueh1 requested a review from terrykong May 21, 2026 16:50
@terrykong
terrykong merged commit 2a2e7c1 into NVIDIA-NeMo:main May 21, 2026
45 checks passed
yfw pushed a commit that referenced this pull request May 27, 2026
Signed-off-by: Guyue Huang <guyueh@login-lyris01.lyris.clusters.nvidia.com>
Signed-off-by: Guyue Huang <guyueh@nvidia.com>
Co-authored-by: Guyue Huang <guyueh@login-lyris01.lyris.clusters.nvidia.com>
terrykong added a commit that referenced this pull request Jun 1, 2026
pengdurice pushed a commit to pengdurice/RL that referenced this pull request Jun 12, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CI:L2 Run doctests, unit tests, functional tests, and convergence tests

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants