Skip to content

Conversation

@wang2yn84
Copy link
Collaborator

@wang2yn84 wang2yn84 commented Jan 10, 2026

#926 reported the current cli script is broken. The root cause is we only set the reference model config and other model config (rollout, actor, etc.) cannot pick up from there. This PR fixes this issue by dynamically picking up the updated model config and override in other model configs. This PR also fixes the issue that "gemma-3" in the model name is not supported issue. So now all the script at least can process the configs properly.

It's a good idea to open an issue first for discussion.

Reference

Colab Notebook

Checklist

  • I have added all the necessary unit tests for my change.
  • I have verified that my change does not break existing code and all unit tests pass.
  • I have added all appropriate doc-strings/documentation.
  • My PR is based on the latest changes of the main branch (if unsure, rebase the code).
  • I have signed the Contributor License Agreement.
  • I have followed Contribution Guidelines.

@wang2yn84
Copy link
Collaborator Author

@Hanjun-Dai for review.

@wang2yn84
Copy link
Collaborator Author

#926 reported the current cli script is broken. The root cause is for certain keys, such as model_name etc., we only set the reference model config in the script and other model config (rollout, actor, etc.) still inherits from the base_config.yml. This PR fixes this issue by dynamically picking up the updated model config and override in other model configs.

It's a good idea to open an issue first for discussion.

Reference

Colab Notebook

Checklist

  • I have added all the necessary unit tests for my change.
  • I have verified that my change does not break existing code and all unit tests pass.
  • I have added all appropriate doc-strings/documentation.
  • My PR is based on the latest changes of the main branch (if unsure, rebase the code).
  • I have signed the Contributor License Agreement.
  • I have followed Contribution Guidelines.

@wang2yn84
Copy link
Collaborator Author

#926 reported the current cli script is broken. The root cause is we only set the reference model config and other model config (rollout, actor, etc.) cannot pick up from there. This PR fixes this issue by dynamically picking up the updated model config and override other model configs.

It's a good idea to open an issue first for discussion.

Reference

Colab Notebook

Checklist

  • I have added all the necessary unit tests for my change.
  • I have verified that my change does not break existing code and all unit tests pass.
  • I have added all appropriate doc-strings/documentation.
  • My PR is based on the latest changes of the main branch (if unsure, rebase the code).
  • I have signed the Contributor License Agreement.
  • I have followed Contribution Guidelines.

@Hanjun-Dai
Copy link
Contributor

LGTM, thank you so much for the elegant refactoring! this makes things much more consistent and unified!!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants