-
Notifications
You must be signed in to change notification settings - Fork 46
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
Add support for MPS device #176
Merged
Merged
Changes from 5 commits
Commits
Show all changes
11 commits
Select commit
Hold shift + click to select a range
81958a1
Add support for MPS device
constantinpape 41951f0
Fix typo
constantinpape 9289e38
Make micro-sam compatible with mps torch Macbook backend
GenevieveBuckley 6916e53
Use MPS fallback to CPU, not all torch/torchvision functions are impl…
GenevieveBuckley 92e3087
Merge pull request #178 from GenevieveBuckley/mps-edits
constantinpape 3e8fd1b
Vendor batched_mask_to_box function from segment_anything/util/amg.py…
GenevieveBuckley bbcc649
Endure input to batched_mask_to_box is boolean array, otherwise resul…
GenevieveBuckley 72a30dc
Make batched_mask_to_box compatible with MPS Pytorch backend (Apple S…
GenevieveBuckley 2c41d23
Only pass boolean masks to batched_mask_to_box function (bug means in…
GenevieveBuckley 6981148
Add tests for vendored batched_mask_to_box function from segment_anyt…
GenevieveBuckley 9e1e3bf
Merge pull request #180 from GenevieveBuckley/mps
constantinpape File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have traced down the failing test to this line. It seems like the bounding boxes computed by
batched_mask_to_box
are different when abool
tensor is passed than when anint
tensor is passed. This is unfortunate, but would be a bit of effort to fix since this is part of upstream code.@GenevieveBuckley I assume that you have made this change because something fails with
mps
if a bool tensor is used. Can you please send me the exact error message you get without this line? Then I will think about the best strategy here.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wow, I did not know boolean masks returned correct results, but integer masks return something totally different. Yikes! I have changed the type of this line to
torch.bool
, thank you for pointing it out.I have also gone ahead with my first suggestion here about how to handle the problem with
batched_mask_to_box()
. I have made a new file_vendored.py
containing a copy ofbatched_mask_to_box()
fromsegment_anything/util/amg.py
, which I have edited to (a) make sure the input mask is boolean, and (b) make compatible with the MPS Pytorch backend for apple silicon.I think this should fix the problems.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, it is really unfortunate... I think this is a pretty big bug on the torch / SegmentAnything side. It could be worth it to figure out what exactly causes it and report that at some point.
👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, I see you made an issue already :D
facebookresearch/segment-anything#552