Fix SFT notebook parameter names for current API (RHOAIENG-37015) - #739
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Central YAML (base), Organization UI (inherited) Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughA Jupyter notebook for SFT training updates YAML/config names: Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 minutes 🚥 Pre-merge checks | ✅ 2✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 Ruff (0.15.9)examples/kfto-sft-llm/sft.ipynbUnexpected end of JSON input Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
5991f2f to
6f46a2a
Compare
6047d48 to
25ccfb7
Compare
Update example notebook to use current transformers/trl API parameter names. **Changes:** Update SFT training example parameter names in examples/kfto-sft-llm/sft.ipynb: - torch_dtype → dtype - use_liger → use_liger_kernel - max_seq_length → max_length - Removed deprecated dataset_batch_size parameter **Why:** The notebook was using deprecated parameter names from older versions of transformers/trl. Universal images (th06-*) already work correctly with the updated API. This fix ensures the example notebook is compatible with current package versions. **Testing:** Tested with universal image (quay.io/opendatahub/odh-th06-cuda130-torch291-py312:odh-stable) which has transformers 4.57.6 and trl 0.24.0 - notebook runs successfully with these parameter updates. **Note:** This only updates the example notebook. Runtime image package versions (py312-cuda128-torch280) are not changed as that image is deprecated. Users should migrate to universal images. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
5c8123a to
b1d3a0d
Compare
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: kapil27, sutaakar The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
/lgtm |
|
/lgtm |
Summary
Updates SFT training notebook to use current transformers/trl API parameter names.
Resolves: RHOAIENG-37015
Changes
Updated Example Notebook
File:
examples/kfto-sft-llm/sft.ipynbYAML parameters:
torch_dtype→dtypeuse_liger→use_liger_kernelmax_seq_length→max_lengthdataset_batch_sizeparameterPython code:
torch_dtype=model_args.torch_dtype→dtype=model_args.dtypeBackground
The notebook was using deprecated parameter names from older versions of transformers/trl packages. These parameter names changed in recent versions:
transformers >= 4.50.0usesdtypeinstead oftorch_dtypetrl >= 0.24.0uses new parameter names for SFTConfigTesting
✅ Tested with universal image:
quay.io/opendatahub/odh-th06-cuda130-torch291-py312:odh-stabletransformers==4.57.6,trl==0.24.0Impact
Note on Runtime Images
This PR only updates the example notebook. Runtime image package versions (e.g.,
py312-cuda128-torch280) are not changed, as those images are deprecated. Users are encouraged to migrate to universal images which already have the correct package versions.Files Changed
examples/kfto-sft-llm/sft.ipynb- Updated 5 parameter namesTotal: 5 insertions(+), 6 deletions(-)
Summary by CodeRabbit