-
Notifications
You must be signed in to change notification settings - Fork 234
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
MacOS test falsely uses MPS, fails and is misreported as passing #1416
Comments
mikekgfb
added a commit
to mikekgfb/torchchat-1
that referenced
this issue
Dec 11, 2024
as per pytorch#1416 torchchat on hosts without MPS (which is all github hosts which use kvm to virtualize MacOS, but not MPS) should choose CPU as "fast" device. The logic is present (see discussion in pytorch#1416 ), but either not fully functional (that would be the easier one to fix, just print the result of get_device_str and fix the code!) or specifically ignored on load in torch/serialization.py (If this is the case, we're effectively looking at a core PyTorch issue....) In the meantime, this bandaid just forces the use of CPU on MacOS tests, to make MacOS tests run on CPU -- labeit hsortcircuiting test/execution of the "fast" device logic. Not ideal, but some testing beats no testing.
Jack-Khuu
added a commit
that referenced
this issue
Jan 24, 2025
…ng to MPS (#1417) * bandaid for run-readme-pr-macos.yml incorrectly loading to MPS as per #1416 torchchat on hosts without MPS (which is all github hosts which use kvm to virtualize MacOS, but not MPS) should choose CPU as "fast" device. The logic is present (see discussion in #1416 ), but either not fully functional (that would be the easier one to fix, just print the result of get_device_str and fix the code!) or specifically ignored on load in torch/serialization.py (If this is the case, we're effectively looking at a core PyTorch issue....) In the meantime, this bandaid just forces the use of CPU on MacOS tests, to make MacOS tests run on CPU -- labeit hsortcircuiting test/execution of the "fast" device logic. Not ideal, but some testing beats no testing. * Update run-readme-pr-macos.yml Add informational message to MacOS CPU tests * Update build_native.sh Update to C++11 ABI for AOTI, similar to ET --------- Co-authored-by: Jack-Khuu <[email protected]>
vmpuri
pushed a commit
that referenced
this issue
Feb 4, 2025
…ng to MPS (#1417) * bandaid for run-readme-pr-macos.yml incorrectly loading to MPS as per #1416 torchchat on hosts without MPS (which is all github hosts which use kvm to virtualize MacOS, but not MPS) should choose CPU as "fast" device. The logic is present (see discussion in #1416 ), but either not fully functional (that would be the easier one to fix, just print the result of get_device_str and fix the code!) or specifically ignored on load in torch/serialization.py (If this is the case, we're effectively looking at a core PyTorch issue....) In the meantime, this bandaid just forces the use of CPU on MacOS tests, to make MacOS tests run on CPU -- labeit hsortcircuiting test/execution of the "fast" device logic. Not ideal, but some testing beats no testing. * Update run-readme-pr-macos.yml Add informational message to MacOS CPU tests * Update build_native.sh Update to C++11 ABI for AOTI, similar to ET --------- Co-authored-by: Jack-Khuu <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
🐛 Describe the bug
#1415, #1404 and all other PRs fail on test-readme-macos when torchchat apparently falsely tries to load the model to MPS.
Due to #1315 , the test don't report as failed, so things get committed anyway.
I don't know why test-readme-macos would try to load into MPS. There's a multi-layered story here, where virtualized Mac does not support MPS (I think because most likely there's no MMU for MPS, so you can't virtualize MPS). MPS is however still reported as available by the OS, and hence a simple check
torch.backends.mps.is_available():
is not sufficient because pytorch thinks that MPS is actually available (but any and all memory allocations should fail).We're trying to fix this by doing an allocation of a tensor in MPS memory and see if that succeeds or fails in
is_mps_available()
here => https://github.com/pytorch/torchchat/blob/main/torchchat/utils/build_utils.py#L269 whenget_device_str()
is looking to check if MPS is available, and the fastest device should be returned as MPS.Ideally, this should be fixed in the expansion
get_device_str()
andis_mps_available()
functions, together with #1315.Fail example is here => https://github.com/pytorch/torchchat/actions/runs/12243820522/job/34154220414?pr=1404
Versions
problem occurs in github ci/cd
The text was updated successfully, but these errors were encountered: