Skip to content

refactor: clean up code #262

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
Open
wants to merge 41 commits into
base: main
Choose a base branch
from
Open

refactor: clean up code #262

wants to merge 41 commits into from

Conversation

ashors1
Copy link
Collaborator

@ashors1 ashors1 commented Apr 24, 2025 •
edited
Loading

What does this PR do ?

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

Issues

closes #228

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 and others added 19 commits April 22, 2025 17:31
@ashors1
[WIP] refactor to reduce duplicate code
16dc401 
Signed-off-by: ashors1 <ashors@nvidia.com>
@ashors1
small fixes
347ad33 
Signed-off-by: ashors1 <ashors@nvidia.com>
@ashors1
remove unused imports
194d058 
Signed-off-by: ashors1 <ashors@nvidia.com>
@ashors1
avoid passing logger to validation fn
56dcbfa 
Signed-off-by: ashors1 <ashors@nvidia.com>
@ashors1
remove empty folder
a49513d 
Signed-off-by: ashors1 <ashors@nvidia.com>
@ashors1
remove unneeded utils
694442a 
Signed-off-by: ashors1 <ashors@nvidia.com>
@ashors1
fixes
d52e8f7 
Signed-off-by: ashors1 <ashors@nvidia.com>
@ashors1
Merge branch 'main' of github.com:NVIDIA/reinforcer into ashors/cleanup
5fced97
@yuki-666 @zpqiu
@parthchadha @ashors1
feat: streaming each dtensor in refit (#176)
f479c17 
Signed-off-by: Yuki Huang <yukih@nvidia.com>
Signed-off-by: Alex Qiu <alexq@nvidia.com>
Signed-off-by: Parth Chadha <pchadha@nvidia.com>
Co-authored-by: Alex Qiu <alexq@nvidia.com>
Co-authored-by: Parth Chadha <pchadha@nvidia.com>
@ashors1
fix: Fix indent in dtensor policy (#248)
87c828e 
Signed-off-by: ashors1 <ashors@nvidia.com>
@parthchadha @SahilJain314
@terrykong @yfw @mckimn @ashors1
fix: raise error if tied weights model is being trained with fsdp1 or… (
97b58aa 
#229)

Signed-off-by: Parth Chadha <pchadha@nvidia.com>
Signed-off-by: Sahil Jain <sahilj@nvidia.com>
Signed-off-by: Terry Kong <terryk@nvidia.com>
Signed-off-by: Yi-Fu Wu <yifu.wu@gmail.com>
Signed-off-by: Nathan McKimpson <nmckimpson@nvidia.com>
Co-authored-by: Sahil Jain <48468750+SahilJain314@users.noreply.github.com>
Co-authored-by: Terry Kong <terrycurtiskong@gmail.com>
Co-authored-by: Yi-Fu Wu <yifu.wu@gmail.com>
Co-authored-by: mckimn <nmckimpson@nvidia.com>
@parthchadha @ashors1
fix: use find_tied_parameters api from HF for tied weight keys (#250)
ffe4550 
Signed-off-by: Parth Chadha <pchadha@nvidia.com>
@terrykong @ashors1
ci: L1 default and increase test time (#252)
8316579 
Signed-off-by: Terry Kong <terryk@nvidia.com>
@parthchadha @ashors1
fix: fix broken eval script (#253)
035bdc0 
Signed-off-by: Parth Chadha <pchadha@nvidia.com>
@terrykong @ashors1
docs: add qwen 32b instruction and add 0.3 planned features (#255)
456157f 
Signed-off-by: Terry Kong <terryk@nvidia.com>
@ashors1
fix: Fix fsdp1 grad clipping and log grad norm (#251)
c12759b 
Signed-off-by: ashors1 <ashors@nvidia.com>
@ashors1
fixes
8504283 
Signed-off-by: ashors1 <ashors@nvidia.com>
@ashors1
Merge branch 'ashors/cleanup' of github.com:NVIDIA/reinforcer into as…
f36f9c4 
…hors/cleanup
@ashors1
remove duplicate code in loss functions, handle multi-epoch step coun…
c5750ac 
…ting better, always save a checkpoint at the end of training

Signed-off-by: ashors1 <ashors@nvidia.com>
@ashors1
small fixes
5c04c2d 
Signed-off-by: ashors1 <ashors@nvidia.com>
@ashors1 ashors1 changed the title refactor: clean up algorithm code refactor: clean up code Apr 25, 2025
@ashors1
fix pyproject.toml
ba945da 
Signed-off-by: ashors1 <ashors@nvidia.com>
@ashors1 ashors1 marked this pull request as draft April 25, 2025 23:25
ashors1 added 3 commits April 28, 2025 11:34
@ashors1
add docstrings to util functions
926eb51 
Signed-off-by: ashors1 <ashors@nvidia.com>
@ashors1
add more documentation
f24edeb 
Signed-off-by: ashors1 <ashors@nvidia.com>
@ashors1
Merge branch 'main' of github.com:NVIDIA/nemo-rl into ashors/cleanup
60ea67b
@ashors1
small fixes
e112008 
Signed-off-by: ashors1 <ashors@nvidia.com>
@github-actions github-actions bot added the documentation Improvements or additions to documentation label Apr 30, 2025
@ashors1 ashors1 marked this pull request as ready for review April 30, 2025 04:46
ashors1 added 2 commits April 29, 2025 22:24
@ashors1
fix grpo final validation
7890021 
Signed-off-by: ashors1 <ashors@nvidia.com>
@ashors1
fix validation timing
71603d5 
Signed-off-by: ashors1 <ashors@nvidia.com>
SahilJain314
SahilJain314 reviewed May 2, 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.

(reviewing in chunks)

terrykong
terrykong requested changes May 3, 2025
View reviewed changes
Copy link
Collaborator

@terrykong terrykong left a comment

Choose a reason for hiding this comment

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

thanks for cleaning up our loops. i've left some comments

ashors1 added 2 commits May 5, 2025 11:35
@ashors1
address comments
7c657a3 
Signed-off-by: ashors1 <ashors@nvidia.com>
@ashors1
address Terry's comments
5e08f9b 
Signed-off-by: ashors1 <ashors@nvidia.com>
terrykong
terrykong previously approved these changes May 5, 2025
View reviewed changes
@ashors1
small fix
9ca06bf 
Signed-off-by: ashors1 <ashors@nvidia.com>
@ashors1
add unit tests
30a9f47 
Signed-off-by: ashors1 <ashors@nvidia.com>
SahilJain314
SahilJain314 reviewed May 7, 2025
View reviewed changes
@yfw yfw mentioned this pull request May 13, 2025
Merged
4 tasks
@ashors1 ashors1 added the CI:L1 Run doctests, unit tests, and functional tests label May 15, 2025
ashors1 added 4 commits May 15, 2025 13:24
@ashors1
improve dataloader util
6d10b15 
Signed-off-by: ashors1 <ashors@nvidia.com>
@ashors1
fix num steps in dpo and grpo
af26c69 
Signed-off-by: ashors1 <ashors@nvidia.com>
@ashors1
fix indexing
e46aa89 
Signed-off-by: ashors1 <ashors@nvidia.com>
@ashors1
remove unused grpo seed
7e53c72 
Signed-off-by: ashors1 <ashors@nvidia.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@SahilJain314 SahilJain314 SahilJain314 left review comments

@terrykong terrykong terrykong left review comments

@parthchadha parthchadha Awaiting requested review from parthchadha

At least 1 approving review is required to merge this pull request.

Assignees
No one assigned
Labels
CI:L1 Run doctests, unit tests, and functional tests documentation Improvements or additions to documentation
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Refactor and clean up codebase
5 participants
@ashors1 @terrykong @SahilJain314 @yuki-666 @parthchadha