Skip to content

[cfg,trainer] feat: (MOPD, 1/3): Multi-teacher config dict#5774

Closed
JacobHelwig wants to merge 9 commits intoverl-project:mainfrom
JacobHelwig:jhelwig/mopdCfgDict
Closed

[cfg,trainer] feat: (MOPD, 1/3): Multi-teacher config dict#5774
JacobHelwig wants to merge 9 commits intoverl-project:mainfrom
JacobHelwig:jhelwig/mopdCfgDict

Conversation

@JacobHelwig
Copy link
Copy Markdown
Collaborator

What does this PR do?

Config changes for Multi-teacher OPD. Adds multiple teacher model configs to DistillationConfig. Maintains single teacher support elsewhere.

Design & Code Changes

Adds a teacher model config dict to the distillation config. We also maintain a teacher_model entry for single teacher OPD.

For the multi-teacher training script, teacher model args will be specified as:

+distillation.teacher_models.gsm8k.task="openai/gsm8k"    
+distillation.teacher_models.gsm8k.model_path="path/to/math_teacher"
+distillation.teacher_models.geo3k.task="hiyouga/geometry3k"
+distillation.teacher_models.geo3k.model_path="path/to/vision_math_teacher"

Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request refactors the distillation configuration by moving resource pool settings, such as enable_resource_pool, n_gpus_per_node, and nnodes, from the individual teacher model level to the top-level distillation configuration. It also removes the num_workers parameter and introduces a teacher_models dictionary to support future multi-teacher distillation. Additionally, the changes include improved validation logic for teacher model initialization and resource allocation across the trainer and experimental loops. I have no feedback to provide as there were no review comments to evaluate.

@JacobHelwig JacobHelwig changed the title [MOPD, 1/n][cfg,trainer] feat: Multi-teacher config dict [MOPD, 1/3][cfg,trainer] feat: Multi-teacher config dict Apr 7, 2026
@JacobHelwig JacobHelwig changed the title [MOPD, 1/3][cfg,trainer] feat: Multi-teacher config dict [cfg,trainer] feat: (MOPD, 1/3): Multi-teacher config dict Apr 7, 2026
@JacobHelwig
Copy link
Copy Markdown
Collaborator Author

/gemini review

Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request refactors the distillation configuration to support multi-teacher setups by moving resource pool settings from the teacher model configuration to the top-level distillation configuration and introducing a teacher_models dictionary. The changes include updating the trainer logic, configuration files, and validation methods to accommodate these structural updates. I have no feedback to provide as there were no review comments.

@wuxibin89
Copy link
Copy Markdown
Collaborator

Merged into #5834

@wuxibin89 wuxibin89 closed this Apr 13, 2026
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