Skip to content

Remove deprecated AcceleratorConnector.parallel_devices #12074

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
DuYicong515 opened this issue Feb 23, 2022 · 2 comments · Fixed by #12075
Closed

Remove deprecated AcceleratorConnector.parallel_devices #12074

DuYicong515 opened this issue Feb 23, 2022 · 2 comments · Fixed by #12075
Labels

Comments

@DuYicong515
Copy link
Contributor

DuYicong515 commented Feb 23, 2022

Proposed refactor

Remove the unused property AcceleratorConnector.parallel_devices

https://github.com/PyTorchLightning/pytorch-lightning/blob/550d3a640d1bfeb731a16b2996d3160f0f7eb071/pytorch_lightning/trainer/connectors/accelerator_connector.py#L762-L764

Alternative

parallel_devices = getattr(Trainer.strategy, "parallel_devices", [Trainer.strategy.root_device])

Motivation

Remove unused accelerator_connector properties

AcceleratorConnector.parallel_devices was not used within PytorchLightning anymore and this property were not meant to be public. Trainer.strategy.parallel_devices is preferred compared to Trainer.accelerator_connector.parallel_devices

Pitch

Remove the property AcceleratorConnector.parallel_devices

Additional context

No other usage, should be a safe remove

cc @justusschock @awaelchli @rohitgr7 @four4fish

@kaushikb11
Copy link
Contributor

@DuYicong515 Could you elaborate why we should remove it? It is used at multiple places in AcceleratorConnector.

@DuYicong515
Copy link
Contributor Author

DuYicong515 commented Feb 25, 2022

Hi @kaushikb11, Thanks for the comment!

There was no AcceleratorConnector.parallel_devices used in existing code base. Within accelerator_connector.py it's using a private variable _parallel_devices instead. Externally I think it's preferred to accessTrain.strategy.parallel_devices to access parallel_devices when using ParallelStrategy

The issue was intended to stop it being a public property, AcceleratorConnector._parallel_devices should be kept within accelrator_connector.py, while Trainer.strategy.parallel_devices is preferred compared to Trainer.accelerator_connector.parallel_devices

I also updated alternative + motivation section of the issue. Let me know if you have any further questions!

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

Successfully merging a pull request may close this issue.

2 participants