ref: separate slurm from ddp #3809
Conversation
|
This pull request is now in conflict... :( |
| def broadcast(self, obj, src=0): | ||
| return self.dist.broadcast(obj) |
There was a problem hiding this comment.
this is an override of the parent method in Accelerator. It's to keep behavior consistent
across accelerators.
src is already set into this object automatically.
|
Hello @williamFalcon! Thanks for updating this PR. There are currently no PEP 8 issues detected in this Pull Request. Cheers! 🍻 Comment last updated at 2020-10-03 02:47:46 UTC |
| # ------------------------------------------- | ||
| # !!!!!!!!!!!!!! NOTE !!!!!!!!!!!!!!!!!!!!!! | ||
| # TEMP CLASS WHILE WE DECOUPLE SLURM FROM DDP | ||
| # !!!!!!!!!!!!!! NOTE !!!!!!!!!!!!!!!!!!!!!! | ||
| # ------------------------------------------- | ||
| class DDPSLURMBackend(Accelerator): |
There was a problem hiding this comment.
what's the final envisioned setup for slurm + DDP? is torchelastic being split out too?
There was a problem hiding this comment.
te is already split out :)
I think right now it's to finish fixing DDP and having 3 types of ddp tied to the same file is making debugging impossible.
Once those tests pass, i'll fix the TE cpu ddp.
Then we'll see where we are with time, but i'd like to make something like a cluster_manager or something that is passed into the trainer as well and then linked to a backend.
ref: separate slurm from ddp