Skip to content

Conversation

@Likhita-17
Copy link

This change clarifies how SFTTrainer relies on Accelerate for device placement and distributed training.

It explains why device_map="auto" should not be used during training, and adds a minimal runnable example launched via accelerate.

Before submitting

  • This PR improves the documentation.

Who can review?

@stevhliu

This change clarifies how SFTTrainer relies on Accelerate for device placement and distributed training.

It explains why device_map="auto" should not be used during training, and adds a minimal runnable example launched via accelerate.
@qgallouedec
Copy link
Member

qgallouedec commented Dec 23, 2025

Thank you, I find that useful!

The fact is that the philosophy of trainers in TRL is that, we recommend passing the model ID to the trainer (i.e. a str), rather than the instantiated model itself. If the user passes the model (i.e. a nn.Module), we consider this to be advanced usage, and that the user knows what they are doing.

Therefore, I think it would be better to simply mention this briefly in the docstring for SFTTrainer.model, here: https://huggingface.co/docs/trl/en/sft_trainer#trl.SFTTrainer.model

@Likhita-17
Copy link
Author

Likhita-17 commented Dec 24, 2025

Thank you, I find that useful!

The fact is that the philosophy of trainers in TRL is that, we recommend passing the model ID to the trainer (i.e. a str), rather than the instantiated model itself. If the user passes the model (i.e. a nn.Module), we consider this to be advanced usage, and that the user knows what they are doing.

Therefore, I think it would be better to simply mention this briefly in the docstring for SFTTrainer.model, here: https://huggingface.co/docs/trl/en/sft_trainer#trl.SFTTrainer.model

Thanks for the clarification — that makes a lot of sense.

I agree that this fits better as a brief note in the SFTTrainer.model docstring , especially given the distinction between passing a model ID versus an instantiated module.

I’ll update the documentation accordingly and move the guidance there.
Thanks again for the review!

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