Skip to content
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

Fix torch device #41

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open

Conversation

deepfates
Copy link

Bug: device not set correctly, some tensors on cpu while the rest are successfully on mps.

RuntimeError: Placeholder storage has not been allocated on MPS device!

The device logic was duplicated across several files, so it was easy to miss the one line where this happened. I've moved that logic into a function in torch_device.py.

@HaileyStorm
Copy link
Contributor

HaileyStorm commented Oct 7, 2024

You left your prompts.py change in, might want to remove that for the PR (a suggestion, to be clear, I'm not important :P).

@deepfates deepfates changed the title Df/get torch device Fix torch device Oct 7, 2024
@deepfates
Copy link
Author

That's what I get for trying to go back and isolate changes in commits instead of just doing it all at once lol. It's a good prompt for this kind of thing but i'm taking it back for clarity of PR

@xjdr-alt
Copy link
Owner

xjdr-alt commented Oct 8, 2024

@Arrabonae

@Laz4rz
Copy link

Laz4rz commented Oct 10, 2024

Just wasted 2h figuring out why KVCache is misplaced on mps, and then saw this PR already exists. One thing here: the buffers are already on the correct device, hence we don't have to move the KVCache to device while KVCache.new.

@ParthSareen
Copy link

Just wasted 2h figuring out why KVCache is misplaced on mps, and then saw this PR already exists. One thing here: the buffers are already on the correct device, hence we don't have to move the KVCache to device while KVCache.new.

Dude I literally made the fix and was about to PR 💀

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

Successfully merging this pull request may close these issues.

6 participants