Skip to content

fix: Tie weights after set_model_state_dict if required #666

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 4 commits into from
Jul 15, 2025
Merged

fix: Tie weights after set_model_state_dict if required #666

merged 4 commits into from
Jul 15, 2025

Conversation

@parthchadha
Copy link
Contributor

@parthchadha parthchadha commented Jul 14, 2025 •
edited
Loading

What does this PR do ?

Add a one line overview of what this PR aims to accomplish.

Screenshot 2025-07-14 at 3 49 37 PM Screenshot 2025-07-14 at 3 50 13 PM

Issues

Closes #227.

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

  • ...

@parthchadha parthchadha requested a review from terrykong July 15, 2025 15:47
SahilJain314
SahilJain314 previously approved these changes Jul 15, 2025
View reviewed changes
@parthchadha parthchadha added this pull request to the merge queue Jul 15, 2025
terrykong
terrykong previously approved these changes Jul 15, 2025
View reviewed changes
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Jul 15, 2025
@parthchadha
Fix failing unit test
a372d92 
Signed-off-by: Parth Chadha <[email protected]>
@parthchadha parthchadha dismissed stale reviews from terrykong and SahilJain314 via a372d92 July 15, 2025 19:13
@parthchadha parthchadha added the CI:L0 Run doctests and unit tests label Jul 15, 2025
@parthchadha parthchadha added this pull request to the merge queue Jul 15, 2025
@terrykong terrykong added the r0.3.0 Release r0.3.0 label Jul 15, 2025
Merged via the queue into main with commit 755109d Jul 15, 2025
30 of 33 checks passed
@parthchadha parthchadha deleted the pchadha/fix-tied-weights branch July 15, 2025 22:58
ZhiyuLi-Nvidia pushed a commit that referenced this pull request Jul 21, 2025
@parthchadha @ZhiyuLi-Nvidia
fix: Tie weights after set_model_state_dict if required (#666)
5c5892f 
Signed-off-by: Parth Chadha <[email protected]>
Signed-off-by: Zhiyu Li <[email protected]>
jialei777 pushed a commit to jialei777/nemo-rl that referenced this pull request Jul 23, 2025
@parthchadha @jialei777
fix: Tie weights after set_model_state_dict if required (NVIDIA-NeMo#666
171ef50 
)

Signed-off-by: Parth Chadha <[email protected]>
Signed-off-by: Jialei Chen <[email protected]>
KiddoZhu pushed a commit that referenced this pull request Jul 28, 2025
@parthchadha @KiddoZhu
fix: Tie weights after set_model_state_dict if required (#666)
51276de 
Signed-off-by: Parth Chadha <[email protected]>
xxman-google pushed a commit to xxman-google/NeMo-RL that referenced this pull request Jul 30, 2025
@parthchadha @xxman-google
fix: Tie weights after set_model_state_dict if required (NVIDIA-NeMo#666
519e031 
)

Signed-off-by: Parth Chadha <[email protected]>
@yfw yfw mentioned this pull request Jul 31, 2025
Closed
@parthchadha parthchadha self-assigned this Aug 1, 2025
FannYYW pushed a commit to xxman-google/NeMo-RL that referenced this pull request Aug 5, 2025
@parthchadha
fix: Tie weights after set_model_state_dict if required (NVIDIA-NeMo#666
356435f 
)

Signed-off-by: Parth Chadha <[email protected]>
FannYYW pushed a commit to xxman-google/NeMo-RL that referenced this pull request Aug 5, 2025
@parthchadha @xxman-google
fix: Tie weights after set_model_state_dict if required (NVIDIA-NeMo#666
360eafa 
)

Signed-off-by: Parth Chadha <[email protected]>
soodoshll pushed a commit to soodoshll/RL that referenced this pull request Aug 13, 2025
@parthchadha @soodoshll
fix: Tie weights after set_model_state_dict if required (NVIDIA-NeMo#666
08c02e5 
)

Signed-off-by: Parth Chadha <[email protected]>
Signed-off-by: Qidong Su <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@terrykong terrykong terrykong left review comments

+1 more reviewer

@SahilJain314 SahilJain314 SahilJain314 approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@parthchadha parthchadha

Labels

CI:L0 Run doctests and unit tests r0.3.0 Release r0.3.0

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Tied weights are not respected in fsdp1 and fsp2+tp>1

4 participants

@parthchadha @terrykong @SahilJain314