Skip to content

Wrong documentations for on_fit_* of TrainerCallbackHookMixin #11961

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

Closed
function2-llx opened this issue Feb 17, 2022 · 4 comments · Fixed by #12016
Closed

Wrong documentations for on_fit_* of TrainerCallbackHookMixin #11961

function2-llx opened this issue Feb 17, 2022 · 4 comments · Fixed by #12016
Labels
docs Documentation related hooks Related to the hooks API

Comments

@function2-llx
Copy link
Contributor

function2-llx commented Feb 17, 2022

Hello PyTorch Lightning team,

it seems that documentations for on_fit_* methods of TrainerCallbackHookMixin class are wrong (seems to misplace doc for on_init_start here).

https://github.com/PyTorchLightning/pytorch-lightning/blob/4dba492fb55e2862bb20f304e6bef43f1f151fc7/pytorch_lightning/trainer/callback_hook.py#L115-L139

cc @Borda @rohitgr7 @carmocca @awaelchli @ninginthecloud @daniellepintz

@ananthsub ananthsub added docs Documentation related hooks Related to the hooks API labels Feb 18, 2022
@daniellepintz
Copy link
Contributor

Hi @function2-llx, not sure I follow exactly. The on_fit_* hooks are not deprecated, but rather the whole TrainerCallbackHookMixin is deprecated which is why you see those deprecation messages in the code snippet. What is the problem exactly?

@ananthsub
Copy link
Contributor

What is the problem exactly?

The docs reference this line:

     Called when the trainer initialization begins, model has not yet been set. 

Which is not true for on_fit_start

@function2-llx
Copy link
Contributor Author

What is the problem exactly?

The docs reference this line:

     Called when the trainer initialization begins, model has not yet been set. 

Which is not true for on_fit_start

Exactly! Thanks.

@awaelchli
Copy link
Contributor

This used to be true but not anymore. It has changed roughly around the work we did on #10896 and such a doc update got easily missed. A PR for correcting this would be very welcomed ❤️

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
docs Documentation related hooks Related to the hooks API
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants