refactor: update TypedDict to BaseModel (MasterConfig & ClippedPGLossConfig)#2325
Merged
Conversation
Contributor
Author
|
/ok to test a2ce5d6 |
Contributor
Author
|
/ok to test 88ffb9f |
Contributor
Author
|
/ok to test 73fcbb9 |
Contributor
Author
|
/ok to test 3be0da1 |
Contributor
Author
|
/ok to test 9ea8a92 |
Contributor
Author
|
/ok to test f6e2623 |
Contributor
Author
|
/ok to test 3f17dce |
Signed-off-by: Yuki Huang <yukih@nvidia.com>
Signed-off-by: Yuki Huang <yukih@nvidia.com>
Signed-off-by: Yuki Huang <yukih@nvidia.com>
Signed-off-by: Yuki Huang <yukih@nvidia.com>
Signed-off-by: Yuki Huang <yukih@nvidia.com>
Signed-off-by: Yuki Huang <yukih@nvidia.com>
Signed-off-by: Yuki Huang <yukih@nvidia.com>
Signed-off-by: Yuki Huang <yukih@nvidia.com>
Signed-off-by: Yuki Huang <yukih@nvidia.com>
Signed-off-by: Yuki Huang <yukih@nvidia.com>
Signed-off-by: Yuki Huang <yukih@nvidia.com>
|
Auto-sync is disabled for ready for review pull requests in this repository. Workflows must be run manually. Contributors can view more details about this message here. |
Contributor
|
/ok to test 6fd074f |
Signed-off-by: Yuki Huang <yukih@nvidia.com>
Contributor
Author
|
/ok to test 92bf84f |
RayenTian
reviewed
May 14, 2026
RayenTian
reviewed
May 14, 2026
RayenTian
reviewed
May 14, 2026
RayenTian
reviewed
May 14, 2026
Signed-off-by: Yuki Huang <yukih@nvidia.com>
Contributor
Author
|
/ok to test 8719153 |
Contributor
Author
|
thanks @RayenTian , good catch! resolved in 8719153, could you help to take a look again? |
RayenTian
reviewed
May 14, 2026
Contributor
|
Thanks @yuki-97. LGTM! Approved! |
RayenTian
approved these changes
May 14, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Related issues
dataclassinstead ofTypedDictfor config #1675dataclassesinstead ofTypedDictto handle defaults (but not #2102Summary
This PR refactors
MasterConfig(in all algorithm entry points:grpo.py,sft.py,dpo.py,rm.py,distillation.py,eval.py) andClippedPGLossConfigfromTypedDicttopydantic.BaseModel.We can now get four abilities when overriding config fields via Hydra/OmegaConf:
uv run python examples/run_grpo.py ++loss_fn.custom=1 # cfg.custom is now accessible inside ClippedPGLossFn.__init__uv run python examples/run_grpo.py ++loss_fn.disable_ppo_ratio=1.2 # Will raise error: "Input should be a valid boolean [type=bool_type, input_value=1.2, input_type=float]"config.disable_ppo_ratiojumps directly to the field definition inClippedPGLossConfig, whereas the previous dict-style access (config["disable_ppo_ratio"]) was opaque to static analysis tools.Changes
MasterConfigingrpo.py,sft.py,dpo.py,rm.py,distillation.py,eval.py:TypedDict→BaseModel(extra="allow")ClippedPGLossConfiginloss_functions.py:TypedDict→BaseModel(extra="allow")config["key"]) on these two config types updated to attribute access (config.key)skills/config-conventions/SKILL.mdupdated to reflect the newBaseModelconventionResults before / after the changes