Skip to content
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

[RFC] Deprecate callback hooks on_init_start and on_init_end #10894

Closed
ananthsub opened this issue Dec 2, 2021 Discussed in #10677 · 6 comments · Fixed by #10940
Closed

[RFC] Deprecate callback hooks on_init_start and on_init_end #10894

ananthsub opened this issue Dec 2, 2021 Discussed in #10677 · 6 comments · Fixed by #10940
Assignees
Labels
callback deprecation Includes a deprecation hooks Related to the hooks API

Comments

@ananthsub
Copy link
Contributor

ananthsub commented Dec 2, 2021

Discussed in #10677

Part of #7740

Originally posted by daniellepintz November 22, 2021
These hooks are called when trainer initialization begins and ends, before the model has been set, essentially allowing the user to modify the Trainer constructor. Should we be giving the user this much control over Trainer constructor? Are there scenarios where this is needed? Or can we deprecate these hooks?

https://github.com/PyTorchLightning/pytorch-lightning/blob/338f3cf63686935355c749920b2f298f3d18a26f/pytorch_lightning/trainer/callback_hook.py#L55-L63

@carmocca @tchaton @awaelchli do you know how these hooks are used? Have you seen any examples of these being used by the community? These hooks go way, way back, but I can't think of when they'd be needed given the user handles the Trainer initialization. It's also unclear when on_init_start actually happens: does that mean callbacks should be the first thing initialized?

We also have correctness issues for these hooks when callbacks are passed directly to the Trainer vs configured from the LightningModule: https://github.com/PyTorchLightning/pytorch-lightning/blob/f407a00cec59886cbaf8816630f6760e34926bd5/pytorch_lightning/core/lightning.py#L1140-L1142

Therefore, as a user, it seems more concise to write this:

trainer = Trainer(...)
run_all_my_fancy_logic_now(trainer)

# use the trainer here

vs this:

class MyCallback(Callback):
    def on_init_start(self, trainer) -> None:
        ...
    
    def on_init_end(self, trainer) -> None:
        ...
 
cb = MyCallback()
trainer = Trainer(callbacks=[cb])

cc @awaelchli @ananthsub @tchaton @carmocca @Borda @ninginthecloud

@ananthsub ananthsub added callback hooks Related to the hooks API labels Dec 2, 2021
@awaelchli
Copy link
Contributor

awaelchli commented Dec 3, 2021

I agree this part

We also have correctness issues for these hooks when callbacks are passed directly to the Trainer vs configured from the LightningModule:

is probably the strangest about these hooks. Currently on_init_start is used by none and on_init_end is used by three of our own callbacks in Lightning: EarlyStopping, ModelCheckpoint, Progress bar. The instructions in these hooks could probably also be move to another hook. None of them modify the trainer.

For context, the init hooks were introduced way back in #889

@daniellepintz daniellepintz self-assigned this Dec 3, 2021
@ananthsub ananthsub added the deprecation Includes a deprecation label Dec 3, 2021
@daniellepintz daniellepintz changed the title [RFC] Deprecate callback hooks on_init_start and on_init_end hooks [RFC] Deprecate callback hooks on_init_start and on_init_end Dec 4, 2021
@Erotemic
Copy link
Contributor

Is there a strategy for migrating away from these hooks? I've been using them to create a custom logger:

    def on_init_end(self, trainer: 'pl.Trainer') -> None:
        import pathlib
        self.log_dir = pathlib.Path(trainer.log_dir)
        self.log_fpath = self.log_dir / 'text_logs.log'
        self._log = _InstanceLogger.from_instance(trainer, self.log_fpath)
        self._log.info('on_init_end')
        self._log.info('sys.argv = {!r}'.format(sys.argv))
        trainer.text_logger = self
        if self.args is not None:
            self._log.info('args_dict = {}'.format(ub.repr2(self.args.__dict__, nl=1, sort=0)))

Am I SOL when 1.8 drops?

@carmocca
Copy link
Contributor

Have you tried the setup hook?

@iamnmn9
Copy link

iamnmn9 commented Nov 3, 2022

Hey!
Is there any solution for this?
AttributeError: 'EarlyStopping' object has no attribute 'on_init_end'
I am trying to implement early stopping.

@awaelchli
Copy link
Contributor

@NamanPundir Can you share more from that error message please? Please note that on_init_start and on_init_end have been deprecated and then eventually removed. So there is no way to "fix" it. If you needed that hook, an alternative is the setup() hook. Another option is to downgrade to Lightning version <= 1.7.x. Hope this helps.

@iamnmn9
Copy link

iamnmn9 commented Nov 4, 2022

@NamanPundir Can you share more from that error message please? Please note that on_init_start and on_init_end have been deprecated and then eventually removed. So there is no way to "fix" it. If you needed that hook, an alternative is the setup() hook. Another option is to downgrade to Lightning version <= 1.7.x. Hope this helps.

ahh i figured. Anyways I am using huggingface trainer for earlystopping now. Its working. :) Thanks

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
callback deprecation Includes a deprecation hooks Related to the hooks API
Projects
No open projects
Status: No status
Development

Successfully merging a pull request may close this issue.

6 participants