-
Notifications
You must be signed in to change notification settings - Fork 2.3k
refactor: Move Mergekit integration to experimental submodule #4438
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.
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
base: main
Are you sure you want to change the base?
Conversation
Resolves #4404 - Remove SIMPLE_CHAT_TEMPLATE import from 7 trainer scripts - Remove chat template setting for non-SFT trainers (DPO, CPO, ORPO, PPO, Nash-MD, XPO, Online DPO) - Chat templates only make sense for SFT (instruction tuning), not for preference optimization or reward-based training - Scripts modified: - examples/scripts/online_dpo.py - examples/scripts/orpo.py - examples/scripts/cpo.py - examples/scripts/nash_md.py - examples/scripts/xpo.py - examples/scripts/ppo/ppo.py - examples/scripts/ppo/ppo_tldr.py
Resolves #4395 - Move mergekit_utils.py to trl/experimental/mergekit/ - Move MergeModelCallback to trl/experimental/mergekit/callbacks.py - Create trl/experimental/mergekit/__init__.py for module exports - Remove MergeModelCallback from main trl exports (trl/__init__.py, trl/trainer/__init__.py) - Remove MergeModelCallback from trl/trainer/callbacks.py - Update test imports to use trl.experimental.mergekit - Update callbacks documentation with experimental status note This change consolidates underutilized Mergekit functionality into the experimental namespace as part of v1.0 preparations. The module may be deprecated or removed in future versions if usage does not materialize. To use MergeModelCallback after this change: ```python from trl.experimental.mergekit import MergeConfig, MergeModelCallback ```
|
The docs for this PR live here. All of your documentation changes will be reflected on that endpoint. The docs are available until 30 days after the last update. |
docs/source/callbacks.md
Outdated
|
|
||
| [[autodoc]] WeaveCallback | ||
|
|
||
| ## Experimental Callbacks |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd prefer having a dedicated section under Experimental instead of(like BEMA)
… page - Created docs/source/merge_model_callback.md following BEMA pattern - Added merge_model_callback to experimental section in _toctree.yml - Removed inline experimental section from callbacks.md This addresses reviewer feedback to have a dedicated documentation page under Experimental section (like BEMA) instead of inline docs.
|
@qgallouedec I've restructured the MergeModelCallback documentation as requested. Changes made:
Pattern followed: Matches BEMA's structure with dedicated page under Experimental section, including:
The MergeModelCallback now has its own dedicated page in the Experimental section, just like BEMA for Reference Model. |
Summary
Resolves #4395
This PR moves all Mergekit-related functionality to the experimental submodule (
trl.experimental.mergekit) as part of v1.0 preparations. The Mergekit integration has seen low usage and may be deprecated or removed in future versions.Changes
Module Structure
trl/experimental/mergekit/module with:mergekit_utils.py- Core mergekit utilities (moved fromtrl/mergekit_utils.py)callbacks.py-MergeModelCallbackclass (extracted fromtrl/trainer/callbacks.py)__init__.py- Module exportsRemoved from Main API
MergeModelCallbackfromtrl/__init__.pyexportsMergeModelCallbackfromtrl/trainer/__init__.pyexportsMergeModelCallbackclass fromtrl/trainer/callbacks.pyis_mergekit_available,MergeConfig,merge_models,upload_model_to_hf)Updated Imports
tests/test_callbacks.pyto import fromtrl.experimental.mergekitDocumentation
docs/source/callbacks.mdwith:MergeModelCallbackfrom main callbacks sectionMigration Guide
Before
After
Rationale
As noted in issue #4395:
Moving to the experimental submodule:
Testing
@require_mergekitdecorator still works correctlyBreaking Change
MergeModelCallbackfrom the maintrlmodule. Users will need to update their imports to usetrl.experimental.mergekitinstead.