-
Notifications
You must be signed in to change notification settings - Fork 3.6k
Description
🚀 Feature
Title says it all
Motivation
This is a legacy pattern internal to the trainer codebase today:
https://github.com/PyTorchLightning/pytorch-lightning/blob/b9a52fa2ef31f12f6992ece18a033318ec551907/pytorch_lightning/trainer/trainer.py#L315-L331
Inside the Trainer constructor, these connector classes are passed the trainer in their own constructors.
The connector class can then expose a set of hooks that the Trainer calls. Internally, the connector class updates the state on the trainer.
This adds an extra level of indirection, obfuscates what properties are set on the trainer, and makes unit testing more challenging since these components all end up requiring a trainer object.
For simple cases like the profiler connector, we should directly instantiate the object inside of the trainer constructor itself. https://github.com/PyTorchLightning/pytorch-lightning/blob/b9a52fa2ef31f12f6992ece18a033318ec551907/pytorch_lightning/trainer/connectors/profiler_connector.py#L38-L55
This is the approach the most recent AcceleratorConnector has taken, so we should ensure consistency across the codebase.
Pitch
Deprecate connector classes in favor of utility functions and/or direct instantiation/property setting from within the trainer constructor
Alternatives
Keep as is