8000 fix: Do not initialize reference model for sft by ashors1 · Pull Request #71 · NVIDIA/NeMo-RL · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

fix: Do not initialize reference model for sft #71

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 12 commits into from
Apr 1, 2025
Merged

fix: Do not initialize reference model for sft #71

merged 12 commits into from
Apr 1, 2025

Conversation

ashors1
Copy link
Collaborator
@ashors1 ashors1 commented Mar 24, 2025

What does this PR do ?

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

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

  • ...

@ashors1
do not initialze reference model for sft
455f46c 
Signed-off-by: ashors1 <ashors@nvidia.com>
@ashors1 ashors1 changed the title Do not initialize reference model for sft fix: Do not initialize reference model for sft Mar 24, 2025
parthchadha
parthchadha previously approved these changes Mar 24, 2025
View reviewed changes
@ashors1
fixes
990f8a1 
Signed-off-by: ashors1 <ashors@nvidia.com>
terrykong
terrykong previously approved these changes Mar 26, 2025
View reviewed changes
@terrykong terrykong added the Run CICD Set to run CI (unset + set to rerun) label Mar 26, 2025
SahilJain314
SahilJain314 reviewed Mar 26, 2025
View reviewed changes
Copy link
Collaborator
@SahilJain314 SahilJain314 left a comment

Choose a reason for hiding this comment

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

Can you modify SFT tests to use init_reference_model=False?

@ashors1
Copy link
Collaborator Author
ashors1 commented Mar 27, 2025

Can you modify SFT tests to use init_reference_model=False?

Do you mean adding a new test? The SFT functional test will of course already use init_reference_model=False since we just run the loop in sft.py

@ashors1
update test
6364d77 
Signed-off-by: ashors1 <ashors@nvidia.com>
@ashors1 ashors1 added Run CICD Set to run CI (unset + set to rerun) and removed Run CICD Set to run CI (unset + set to rerun) labels Mar 31, 2025
@ashors1
update pure generation test
897e602 
Signed-off-by: ashors1 <ashors@nvidia.com>
@ashors1 ashors1 added Run CICD Set to run CI (unset + set to rerun) and removed Run CICD Set to run CI (unset + set to rerun) labels Mar 31, 2025
@ashors1 ashors1 added Run CICD Set to run CI (unset + set to rerun) and removed Run CICD Set to run CI (unset + set to rerun) labels Mar 31, 2025
@ashors1
fix
45363fb 
Signed-off-by: ashors1 <ashors@nvidia.com>
@ashors1 ashors1 added Run CICD Set to run CI (unset + set to rerun) and removed Run CICD Set to run CI (unset + set to rerun) labels Apr 1, 2025
@ashors1
fix
f018f83 
Signed-off-by: ashors1 <ashors@nvidia.com>
@ashors1 ashors1 added Run CICD Set to run CI (unset + set to rerun) and removed Run CICD Set to run CI (unset + set to rerun) labels Apr 1, 2025
ashors1 added 2 commits April 1, 2025 08:51
@ashors1
Merge branch 'main' of github.com:NVIDIA/reinforcer into ashors/sft-r…
51266a2 
…emove-ref-model
@ashors1
fix tests
6182ec6 
Signed-off-by: Anna Shors <ashors@nvidia.com>
@ashors1 ashors1 removed the Run CICD Set to run CI (unset + set to rerun) label Apr 1, 2025
@ashors1 ashors1 added Run CICD Set to run CI (unset + set to rerun) and removed Run CICD Set to run CI (unset + set to rerun) labels Apr 1, 2025
@ashors1
linting
af85ca9 
Signed-off-by: ashors1 <ashors@nvidia.com>
@ashors1 ashors1 added Run CICD Set to run CI (unset + set to rerun) and removed Run CICD Set to run CI (unset + set to rerun) labels Apr 1, 2025
terrykong
terrykong reviewed Apr 1, 2025
View reviewed changes
@ashors1 ashors1 added Run CICD Set to run CI (unset + set to rerun) and removed Run CICD Set to run CI (unset + set to rerun) labels Apr 1, 2025
@ashors1
use indirect for fixtures with args
122f37a 
Signed-off-by: ashors1 <ashors@nvidia.com>
@ashors1 ashors1 force-pushed the ashors/sft-remove-ref-model branch from 522633f to 122f37a Compare April 1, 2025 16:55
@ashors1 ashors1 added Run CICD Set to run CI (unset + set to rerun) and removed Run CICD Set to run CI (unset + set to rerun) labels Apr 1, 2025
SahilJain314
SahilJain314 approved these changes Apr 1, 2025
View reviewed changes
Copy link
Collaborator
@SahilJain314 SahilJain314 left a comment

Choose a reason for hiding this comment

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

Tests look good, thanks

@terrykong terrykong added Run CICD Set to run CI (unset + set to rerun) and removed Run CICD Set to run CI (unset + set to rerun) labels Apr 1, 2025
@terrykong terrykong enabled auto-merge (squash) April 1, 2025 19:25
@terrykong terrykong merged commit 640297c into main Apr 1, 2025
11 checks passed
@terrykong terrykong deleted the ashors/sft-remove-ref-model branch April 1, 2025 19:44
yfw pushed a commit that referenced this pull request Apr 2, 2025
@ashors1 @terrykong @yfw
fix: Do not initialize reference model for sft (#71)
< 9010 a href="/NVIDIA/NeMo-RL/commit/f87474667e140f653435d66620aac37312e3b517" class="Link--secondary">f874746 
Signed-off-by: ashors1 <ashors@nvidia.com>
Signed-off-by: Anna Shors <ashors@nvidia.com>
Co-authored-by: Terry Kong <terrycurtiskong@gmail.com>
KiddoZhu pushed a commit that referenced this pull request May 6, 2025
@ashors1 @terrykong
fix: Do not initialize reference model for sft (#71)
ef7b819 
Signed-off-by: ashors1 <ashors@nvidia.com>
Signed-off-by: Anna Shors <ashors@nvidia.com>
Co-authored-by: Terry Kong <terrycurtiskong@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@terrykong terrykong terrykong approved these changes

@parthchadha parthchadha parthchadha approved these changes

@SahilJain314 SahilJain314 SahilJain314 approved these changes

Assignees
No one assigned
Labels
Run CICD Set to run CI (unset + set to rerun)
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@ashors1 @terrykong @parthchadha @SahilJain314
0