[train] Clean up checkpoint config and trainer param deprecations #58022
Conversation
Signed-off-by: Justin Yu <[email protected]>
Signed-off-by: Justin Yu <[email protected]>
Signed-off-by: Justin Yu <[email protected]>
Signed-off-by: Justin Yu <[email protected]>
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request effectively deprecates checkpoint_at_end and checkpoint_frequency in CheckpointConfig, and resume_from_checkpoint and metadata in various Trainer constructors. The implementation for deprecation is solid, raising DeprecationWarning upon usage. The changes are consistent across the affected modules.
My main feedback is to improve the docstrings for the deprecated parameters. While the deprecation warnings at runtime are informative, updating the docstrings to include the reason for deprecation and the recommended alternative would greatly improve the developer experience for users browsing the API documentation. I've left specific suggestions on the relevant files.
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
Signed-off-by: Justin Yu <[email protected]>
…_config_cleanup
Signed-off-by: Justin Yu <[email protected]>
Signed-off-by: Justin Yu <[email protected]>
…y-project#58022) Deprecate `CheckpointConfig(checkpoint_at_end, checkpoint_frequency)` and mark the `resume_from_checkpoint, metadata` Trainer constructor arguments as deprecated in the docstrings. Update the "inspecting results" user guide doc code to show how to catch and inspect errors raised by trainer.fit(). The previous recommendation to check result.error is unusable because we always raise the error which prevents the user from accessing the result object. --------- Signed-off-by: Justin Yu <[email protected]>
…y-project#58022) Deprecate `CheckpointConfig(checkpoint_at_end, checkpoint_frequency)` and mark the `resume_from_checkpoint, metadata` Trainer constructor arguments as deprecated in the docstrings. Update the "inspecting results" user guide doc code to show how to catch and inspect errors raised by trainer.fit(). The previous recommendation to check result.error is unusable because we always raise the error which prevents the user from accessing the result object. --------- Signed-off-by: Justin Yu <[email protected]> Signed-off-by: Aydin Abiar <[email protected]>
2 participants
Description
Deprecate
CheckpointConfig(checkpoint_at_end, checkpoint_frequency)and mark theresume_from_checkpoint, metadataTrainer constructor arguments as deprecated in the docstrings.Drive-by changes
Update the "inspecting results" user guide doc code to show how to catch and inspect errors raised by trainer.fit(). The previous recommendation to check result.error is unusable because we always raise the error which prevents the user from accessing the result object.
Related issues
#49454