Skip to content

[RFC] Revisit/merge/deprecate pre_dispatch, dispatch, post_dispatch and run_stage functions from strategies and trainer #10987

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
four4fish opened this issue Dec 8, 2021 · 2 comments · Fixed by #11166

Comments

@four4fish
Copy link
Contributor

four4fish commented Dec 8, 2021

Proposed refactor

Discussion raised from #10896

After 2/n simplify spawning logic @awaelchli has done, we can take a step further to simplify hooks

It was in the plan of #10059 in step 4, "deprecate dispatch and post_dispatch"

Will it make sense to deprecate pre/post dispatch and run_stage and aggregate logic into dispatch in both trainer and accelerator?

Motivation

Pitch

Additional context


If you enjoy Lightning, check out our other projects! ⚡

  • Metrics: Machine learning metrics for distributed, scalable PyTorch applications.

  • Lite: enables pure PyTorch users to scale their existing code on any kind of device while retaining full control over their own loops and optimization logic.

  • Flash: The fastest way to get a Lightning baseline! A collection of tasks for fast prototyping, baselining, fine-tuning, and solving problems with deep learning.

  • Bolts: Pretrained SOTA Deep Learning models, callbacks, and more for research and production with PyTorch Lightning and PyTorch.

  • Lightning Transformers: Flexible interface for high-performance research using SOTA Transformers leveraging Pytorch Lightning, Transformers, and Hydra.

cc @justusschock @awaelchli @akihironitta @kaushikb11 @ananthsub

@awaelchli
Copy link
Contributor

Alright, I looked at how/whether we can cleanly merge the setup() and pre_dispatch() methods and everything seems to work out quite well. I propose to merge them like so, with the on_fit_start call previously in the middle:

Before:

# trainer.py, _run():

...

self.training_type_plugin.setup(self)  # WE WANT TO MERGE THIS ...
call_hook("on_fit_start")
self.training_type_plugin.pre_dispatch()  #  ... AND THIS

...

After:

# trainer.py, _run():

...

self.training_type_plugin.setup(self)  # PRE_DISPATCH BECOMES JUST SETUP()
call_hook("on_fit_start")
...

Here is the PR showing how this would look like: #11137

@awaelchli
Copy link
Contributor

Had to be very careful here, because various plugins who previously overrode the pre_dispatch() or setup() had to be adjusted depending on whether they called super().x, to avoid unwanted calls to trickle down the inheritance chain. For this reason, the PR might be a bit hard to review for anyone who is not fully on top of these details ^^

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants