Fix SFTConfig max_seq_length compatibility with newer TRL versions #8836
Conversation
- Add compatibility layer for TRL versions >= 0.16.0 that use 'max_length' instead of 'max_seq_length' - Dynamically detect which parameter is supported by inspecting SFTConfig signature - Maintains backward compatibility with older TRL versions - Fixes TypeError: SFTConfig.__init__() got an unexpected keyword argument 'max_seq_length' Resolves stanfordnlp#8762
- Remove trailing whitespace - Fix line spacing consistency
| # Add the sequence length parameter using the appropriate name for the TRL version | ||
| if "max_seq_length" in sft_config_params: | ||
| # Older TRL versions (< 0.16.0) | ||
| config_kwargs["max_seq_length"] = train_kwargs["max_seq_length"] |
There was a problem hiding this comment.
It's possible that the user is setting max_length instead of max_seq_length, then max_length will go nowhere.
Instead of this silent transformation, can we raise an error on detecting mismatch between train_kwargs and SFTConfig on the name of max length? The error can be something like "max_seq_length is replaced by max_length in trl>=0.16.0, but you set ... on trl={the_detected_version}"
There was a problem hiding this comment.
Good point, thank you for the feedback. After thinking about this, I’ve come to the conclusion that it’s probably better to just let the code fail as it currently does, and let the user either install the correct version of trl or update the current implementation in DSPy to work only with the latest trl API and fail otherwise. In both cases, we should specify which version of trl is recommended and perhaps include a check for it. Otherwise it becomes a maintenance mess if trl API changes again in the future ?
Fixes compatibility issue with TRL >= 0.16.0 where SFTConfig parameter changed from max_seq_length to max_length.
Added dynamic parameter detection to support both old and new TRL versions.
Fixes #8762