-
Notifications
You must be signed in to change notification settings - Fork 3.5k
use auto for MPS #14639
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
Comments
@williamFalcon You are on Lightning PyTorch 1.7.5 right? The Trainer defaults to CPU always, on all systems. This is why the warning shows. Only when setting accelerator="auto" does the selection happen automatically (and the warning shouldn't appear). Can you confirm this for your Trainer settings?
Agreed :) |
interesting… i thought the default was auto. what about making auto the default? not CPU? accelerator=auto |
Yep, I faintly remember discussions regarding this around the time when "auto" was introduced and the topic of Trainer 2.0 came up. It would be a massive "breaking" change, which is why we had held back on exploring this I guess. But I would consider it. As for MPS, defaulting to it in the current state would IMO be unsafe, as PyTorch is still working on supporting many torch ops. Right now, lots of PyTorch code would break on MPS. I would expect this to improve a lot in the future so that eventually one can default to it. Not an expert opinion, just my impression atm. |
Adrian already described the reasons why this isn't done. The open issue about defaulting to auto is #10606. I'll close this one as we would want to make this change for all accelerators together, not just MPS. Let's continue discussing
It does appear as a specification of GPU: We chose this for several reasons:
|
This shall be considered a change for potential v2.0, as in any major release, we are allowed to break some exiting patterns... |
Uh oh!
There was an error while loading. Please reload this page.
Why do we have this warning? instead, we should use MPS by default...
also, MPS should show up as one of the accelerators.
@carmocca
cc @akihironitta @justusschock
The text was updated successfully, but these errors were encountered: