You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@beam.apache.org by "bfcrampton (via GitHub)" <gi...@apache.org> on 2024/01/12 01:08:48 UTC
[PR] Update pytorch_inference.py [beam]
bfcrampton opened a new pull request, #29999:
URL: https://github.com/apache/beam/pull/29999
Add support for M1/M2 mac
Add support for M1/M2 mac by allowing `mps` pytorch device type.
------------------------
Thank you for your contribution! Follow this checklist to help us incorporate your contribution quickly and easily:
- [X] Mention the appropriate issue in your description (for example: `addresses #123`), if applicable. This will automatically add a link to the pull request in the issue. If you would like the issue to automatically close on merging the pull request, comment `fixes #<ISSUE NUMBER>` instead.
- N/A
- [X] Update `CHANGES.md` with noteworthy changes.
- N/A
- [X] If this contribution is large, please file an Apache [Individual Contributor License Agreement](https://www.apache.org/licenses/icla.pdf).
- N/A
See the [Contributor Guide](https://beam.apache.org/contribute) for more tips on [how to make review process smoother](https://github.com/apache/beam/blob/master/CONTRIBUTING.md#make-the-reviewers-job-easier).
To check the build health, please visit [https://github.com/apache/beam/blob/master/.test-infra/BUILD_STATUS.md](https://github.com/apache/beam/blob/master/.test-infra/BUILD_STATUS.md)
GitHub Actions Tests Status (on master branch)
------------------------------------------------------------------------------------------------
[](https://github.com/apache/beam/actions?query=workflow%3A%22Build+python+source+distribution+and+wheels%22+branch%3Amaster+event%3Aschedule)
[](https://github.com/apache/beam/actions?query=workflow%3A%22Python+Tests%22+branch%3Amaster+event%3Aschedule)
[](https://github.com/apache/beam/actions?query=workflow%3A%22Java+Tests%22+branch%3Amaster+event%3Aschedule)
[](https://github.com/apache/beam/actions?query=workflow%3A%22Go+tests%22+branch%3Amaster+event%3Aschedule)
See [CI.md](https://github.com/apache/beam/blob/master/CI.md) for more information about GitHub Actions CI or the [workflows README](https://github.com/apache/beam/blob/master/.github/workflows/README.md) to see a list of phrases to trigger workflows.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add support for M1/M2 mac in PytorchModelHandlerTensor [beam]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #29999:
URL: https://github.com/apache/beam/pull/29999#issuecomment-2022668674
This pull request has been marked as stale due to 60 days of inactivity. It will be closed in 1 week if no further activity occurs. If you think that’s incorrect or this pull request requires a review, please simply write any comment. If closed, you can revive the PR at any time and @mention a reviewer or discuss it on the dev@beam.apache.org list. Thank you for your contributions.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add support for M1/M2 mac in PytorchModelHandlerTensor [beam]
Posted by "bfcrampton (via GitHub)" <gi...@apache.org>.
bfcrampton commented on PR #29999:
URL: https://github.com/apache/beam/pull/29999#issuecomment-1905023000
Thanks for the review!
Addressed the comments. On #3 I decided to make everything case insensitive as well as support specifying `cuda` or `CUDA` directly. This is a minor nit that I have to workaround when using this library, everywhere else a `device` would be either `cpu`, `cuda`, or `mps`.
Lmk what you think, happy to just leave the case insensitivity part.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add support for M1/M2 mac in PytorchModelHandlerTensor [beam]
Posted by "damccorm (via GitHub)" <gi...@apache.org>.
damccorm commented on PR #29999:
URL: https://github.com/apache/beam/pull/29999#issuecomment-1902774847
waiting on author
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add support for M1/M2 mac in PytorchModelHandlerTensor [beam]
Posted by "damccorm (via GitHub)" <gi...@apache.org>.
damccorm commented on code in PR #29999:
URL: https://github.com/apache/beam/pull/29999#discussion_r1467868929
##########
sdks/python/apache_beam/ml/inference/pytorch_inference.py:
##########
@@ -243,9 +250,12 @@ def __init__(
with PyTorch 1.9 and 1.10.
"""
self._state_dict_path = state_dict_path
- if device == 'GPU':
+ if device in ['gpu', 'GPU', 'cuda', 'CUDA']:
Review Comment:
Hey, sorry for the slow review - this looks mostly good. The only thing I'd ask is that if we go case-insensitive we do it all the way (so `device.lower() == 'gpu'` here, and the same type of change applied elsewhere in the PR). Otherwise, this looks good to me, thanks!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add support for M1/M2 mac in PytorchModelHandlerTensor [beam]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #29999:
URL: https://github.com/apache/beam/pull/29999#issuecomment-2037090933
This pull request has been closed due to lack of activity. If you think that is incorrect, or the pull request requires review, you can revive the PR at any time.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add support for M1/M2 mac in PytorchModelHandlerTensor [beam]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] closed pull request #29999: Add support for M1/M2 mac in PytorchModelHandlerTensor
URL: https://github.com/apache/beam/pull/29999
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add support for M1/M2 mac in PytorchModelHandlerTensor [beam]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #29999:
URL: https://github.com/apache/beam/pull/29999#issuecomment-1888309962
Assigning reviewers. If you would like to opt out of this review, comment `assign to next reviewer`:
R: @damccorm for label python.
Available commands:
- `stop reviewer notifications` - opt out of the automated review tooling
- `remind me after tests pass` - tag the comment author after tests pass
- `waiting on author` - shift the attention set back to the author (any comment or push by the author will return the attention set to the reviewers)
The PR bot will only process comments in the main thread (not review comments).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add support for M1/M2 mac in PytorchModelHandlerTensor [beam]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #29999:
URL: https://github.com/apache/beam/pull/29999#issuecomment-1902079433
Reminder, please take a look at this pr: @damccorm
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add support for M1/M2 mac in PytorchModelHandlerTensor [beam]
Posted by "damccorm (via GitHub)" <gi...@apache.org>.
damccorm commented on PR #29999:
URL: https://github.com/apache/beam/pull/29999#issuecomment-1902774816
next action author
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add support for M1/M2 mac in PytorchModelHandlerTensor [beam]
Posted by "damccorm (via GitHub)" <gi...@apache.org>.
damccorm commented on code in PR #29999:
URL: https://github.com/apache/beam/pull/29999#discussion_r1467873818
##########
sdks/python/apache_beam/ml/inference/pytorch_inference.py:
##########
@@ -91,6 +91,12 @@ def _load_model(
"Model handler specified a 'GPU' device, but GPUs are not available. "
"Switching to CPU.")
device = torch.device('cpu')
+ if device == torch.device('mps') and not (torch.backend.mps.is_available() and
Review Comment:
Ah, it also looks like `torch.device('mps')` causes older versions of pytorch to throw. To maintain compat with older versions, could we wrap this in a try except? It should be fine to silently swallow the exception since we'll throw below if someone tries to set the device to mps and the pytorch version doesn't support it.
This is why the test checks are failing
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org