You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@beam.apache.org by GitBox <gi...@apache.org> on 2022/06/14 21:06:37 UTC
[GitHub] [beam] ryanthompson591 opened a new pull request, #21868: Updated documentation for ml.inference docs.
ryanthompson591 opened a new pull request, #21868:
URL: https://github.com/apache/beam/pull/21868
Added documentation updates, mostly cleaning the documents for formatting but adding some extra clarifications in module documentation.
------------------------
Thank you for your contribution! Follow this checklist to help us incorporate your contribution quickly and easily:
- [ ] [**Choose reviewer(s)**](https://beam.apache.org/contribute/#make-your-change) and mention them in a comment (`R: @username`).
- [ ] 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.
- [ ] Update `CHANGES.md` with noteworthy changes.
- [ ] If this contribution is large, please file an Apache [Individual Contributor License Agreement](https://www.apache.org/licenses/icla.pdf).
See the [Contributor Guide](https://beam.apache.org/contribute) for more tips on [how to make review process smoother](https://beam.apache.org/contribute/#make-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)
See [CI.md](https://github.com/apache/beam/blob/master/CI.md) for more information about GitHub Actions CI.
--
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
[GitHub] [beam] yeandy commented on a diff in pull request #21868: Updated documentation for ml.inference docs.
Posted by GitBox <gi...@apache.org>.
yeandy commented on code in PR #21868:
URL: https://github.com/apache/beam/pull/21868#discussion_r898278520
##########
sdks/python/apache_beam/ml/inference/pytorch_inference.py:
##########
@@ -103,6 +106,13 @@ def run_inference(
This method stacks the list of Tensors in a vectorized format to optimize
the inference call.
+
+ Args:
+ batch: A sequence of Tensors.
+ model: A pytorch model.
Review Comment:
```suggestion
batch: A sequence of Tensors. These Tensors should be batchable, as this method will call `torch.stack()` and pass in batched Tensors with dimensions (batch_size, n_features, etc.) into the model's forward() function.
model: A PyTorch model.
inference_args: Non-batchable arguments required as inputs to the model's forward() function. Unlike Tensors in `batch`, these parameters will not be dynamically batched
```
--
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
[GitHub] [beam] TheNeuralBit commented on a diff in pull request #21868: Updated documentation for ml.inference docs.
Posted by GitBox <gi...@apache.org>.
TheNeuralBit commented on code in PR #21868:
URL: https://github.com/apache/beam/pull/21868#discussion_r898554493
##########
sdks/python/apache_beam/ml/inference/sklearn_inference.py:
##########
@@ -76,13 +77,21 @@ def _validate_inference_args(inference_args):
class SklearnModelHandlerNumpy(ModelHandler[numpy.ndarray,
PredictionResult,
BaseEstimator]):
- """ Implementation of the ModelHandler interface for scikit-learn
- using numpy arrays as input.
- """
def __init__(
self,
model_uri: str,
model_file_type: ModelFileType = ModelFileType.PICKLE):
+ """ Implementation of the ModelHandler interface for scikit-learn
+ using numpy arrays as input.
+
+ Example Usage:
+ pcoll | RunInference(SklearnModelHandlerNumpy(model_uri="my_uri"))
Review Comment:
Unfortunate that these were missed, but let's do these in a follow-up PR just to make sure we get the basic updates into the release cut. We can probably cherrypick these minor changes
--
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
[GitHub] [beam] asf-ci commented on pull request #21868: Updated documentation for ml.inference docs.
Posted by GitBox <gi...@apache.org>.
asf-ci commented on PR #21868:
URL: https://github.com/apache/beam/pull/21868#issuecomment-1155713197
Can one of the admins verify this patch?
--
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
[GitHub] [beam] yeandy commented on a diff in pull request #21868: Updated documentation for ml.inference docs.
Posted by GitBox <gi...@apache.org>.
yeandy commented on code in PR #21868:
URL: https://github.com/apache/beam/pull/21868#discussion_r897955352
##########
sdks/python/apache_beam/ml/inference/pytorch_inference.py:
##########
@@ -170,6 +183,9 @@ def run_inference(
For the same key across all examples, this will stack all Tensors values
in a vectorized format to optimize the inference call.
+
Review Comment:
Can we add inputs here?
##########
sdks/python/apache_beam/ml/inference/sklearn_inference.py:
##########
@@ -76,29 +85,47 @@ def load_model(self) -> BaseEstimator:
def run_inference(
self, batch: Sequence[numpy.ndarray], model: BaseEstimator,
**kwargs) -> Iterable[PredictionResult]:
+ """Runs inferences on a batch of numpy arrays.
+
Review Comment:
Can we add inputs here?
--
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
[GitHub] [beam] yeandy commented on a diff in pull request #21868: Updated documentation for ml.inference docs.
Posted by GitBox <gi...@apache.org>.
yeandy commented on code in PR #21868:
URL: https://github.com/apache/beam/pull/21868#discussion_r898253546
##########
sdks/python/apache_beam/ml/inference/pytorch_inference.py:
##########
@@ -190,6 +194,10 @@ def run_inference(
For the same key across all examples, this will stack all Tensors values
in a vectorized format to optimize the inference call.
+ Args:
+ batch: A sequence of Tensors.
+ model: A pytorch model.
Review Comment:
```suggestion
batch: A sequence of keyed Tensors. These Tensors should be batchable, as this method will call `torch.stack()` and pass in batched Tensors with dimensions (batch_size, n_features, etc.) into the model's forward() function.
model: A PyTorch model.
inference_args: Extra non-batchable arguments required as inputs to the model's forward() function. These should not be batchable, as this method will not call `torch.stack()` and pass them directly into the model's forward function()`
```
@AnandInguva Can you review my wording here?
--
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
[GitHub] [beam] yeandy commented on a diff in pull request #21868: Updated documentation for ml.inference docs.
Posted by GitBox <gi...@apache.org>.
yeandy commented on code in PR #21868:
URL: https://github.com/apache/beam/pull/21868#discussion_r898282378
##########
sdks/python/apache_beam/ml/inference/pytorch_inference.py:
##########
@@ -170,6 +183,9 @@ def run_inference(
For the same key across all examples, this will stack all Tensors values
in a vectorized format to optimize the inference call.
+
Review Comment:
I added suggestions. (you may need to fix my formatting from those suggestions though)
--
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
[GitHub] [beam] TheNeuralBit commented on a diff in pull request #21868: Updated documentation for ml.inference docs.
Posted by GitBox <gi...@apache.org>.
TheNeuralBit commented on code in PR #21868:
URL: https://github.com/apache/beam/pull/21868#discussion_r898554493
##########
sdks/python/apache_beam/ml/inference/sklearn_inference.py:
##########
@@ -76,13 +77,21 @@ def _validate_inference_args(inference_args):
class SklearnModelHandlerNumpy(ModelHandler[numpy.ndarray,
PredictionResult,
BaseEstimator]):
- """ Implementation of the ModelHandler interface for scikit-learn
- using numpy arrays as input.
- """
def __init__(
self,
model_uri: str,
model_file_type: ModelFileType = ModelFileType.PICKLE):
+ """ Implementation of the ModelHandler interface for scikit-learn
+ using numpy arrays as input.
+
+ Example Usage:
+ pcoll | RunInference(SklearnModelHandlerNumpy(model_uri="my_uri"))
Review Comment:
Let's do these in a follow-up PR
--
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
[GitHub] [beam] asf-ci commented on pull request #21868: Updated documentation for ml.inference docs.
Posted by GitBox <gi...@apache.org>.
asf-ci commented on PR #21868:
URL: https://github.com/apache/beam/pull/21868#issuecomment-1155713195
Can one of the admins verify this patch?
--
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
[GitHub] [beam] codecov[bot] commented on pull request #21868: Updated documentation for ml.inference docs.
Posted by GitBox <gi...@apache.org>.
codecov[bot] commented on PR #21868:
URL: https://github.com/apache/beam/pull/21868#issuecomment-1155804323
# [Codecov](https://codecov.io/gh/apache/beam/pull/21868?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#21868](https://codecov.io/gh/apache/beam/pull/21868?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (772f8eb) into [master](https://codecov.io/gh/apache/beam/commit/ecea6de6dc8b7d2c682dcaaf860a0ecfe666d380?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (ecea6de) will **decrease** coverage by `0.01%`.
> The diff coverage is `100.00%`.
```diff
@@ Coverage Diff @@
## master #21868 +/- ##
==========================================
- Coverage 74.07% 74.06% -0.02%
==========================================
Files 698 699 +1
Lines 92574 92602 +28
==========================================
+ Hits 68577 68582 +5
- Misses 22742 22765 +23
Partials 1255 1255
```
| Flag | Coverage Δ | |
|---|---|---|
| python | `83.73% <100.00%> (-0.03%)` | :arrow_down: |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/beam/pull/21868?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [...thon/apache\_beam/ml/inference/pytorch\_inference.py](https://codecov.io/gh/apache/beam/pull/21868/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vbWwvaW5mZXJlbmNlL3B5dG9yY2hfaW5mZXJlbmNlLnB5) | `0.00% <ø> (ø)` | |
| [...thon/apache\_beam/ml/inference/sklearn\_inference.py](https://codecov.io/gh/apache/beam/pull/21868/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vbWwvaW5mZXJlbmNlL3NrbGVhcm5faW5mZXJlbmNlLnB5) | `94.54% <ø> (ø)` | |
| [sdks/python/apache\_beam/ml/\_\_init\_\_.py](https://codecov.io/gh/apache/beam/pull/21868/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vbWwvX19pbml0X18ucHk=) | `100.00% <100.00%> (ø)` | |
| [sdks/python/apache\_beam/ml/inference/\_\_init\_\_.py](https://codecov.io/gh/apache/beam/pull/21868/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vbWwvaW5mZXJlbmNlL19faW5pdF9fLnB5) | `100.00% <100.00%> (ø)` | |
| [sdks/python/apache\_beam/ml/inference/base.py](https://codecov.io/gh/apache/beam/pull/21868/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vbWwvaW5mZXJlbmNlL2Jhc2UucHk=) | `95.83% <100.00%> (ø)` | |
| [.../python/apache\_beam/testing/test\_stream\_service.py](https://codecov.io/gh/apache/beam/pull/21868/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vdGVzdGluZy90ZXN0X3N0cmVhbV9zZXJ2aWNlLnB5) | `88.09% <0.00%> (-4.77%)` | :arrow_down: |
| [sdks/python/apache\_beam/io/source\_test\_utils.py](https://codecov.io/gh/apache/beam/pull/21868/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vaW8vc291cmNlX3Rlc3RfdXRpbHMucHk=) | `88.01% <0.00%> (-1.39%)` | :arrow_down: |
| [...che\_beam/runners/interactive/interactive\_runner.py](https://codecov.io/gh/apache/beam/pull/21868/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy9pbnRlcmFjdGl2ZS9pbnRlcmFjdGl2ZV9ydW5uZXIucHk=) | `90.06% <0.00%> (-1.33%)` | :arrow_down: |
| [...eam/runners/portability/fn\_api\_runner/execution.py](https://codecov.io/gh/apache/beam/pull/21868/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy9wb3J0YWJpbGl0eS9mbl9hcGlfcnVubmVyL2V4ZWN1dGlvbi5weQ==) | `92.44% <0.00%> (-0.65%)` | :arrow_down: |
| [sdks/python/apache\_beam/transforms/combiners.py](https://codecov.io/gh/apache/beam/pull/21868/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vdHJhbnNmb3Jtcy9jb21iaW5lcnMucHk=) | `93.05% <0.00%> (-0.39%)` | :arrow_down: |
| ... and [43 more](https://codecov.io/gh/apache/beam/pull/21868/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/beam/pull/21868?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/beam/pull/21868?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [ecea6de...772f8eb](https://codecov.io/gh/apache/beam/pull/21868?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
--
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
[GitHub] [beam] yeandy commented on a diff in pull request #21868: Updated documentation for ml.inference docs.
Posted by GitBox <gi...@apache.org>.
yeandy commented on code in PR #21868:
URL: https://github.com/apache/beam/pull/21868#discussion_r898253546
##########
sdks/python/apache_beam/ml/inference/pytorch_inference.py:
##########
@@ -190,6 +194,10 @@ def run_inference(
For the same key across all examples, this will stack all Tensors values
in a vectorized format to optimize the inference call.
+ Args:
+ batch: A sequence of Tensors.
+ model: A pytorch model.
Review Comment:
```suggestion
batch: A sequence of keyed Tensors. These Tensors should be batchable, as this method will call `torch.stack()` and pass in batched Tensors with dimensions (batch_size, n_features, etc.) into the model's forward() function.
model: A PyTorch model.
inference_args: Non-batchable arguments required as inputs to the model's forward() function. Unlike Tensors in `batch`, these parameters will not be dynamically batched
```
@AnandInguva Can you review my wording here?
--
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
[GitHub] [beam] ryanthompson591 commented on a diff in pull request #21868: Updated documentation for ml.inference docs.
Posted by GitBox <gi...@apache.org>.
ryanthompson591 commented on code in PR #21868:
URL: https://github.com/apache/beam/pull/21868#discussion_r898068174
##########
sdks/python/apache_beam/ml/inference/pytorch_inference.py:
##########
@@ -99,6 +102,9 @@ def run_inference(
This method stacks the list of Tensors in a vectorized format to optimize
the inference call.
+
+ Returns:
+ An Iterable of type PredictionResult.
Review Comment:
Heejong is making a prediction result change. So I'm going to leave that for now.
--
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
[GitHub] [beam] asf-ci commented on pull request #21868: Updated documentation for ml.inference docs.
Posted by GitBox <gi...@apache.org>.
asf-ci commented on PR #21868:
URL: https://github.com/apache/beam/pull/21868#issuecomment-1155713199
Can one of the admins verify this patch?
--
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
[GitHub] [beam] yeandy commented on a diff in pull request #21868: Updated documentation for ml.inference docs.
Posted by GitBox <gi...@apache.org>.
yeandy commented on code in PR #21868:
URL: https://github.com/apache/beam/pull/21868#discussion_r898253546
##########
sdks/python/apache_beam/ml/inference/pytorch_inference.py:
##########
@@ -190,6 +194,10 @@ def run_inference(
For the same key across all examples, this will stack all Tensors values
in a vectorized format to optimize the inference call.
+ Args:
+ batch: A sequence of Tensors.
+ model: A pytorch model.
Review Comment:
```suggestion
batch: A sequence of keyed Tensors. These Tensors should be batchable, as this method will call `torch.stack()` and pass in batched Tensors with dimensions (batch_size, n_features, etc.) into the model's forward() function.
model: A PyTorch model. Must implement forward(X).
Where the parameter X is a dictionary of torch Tensors.
inference_args: Extra non-batchable arguments required as inputs to the model's forward() function. These should not be batchable, as this method will not call `torch.stack()` and pass them directly into the model's forward function()`
```
@AnandInguva Can you review my wording here?
##########
sdks/python/apache_beam/ml/inference/pytorch_inference.py:
##########
@@ -103,6 +106,13 @@ def run_inference(
This method stacks the list of Tensors in a vectorized format to optimize
the inference call.
+
+ Args:
+ batch: A sequence of Tensors.
+ model: A pytorch model.
Review Comment:
And add `inference_args` like my other comment.
##########
sdks/python/apache_beam/ml/inference/base.py:
##########
@@ -91,24 +88,43 @@ def run_inference(
batch: Sequence[ExampleT],
model: ModelT,
inference_args: Optional[Dict[str, Any]] = None) -> Iterable[PredictionT]:
- """Runs inferences on a batch of examples and
- returns an Iterable of Predictions."""
+ """Runs inferences on a batch of examples.
+
+ Args:
+ batch: A sequence of examples or features.
+ model: The model used to make inferences.
+
Review Comment:
Add
```
inference_args: Extra arguments for models whose inference call requires
extra parameters.
```
##########
sdks/python/apache_beam/ml/inference/pytorch_inference.py:
##########
@@ -103,6 +106,13 @@ def run_inference(
This method stacks the list of Tensors in a vectorized format to optimize
the inference call.
+
+ Args:
+ batch: A sequence of Tensors.
+ model: A pytorch model.
Review Comment:
```suggestion
batch: A sequence of Tensors. These Tensors should be batchable, as this method will call `torch.stack()` and pass in batched Tensors with dimensions (batch_size, n_features, etc.) into the model's forward() function.
model: A pytorch model. Must implement forward(X).
Where the parameter X is a torch Tensor.
```
--
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
[GitHub] [beam] ryanthompson591 commented on a diff in pull request #21868: Updated documentation for ml.inference docs.
Posted by GitBox <gi...@apache.org>.
ryanthompson591 commented on code in PR #21868:
URL: https://github.com/apache/beam/pull/21868#discussion_r898098349
##########
sdks/python/apache_beam/ml/inference/base.py:
##########
@@ -19,15 +19,12 @@
"""An extensible run inference transform.
-Users of this module can extend the ModelHandler class for any MLframework. Then
-pass their extended ModelHandler object into RunInference to create a
-RunInference Beam transform for that framework.
+Users of this module can extend the ModelHandler class for any machine learning
+framework. Then pass their extended ModelHandler object into RunInference to
Review Comment:
I reworked the sentence. Suggest a change if you don't like it.
--
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
[GitHub] [beam] AnandInguva commented on a diff in pull request #21868: Updated documentation for ml.inference docs.
Posted by GitBox <gi...@apache.org>.
AnandInguva commented on code in PR #21868:
URL: https://github.com/apache/beam/pull/21868#discussion_r898290184
##########
sdks/python/apache_beam/ml/inference/pytorch_inference.py:
##########
@@ -190,6 +194,10 @@ def run_inference(
For the same key across all examples, this will stack all Tensors values
in a vectorized format to optimize the inference call.
+ Args:
+ batch: A sequence of Tensors.
+ model: A pytorch model.
Review Comment:
Do we need to explicitly say `Must implement forward(X)`?
Also Users passes X as `dict` when the forward call of the model requires multiple positional arguments. If `X` is a `dict`, we unpack the positional arguments to the forward call in RunInference API by calling `model(**X)` which implicitly means `model.forward(**X)`. I think line 200 would confuse the users on this behavior.
--
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
[GitHub] [beam] yeandy commented on a diff in pull request #21868: Updated documentation for ml.inference docs.
Posted by GitBox <gi...@apache.org>.
yeandy commented on code in PR #21868:
URL: https://github.com/apache/beam/pull/21868#discussion_r898505276
##########
sdks/python/apache_beam/ml/inference/pytorch_inference.py:
##########
@@ -112,41 +128,48 @@ def run_inference(
return [PredictionResult(x, y) for x, y in zip(batch, predictions)]
def get_num_bytes(self, batch: Sequence[torch.Tensor]) -> int:
- """Returns the number of bytes of data for a batch of Tensors."""
+ """
+ Returns:
+ The number of bytes of data for a batch of Tensors.
+ """
return sum((el.element_size() for tensor in batch for el in tensor))
def get_metrics_namespace(self) -> str:
"""
- Returns a namespace for metrics collected by the RunInference transform.
+ Returns:
+ A namespace for metrics collected by the RunInference transform.
"""
return 'RunInferencePytorch'
class PytorchModelHandlerKeyedTensor(ModelHandler[Dict[str, torch.Tensor],
PredictionResult,
torch.nn.Module]):
- """ Implementation of the ModelHandler interface for PyTorch.
-
- NOTE: This API and its implementation are under development and
- do not provide backward compatibility guarantees.
- """
def __init__(
self,
state_dict_path: str,
model_class: Callable[..., torch.nn.Module],
model_params: Dict[str, Any],
device: str = 'CPU'):
- """
- Initializes a PytorchModelHandlerKeyedTensor
- :param state_dict_path: path to the saved dictionary of the model state.
- :param model_class: class of the Pytorch model that defines the model
- structure.
- :param device: the device on which you wish to run the model. If
- ``device = GPU`` then a GPU device will be used if it is available.
- Otherwise, it will be CPU.
+ """Implementation of the ModelHandler interface for PyTorch.
+
+ Example Usage:
+ pcoll | RunInference(
Review Comment:
```suggestion
Example Usage::
pcoll | RunInference(
```
##########
sdks/python/apache_beam/ml/inference/sklearn_inference.py:
##########
@@ -76,13 +77,21 @@ def _validate_inference_args(inference_args):
class SklearnModelHandlerNumpy(ModelHandler[numpy.ndarray,
PredictionResult,
BaseEstimator]):
- """ Implementation of the ModelHandler interface for scikit-learn
- using numpy arrays as input.
- """
def __init__(
self,
model_uri: str,
model_file_type: ModelFileType = ModelFileType.PICKLE):
+ """ Implementation of the ModelHandler interface for scikit-learn
+ using numpy arrays as input.
+
+ Example Usage:
+ pcoll | RunInference(SklearnModelHandlerNumpy(model_uri="my_uri"))
Review Comment:
```suggestion
Example Usage::
pcoll | RunInference(SklearnModelHandlerNumpy(model_uri="my_uri"))
```
--
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
[GitHub] [beam] asf-ci commented on pull request #21868: Updated documentation for ml.inference docs.
Posted by GitBox <gi...@apache.org>.
asf-ci commented on PR #21868:
URL: https://github.com/apache/beam/pull/21868#issuecomment-1155713194
Can one of the admins verify this patch?
--
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
[GitHub] [beam] yeandy commented on a diff in pull request #21868: Updated documentation for ml.inference docs.
Posted by GitBox <gi...@apache.org>.
yeandy commented on code in PR #21868:
URL: https://github.com/apache/beam/pull/21868#discussion_r898253546
##########
sdks/python/apache_beam/ml/inference/pytorch_inference.py:
##########
@@ -190,6 +194,10 @@ def run_inference(
For the same key across all examples, this will stack all Tensors values
in a vectorized format to optimize the inference call.
+ Args:
+ batch: A sequence of Tensors.
+ model: A pytorch model.
Review Comment:
```suggestion
batch: A sequence of keyed Tensors. These Tensors should be batchable, as this method will call `torch.stack()` and pass in batched Tensors with dimensions (batch_size, n_features, etc.) into the model's forward() function.
model: A PyTorch model.
inference_args: Non-batchable arguments required as inputs to the model's forward() function. These should not be batchable, as this method will not call `torch.stack()` and pass them directly into the model's forward function()`
```
@AnandInguva Can you review my wording here?
--
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
[GitHub] [beam] yeandy commented on a diff in pull request #21868: Updated documentation for ml.inference docs.
Posted by GitBox <gi...@apache.org>.
yeandy commented on code in PR #21868:
URL: https://github.com/apache/beam/pull/21868#discussion_r898278520
##########
sdks/python/apache_beam/ml/inference/pytorch_inference.py:
##########
@@ -103,6 +106,13 @@ def run_inference(
This method stacks the list of Tensors in a vectorized format to optimize
the inference call.
+
+ Args:
+ batch: A sequence of Tensors.
+ model: A pytorch model.
Review Comment:
```suggestion
batch: A sequence of Tensors. These Tensors should be batchable, as this method will call `torch.stack()` and pass in batched Tensors with dimensions (batch_size, n_features, etc.) into the model's forward() function.
model: A pytorch model.
```
--
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
[GitHub] [beam] ryanthompson591 commented on a diff in pull request #21868: Updated documentation for ml.inference docs.
Posted by GitBox <gi...@apache.org>.
ryanthompson591 commented on code in PR #21868:
URL: https://github.com/apache/beam/pull/21868#discussion_r898072683
##########
sdks/python/apache_beam/ml/inference/sklearn_inference.py:
##########
@@ -76,29 +85,47 @@ def load_model(self) -> BaseEstimator:
def run_inference(
self, batch: Sequence[numpy.ndarray], model: BaseEstimator,
**kwargs) -> Iterable[PredictionResult]:
+ """Runs inferences on a batch of numpy arrays.
+
Review Comment:
sure.
--
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
[GitHub] [beam] pabloem commented on pull request #21868: Updated documentation for ml.inference docs.
Posted by GitBox <gi...@apache.org>.
pabloem commented on PR #21868:
URL: https://github.com/apache/beam/pull/21868#issuecomment-1156940160
have Andy's suggestions been addressed?
--
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
[GitHub] [beam] yeandy commented on a diff in pull request #21868: Updated documentation for ml.inference docs.
Posted by GitBox <gi...@apache.org>.
yeandy commented on code in PR #21868:
URL: https://github.com/apache/beam/pull/21868#discussion_r897408161
##########
sdks/python/apache_beam/ml/inference/sklearn_inference.py:
##########
@@ -76,29 +85,47 @@ def load_model(self) -> BaseEstimator:
def run_inference(
self, batch: Sequence[numpy.ndarray], model: BaseEstimator,
**kwargs) -> Iterable[PredictionResult]:
+ """Runs inferences on a batch of numpy arrays.
+
+ Returns:
+ An Iterable of type PredictionResult.
+ """
# vectorize data for better performance
vectorized_batch = numpy.stack(batch, axis=0)
predictions = model.predict(vectorized_batch)
return [PredictionResult(x, y) for x, y in zip(batch, predictions)]
def get_num_bytes(self, batch: Sequence[pandas.DataFrame]) -> int:
- """Returns the number of bytes of data for a batch."""
+ """
+ Returns:
+ The number of bytes of data for a batch.
+ """
return sum(sys.getsizeof(element) for element in batch)
class SklearnModelHandlerPandas(ModelHandler[pandas.DataFrame,
PredictionResult,
BaseEstimator]):
- """ Implementation of the ModelHandler interface for scikit-learn that
- supports pandas dataframes.
-
- NOTE: This API and its implementation are under development and
- do not provide backward compatibility guarantees.
- """
def __init__(
self,
model_uri: str,
model_file_type: ModelFileType = ModelFileType.PICKLE):
+ """Implementation of the ModelHandler interface for scikit-learn that
+ supports pandas dataframes.
+
+ Example Usage:
+ pcol | RunInference(SklearnModelHandlerPandas(model_uri="my_uri"))
+
+ NOTE::
+ This API and its implementation are under development and
+ do not provide backward compatibility guarantees.
+
+
Review Comment:
nit: extra line?
##########
sdks/python/apache_beam/ml/inference/base.py:
##########
@@ -19,15 +19,12 @@
"""An extensible run inference transform.
-Users of this module can extend the ModelHandler class for any MLframework. Then
-pass their extended ModelHandler object into RunInference to create a
-RunInference Beam transform for that framework.
+Users of this module can extend the ModelHandler class for any machine learning
+framework. Then pass their extended ModelHandler object into RunInference to
Review Comment:
nit: grammar for "Then pass their extended..."
--
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
[GitHub] [beam] tvalentyn commented on a diff in pull request #21868: Updated documentation for ml.inference docs.
Posted by GitBox <gi...@apache.org>.
tvalentyn commented on code in PR #21868:
URL: https://github.com/apache/beam/pull/21868#discussion_r898077250
##########
sdks/python/apache_beam/ml/inference/pytorch_inference.py:
##########
@@ -99,6 +102,9 @@ def run_inference(
This method stacks the list of Tensors in a vectorized format to optimize
the inference call.
+
+ Returns:
+ An Iterable of type PredictionResult.
Review Comment:
it's already merged.
--
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
[GitHub] [beam] tvalentyn commented on a diff in pull request #21868: Updated documentation for ml.inference docs.
Posted by GitBox <gi...@apache.org>.
tvalentyn commented on code in PR #21868:
URL: https://github.com/apache/beam/pull/21868#discussion_r898053104
##########
sdks/python/apache_beam/ml/inference/pytorch_inference.py:
##########
@@ -99,6 +102,9 @@ def run_inference(
This method stacks the list of Tensors in a vectorized format to optimize
the inference call.
+
+ Returns:
+ An Iterable of type PredictionResult.
Review Comment:
we can also add a docstring for PredictionResult, for example, by assigning it to
```
PredictionResult.__doc__ = """\
...
"""
```
##########
sdks/python/apache_beam/ml/inference/pytorch_inference.py:
##########
@@ -55,21 +55,24 @@ def _convert_to_device(examples: torch.Tensor, device) -> torch.Tensor:
class PytorchModelHandlerTensor(ModelHandler[torch.Tensor,
PredictionResult,
torch.nn.Module]):
- """ Implementation of the ModelHandler interface for PyTorch."""
def __init__(
self,
state_dict_path: str,
model_class: Callable[..., torch.nn.Module],
model_params: Dict[str, Any],
device: str = 'CPU'):
- """
- Initializes a PytorchModelHandlerTensor
- :param state_dict_path: path to the saved dictionary of the model state.
- :param model_class: class of the Pytorch model that defines the model
- structure.
- :param device: the device on which you wish to run the model. If
- ``device = GPU`` then a GPU device will be used if it is available.
- Otherwise, it will be CPU.
+ """Implementation of the ModelHandler interface for PyTorch.
+
+ Example Usage:
+ pcol | RunInference(PytorchModelHandlerTensor(state_dict_path="my_uri"))
Review Comment:
(optional nit): I think `pcoll` is more common in our docs.
--
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
[GitHub] [beam] yeandy commented on pull request #21868: Updated documentation for ml.inference docs.
Posted by GitBox <gi...@apache.org>.
yeandy commented on PR #21868:
URL: https://github.com/apache/beam/pull/21868#issuecomment-1156945501
Great, they've been applied. Should be good once doc tests pass
--
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
[GitHub] [beam] ryanthompson591 commented on pull request #21868: Updated documentation for ml.inference docs.
Posted by GitBox <gi...@apache.org>.
ryanthompson591 commented on PR #21868:
URL: https://github.com/apache/beam/pull/21868#issuecomment-1155713434
R: @yeandy
R: @TheNeuralBit
R: @AnandInguva
--
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
[GitHub] [beam] TheNeuralBit merged pull request #21868: Updated documentation for ml.inference docs.
Posted by GitBox <gi...@apache.org>.
TheNeuralBit merged PR #21868:
URL: https://github.com/apache/beam/pull/21868
--
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
[GitHub] [beam] ryanthompson591 commented on a diff in pull request #21868: Updated documentation for ml.inference docs.
Posted by GitBox <gi...@apache.org>.
ryanthompson591 commented on code in PR #21868:
URL: https://github.com/apache/beam/pull/21868#discussion_r902751648
##########
sdks/python/apache_beam/ml/inference/pytorch_inference.py:
##########
@@ -55,21 +55,24 @@ def _convert_to_device(examples: torch.Tensor, device) -> torch.Tensor:
class PytorchModelHandlerTensor(ModelHandler[torch.Tensor,
PredictionResult,
torch.nn.Module]):
- """ Implementation of the ModelHandler interface for PyTorch."""
def __init__(
self,
state_dict_path: str,
model_class: Callable[..., torch.nn.Module],
model_params: Dict[str, Any],
device: str = 'CPU'):
- """
- Initializes a PytorchModelHandlerTensor
- :param state_dict_path: path to the saved dictionary of the model state.
- :param model_class: class of the Pytorch model that defines the model
- structure.
- :param device: the device on which you wish to run the model. If
- ``device = GPU`` then a GPU device will be used if it is available.
- Otherwise, it will be CPU.
+ """Implementation of the ModelHandler interface for PyTorch.
+
+ Example Usage:
+ pcoll | RunInference(PytorchModelHandlerTensor(state_dict_path="my_uri"))
Review Comment:
resolved in https://github.com/apache/beam/pull/21921
--
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
[GitHub] [beam] ryanthompson591 commented on a diff in pull request #21868: Updated documentation for ml.inference docs.
Posted by GitBox <gi...@apache.org>.
ryanthompson591 commented on code in PR #21868:
URL: https://github.com/apache/beam/pull/21868#discussion_r898091963
##########
sdks/python/apache_beam/ml/inference/sklearn_inference.py:
##########
@@ -76,29 +85,47 @@ def load_model(self) -> BaseEstimator:
def run_inference(
self, batch: Sequence[numpy.ndarray], model: BaseEstimator,
**kwargs) -> Iterable[PredictionResult]:
+ """Runs inferences on a batch of numpy arrays.
+
+ Returns:
+ An Iterable of type PredictionResult.
+ """
# vectorize data for better performance
vectorized_batch = numpy.stack(batch, axis=0)
predictions = model.predict(vectorized_batch)
return [PredictionResult(x, y) for x, y in zip(batch, predictions)]
def get_num_bytes(self, batch: Sequence[pandas.DataFrame]) -> int:
- """Returns the number of bytes of data for a batch."""
+ """
+ Returns:
+ The number of bytes of data for a batch.
+ """
return sum(sys.getsizeof(element) for element in batch)
class SklearnModelHandlerPandas(ModelHandler[pandas.DataFrame,
PredictionResult,
BaseEstimator]):
- """ Implementation of the ModelHandler interface for scikit-learn that
- supports pandas dataframes.
-
- NOTE: This API and its implementation are under development and
- do not provide backward compatibility guarantees.
- """
def __init__(
self,
model_uri: str,
model_file_type: ModelFileType = ModelFileType.PICKLE):
+ """Implementation of the ModelHandler interface for scikit-learn that
+ supports pandas dataframes.
+
+ Example Usage:
+ pcol | RunInference(SklearnModelHandlerPandas(model_uri="my_uri"))
+
+ NOTE::
+ This API and its implementation are under development and
+ do not provide backward compatibility guarantees.
+
+
Review Comment:
Done.
--
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
[GitHub] [beam] TheNeuralBit commented on pull request #21868: Updated documentation for ml.inference docs.
Posted by GitBox <gi...@apache.org>.
TheNeuralBit commented on PR #21868:
URL: https://github.com/apache/beam/pull/21868#issuecomment-1157078267
PreCommit failures are unrelated flakes.
--
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
[GitHub] [beam] TheNeuralBit commented on a diff in pull request #21868: Updated documentation for ml.inference docs.
Posted by GitBox <gi...@apache.org>.
TheNeuralBit commented on code in PR #21868:
URL: https://github.com/apache/beam/pull/21868#discussion_r898151499
##########
sdks/python/apache_beam/ml/inference/pytorch_inference.py:
##########
@@ -55,21 +55,24 @@ def _convert_to_device(examples: torch.Tensor, device) -> torch.Tensor:
class PytorchModelHandlerTensor(ModelHandler[torch.Tensor,
PredictionResult,
torch.nn.Module]):
- """ Implementation of the ModelHandler interface for PyTorch."""
def __init__(
self,
state_dict_path: str,
model_class: Callable[..., torch.nn.Module],
model_params: Dict[str, Any],
device: str = 'CPU'):
- """
- Initializes a PytorchModelHandlerTensor
- :param state_dict_path: path to the saved dictionary of the model state.
- :param model_class: class of the Pytorch model that defines the model
- structure.
- :param device: the device on which you wish to run the model. If
- ``device = GPU`` then a GPU device will be used if it is available.
- Otherwise, it will be CPU.
+ """Implementation of the ModelHandler interface for PyTorch.
+
+ Example Usage:
+ pcoll | RunInference(PytorchModelHandlerTensor(state_dict_path="my_uri"))
Review Comment:
(similarly for other examples)
##########
sdks/python/apache_beam/ml/inference/pytorch_inference.py:
##########
@@ -55,21 +55,24 @@ def _convert_to_device(examples: torch.Tensor, device) -> torch.Tensor:
class PytorchModelHandlerTensor(ModelHandler[torch.Tensor,
PredictionResult,
torch.nn.Module]):
- """ Implementation of the ModelHandler interface for PyTorch."""
def __init__(
self,
state_dict_path: str,
model_class: Callable[..., torch.nn.Module],
model_params: Dict[str, Any],
device: str = 'CPU'):
- """
- Initializes a PytorchModelHandlerTensor
- :param state_dict_path: path to the saved dictionary of the model state.
- :param model_class: class of the Pytorch model that defines the model
- structure.
- :param device: the device on which you wish to run the model. If
- ``device = GPU`` then a GPU device will be used if it is available.
- Otherwise, it will be CPU.
+ """Implementation of the ModelHandler interface for PyTorch.
+
+ Example Usage:
+ pcoll | RunInference(PytorchModelHandlerTensor(state_dict_path="my_uri"))
Review Comment:
nit: I think this needs a newline and an extra colon to render as a code block, see for example https://github.com/apache/beam/blob/master/sdks/python/apache_beam/dataframe/convert.py#L50-L55
```suggestion
Example Usage::
pcoll | RunInference(PytorchModelHandlerTensor(state_dict_path="my_uri"))
```
--
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
[GitHub] [beam] asf-ci commented on pull request #21868: Updated documentation for ml.inference docs.
Posted by GitBox <gi...@apache.org>.
asf-ci commented on PR #21868:
URL: https://github.com/apache/beam/pull/21868#issuecomment-1155713198
Can one of the admins verify this patch?
--
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
[GitHub] [beam] tvalentyn commented on a diff in pull request #21868: Updated documentation for ml.inference docs.
Posted by GitBox <gi...@apache.org>.
tvalentyn commented on code in PR #21868:
URL: https://github.com/apache/beam/pull/21868#discussion_r898953768
##########
sdks/python/apache_beam/ml/inference/pytorch_inference.py:
##########
@@ -99,6 +102,9 @@ def run_inference(
This method stacks the list of Tensors in a vectorized format to optimize
the inference call.
+
+ Returns:
+ An Iterable of type PredictionResult.
Review Comment:
Specifically, https://github.com/apache/beam/pull/17773 was the PR and documentation was not included.
--
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
[GitHub] [beam] ryanthompson591 commented on a diff in pull request #21868: Updated documentation for ml.inference docs.
Posted by GitBox <gi...@apache.org>.
ryanthompson591 commented on code in PR #21868:
URL: https://github.com/apache/beam/pull/21868#discussion_r902751289
##########
sdks/python/apache_beam/ml/inference/base.py:
##########
@@ -91,24 +88,43 @@ def run_inference(
batch: Sequence[ExampleT],
model: ModelT,
inference_args: Optional[Dict[str, Any]] = None) -> Iterable[PredictionT]:
- """Runs inferences on a batch of examples and
- returns an Iterable of Predictions."""
+ """Runs inferences on a batch of examples.
+
+ Args:
+ batch: A sequence of examples or features.
+ model: The model used to make inferences.
+
Review Comment:
resolved in https://github.com/apache/beam/pull/21921
--
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
[GitHub] [beam] tvalentyn commented on a diff in pull request #21868: Updated documentation for ml.inference docs.
Posted by GitBox <gi...@apache.org>.
tvalentyn commented on code in PR #21868:
URL: https://github.com/apache/beam/pull/21868#discussion_r897897894
##########
sdks/python/apache_beam/ml/inference/base.py:
##########
@@ -86,24 +83,38 @@ def load_model(self) -> ModelT:
def run_inference(self, batch: Sequence[ExampleT], model: ModelT,
**kwargs) -> Iterable[PredictionT]:
- """Runs inferences on a batch of examples and
- returns an Iterable of Predictions."""
+ """Runs inferences on a batch of examples.
Review Comment:
Can we document inputs as well?
(also, need to resolve conflicts here and update accordingly).
--
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
[GitHub] [beam] ryanthompson591 commented on a diff in pull request #21868: Updated documentation for ml.inference docs.
Posted by GitBox <gi...@apache.org>.
ryanthompson591 commented on code in PR #21868:
URL: https://github.com/apache/beam/pull/21868#discussion_r898068684
##########
sdks/python/apache_beam/ml/inference/pytorch_inference.py:
##########
@@ -55,21 +55,24 @@ def _convert_to_device(examples: torch.Tensor, device) -> torch.Tensor:
class PytorchModelHandlerTensor(ModelHandler[torch.Tensor,
PredictionResult,
torch.nn.Module]):
- """ Implementation of the ModelHandler interface for PyTorch."""
def __init__(
self,
state_dict_path: str,
model_class: Callable[..., torch.nn.Module],
model_params: Dict[str, Any],
device: str = 'CPU'):
- """
- Initializes a PytorchModelHandlerTensor
- :param state_dict_path: path to the saved dictionary of the model state.
- :param model_class: class of the Pytorch model that defines the model
- structure.
- :param device: the device on which you wish to run the model. If
- ``device = GPU`` then a GPU device will be used if it is available.
- Otherwise, it will be CPU.
+ """Implementation of the ModelHandler interface for PyTorch.
+
+ Example Usage:
+ pcol | RunInference(PytorchModelHandlerTensor(state_dict_path="my_uri"))
Review Comment:
ok will change that here and everywhere else.
--
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
[GitHub] [beam] ryanthompson591 commented on a diff in pull request #21868: Updated documentation for ml.inference docs.
Posted by GitBox <gi...@apache.org>.
ryanthompson591 commented on code in PR #21868:
URL: https://github.com/apache/beam/pull/21868#discussion_r898091380
##########
sdks/python/apache_beam/ml/inference/pytorch_inference.py:
##########
@@ -170,6 +183,9 @@ def run_inference(
For the same key across all examples, this will stack all Tensors values
in a vectorized format to optimize the inference call.
+
Review Comment:
Done. @yeandy I've made these inputs fairly light. I was able to make the sklearn inputs more specific. Is there any specificity you recommend for these parameters?
--
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
[GitHub] [beam] AnandInguva commented on pull request #21868: Updated documentation for ml.inference docs.
Posted by GitBox <gi...@apache.org>.
AnandInguva commented on PR #21868:
URL: https://github.com/apache/beam/pull/21868#issuecomment-1156876596
retest this please
--
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
[GitHub] [beam] tvalentyn commented on pull request #21868: Updated documentation for ml.inference docs.
Posted by GitBox <gi...@apache.org>.
tvalentyn commented on PR #21868:
URL: https://github.com/apache/beam/pull/21868#issuecomment-1157448947
> Unfortunate that these were missed, but let's do these in a follow-up PR just to make sure we get the basic updates into the release cut. We can probably cherrypick these minor changes
To make sure we cover all bases, @ryanthompson591 go over the unresolved comments and mark them either as Done or Done in PR # 1234 (the new PR Brian suggests to make). 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
[GitHub] [beam] yeandy commented on a diff in pull request #21868: Updated documentation for ml.inference docs.
Posted by GitBox <gi...@apache.org>.
yeandy commented on code in PR #21868:
URL: https://github.com/apache/beam/pull/21868#discussion_r898299214
##########
sdks/python/apache_beam/ml/inference/pytorch_inference.py:
##########
@@ -190,6 +194,10 @@ def run_inference(
For the same key across all examples, this will stack all Tensors values
in a vectorized format to optimize the inference call.
+ Args:
+ batch: A sequence of Tensors.
+ model: A pytorch model.
Review Comment:
Anand and I review this. @ryanthompson591 Not sure if/when you are reviewing my suggestions, but they have just been updated.
##########
sdks/python/apache_beam/ml/inference/pytorch_inference.py:
##########
@@ -103,6 +106,13 @@ def run_inference(
This method stacks the list of Tensors in a vectorized format to optimize
the inference call.
+
+ Args:
+ batch: A sequence of Tensors.
+ model: A pytorch model.
Review Comment:
Anand and I review this. @ryanthompson591 Not sure if/when you are reviewing my suggestions, but they have just been updated.
--
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
[GitHub] [beam] yeandy commented on a diff in pull request #21868: Updated documentation for ml.inference docs.
Posted by GitBox <gi...@apache.org>.
yeandy commented on code in PR #21868:
URL: https://github.com/apache/beam/pull/21868#discussion_r898278520
##########
sdks/python/apache_beam/ml/inference/pytorch_inference.py:
##########
@@ -103,6 +106,13 @@ def run_inference(
This method stacks the list of Tensors in a vectorized format to optimize
the inference call.
+
+ Args:
+ batch: A sequence of Tensors.
+ model: A pytorch model.
Review Comment:
```suggestion
batch: A sequence of Tensors. These Tensors should be batchable, as this method will call `torch.stack()` and pass in batched Tensors with dimensions (batch_size, n_features, etc.) into the model's forward() function.
model: A pytorch model.
inference_args: Non-batchable arguments required as inputs to the model's forward() function. Unlike Tensors in `batch`, these parameters will not be dynamically batched
```
--
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