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/02/22 14:15:44 UTC

[GitHub] [beam] yeandy opened a new pull request #16917: [BEAM-13972] Add RunInference interface

yeandy opened a new pull request #16917:
URL: https://github.com/apache/beam/pull/16917


   Adding common interface to the `RunInference` transform. Arguments will be wrapper classes around `PyTorch` and `Scikit-learn` models, along with other common parameters.
   
   ------------------------
   
   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`).
    - [ ] Format the pull request title like `[BEAM-XXX] Fixes bug in ApproximateQuantiles`, where you replace `BEAM-XXX` with the appropriate JIRA issue, if applicable. This will automatically link the pull request to the issue.
    - [ ] 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/validate-runner/README.md)
   
   GitHub Actions Tests Status (on master branch)
   ------------------------------------------------------------------------------------------------
   [![Build python source distribution and wheels](https://github.com/apache/beam/workflows/Build%20python%20source%20distribution%20and%20wheels/badge.svg?branch=master&event=schedule)](https://github.com/apache/beam/actions?query=workflow%3A%22Build+python+source+distribution+and+wheels%22+branch%3Amaster+event%3Aschedule)
   [![Python tests](https://github.com/apache/beam/workflows/Python%20tests/badge.svg?branch=master&event=schedule)](https://github.com/apache/beam/actions?query=workflow%3A%22Python+Tests%22+branch%3Amaster+event%3Aschedule)
   [![Java tests](https://github.com/apache/beam/workflows/Java%20Tests/badge.svg?branch=master&event=schedule)](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] ryanthompson591 commented on a change in pull request #16917: [BEAM-13972] Add RunInference interface

Posted by GitBox <gi...@apache.org>.
ryanthompson591 commented on a change in pull request #16917:
URL: https://github.com/apache/beam/pull/16917#discussion_r818729200



##########
File path: sdks/python/apache_beam/ml/inference/api.py
##########
@@ -0,0 +1,84 @@
+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements.  See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License.  You may obtain a copy of the License at
+#
+#    http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+
+from dataclasses import dataclass
+from enum import Enum
+from typing import Tuple, TypeVar, Union
+
+import apache_beam as beam
+
+
+class PyTorchDevice(Enum):
+  CPU = 1
+  GPU = 2
+
+
+class SklearnSerializationType(Enum):
+  PICKLE = 1
+  JOBLIB = 2
+
+
+@dataclass
+class BaseModelSpec:
+  model_url: str
+
+
+@dataclass
+class PyTorchModelSpec(BaseModelSpec):
+  device: PyTorchDevice
+
+
+@dataclass
+class SklearnModelSpec(BaseModelSpec):
+  serialization_type: SklearnSerializationType
+
+
+_K = TypeVar('_K')
+_INPUT_TYPE = TypeVar('_INPUT_TYPE')
+_OUTPUT_TYPE = TypeVar('_OUTPUT_TYPE')
+
+
+@dataclass
+class PredictionResult:
+  example: _INPUT_TYPE
+  inference: _OUTPUT_TYPE
+
+
+@beam.ptransform_fn
+@beam.typehints.with_input_types(Union[_INPUT_TYPE, Tuple[_K, _INPUT_TYPE]])
+@beam.typehints.with_output_types(
+    Union[PredictionResult, Tuple[_K, PredictionResult]])
+def RunInference(
+    examples: beam.pvalue.PCollection,
+    model: BaseModelSpec) -> beam.pvalue.PCollection:
+  """Run inference with a model.
+
+  There one type of inference you can perform using this PTransform:
+    1. In-process inference from a SavedModel instance.

Review comment:
       I'm not sure this will be true once we start implementing this interface. Ideally one of the earlier things we should do is wrap the TFX implementation which will use a service like Vertex AI.

##########
File path: sdks/python/apache_beam/ml/inference/api.py
##########
@@ -0,0 +1,84 @@
+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements.  See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License.  You may obtain a copy of the License at
+#
+#    http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+
+from dataclasses import dataclass
+import apache_beam as beam
+from typing import Tuple, TypeVar, Union
+# TODO: implement RunInferenceImpl
+# from apache_beam.ml.inference.base import RunInferenceImpl
+
+
+@dataclass
+class BaseModelSpec:
+  model_url: str
+
+
+@dataclass
+class PyTorchModelSpec(BaseModelSpec):

Review comment:
       It's fine, though, in general I prefer shorter names where they are adequately precise.

##########
File path: sdks/python/apache_beam/ml/inference/api.py
##########
@@ -0,0 +1,84 @@
+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements.  See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License.  You may obtain a copy of the License at
+#
+#    http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+
+from dataclasses import dataclass
+from enum import Enum
+from typing import Tuple, TypeVar, Union
+
+import apache_beam as beam
+
+
+class PyTorchDevice(Enum):
+  CPU = 1
+  GPU = 2
+
+
+class SklearnSerializationType(Enum):
+  PICKLE = 1
+  JOBLIB = 2
+
+
+@dataclass
+class BaseModelSpec:
+  model_url: str
+
+
+@dataclass
+class PyTorchModelSpec(BaseModelSpec):
+  device: PyTorchDevice
+
+
+@dataclass
+class SklearnModelSpec(BaseModelSpec):
+  serialization_type: SklearnSerializationType
+
+
+_K = TypeVar('_K')
+_INPUT_TYPE = TypeVar('_INPUT_TYPE')
+_OUTPUT_TYPE = TypeVar('_OUTPUT_TYPE')
+
+
+@dataclass
+class PredictionResult:
+  example: _INPUT_TYPE
+  inference: _OUTPUT_TYPE
+
+
+@beam.ptransform_fn
+@beam.typehints.with_input_types(Union[_INPUT_TYPE, Tuple[_K, _INPUT_TYPE]])
+@beam.typehints.with_output_types(
+    Union[PredictionResult, Tuple[_K, PredictionResult]])
+def RunInference(
+    examples: beam.pvalue.PCollection,
+    model: BaseModelSpec) -> beam.pvalue.PCollection:
+  """Run inference with a model.

Review comment:
       Getting this API doc bogged down in the details of what is implemented at any given time will probably have the effect of letting it get out of date and causing confusion. 
   
   I suggest instead we just clearly define what this does without getting into any details about what is yet supported and what our plans are.
   
   Here's a suggestion:
   
   A transform that takes a pcollection of examples (or features) to be used on an ML model. It will then output inferences (or predictions) for those examples in a pcollection of PredictionResults, containing the input examples and output inferences.
   
   If examples are paired with keys, it will output a tuple (key, PredictionResult) for each (key, example) input.
   
   Models for supported frameworks can be loaded via a URI. Supported services can also be used.
   
   TODO: link to a help page that shows what is/isn't supported.




-- 
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] edited a comment on pull request #16917: [BEAM-13972] Add RunInference interface

Posted by GitBox <gi...@apache.org>.
codecov[bot] edited a comment on pull request #16917:
URL: https://github.com/apache/beam/pull/16917#issuecomment-1047862344


   # [Codecov](https://codecov.io/gh/apache/beam/pull/16917?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 [#16917](https://codecov.io/gh/apache/beam/pull/16917?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (e5c18d9) into [master](https://codecov.io/gh/apache/beam/commit/b2f21281b94dc7182d15611a3e6bf1d872ea3205?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (b2f2128) will **increase** coverage by `36.73%`.
   > The diff coverage is `96.96%`.
   
   > :exclamation: Current head e5c18d9 differs from pull request most recent head 71e2f8c. Consider uploading reports for the commit 71e2f8c to get more accurate results
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/beam/pull/16917/graphs/tree.svg?width=650&height=150&src=pr&token=qcbbAh8Fj1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)](https://codecov.io/gh/apache/beam/pull/16917?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   
   ```diff
   @@             Coverage Diff             @@
   ##           master   #16917       +/-   ##
   ===========================================
   + Coverage   46.89%   83.63%   +36.73%     
   ===========================================
     Files         204      454      +250     
     Lines       20122    62595    +42473     
   ===========================================
   + Hits         9436    52349    +42913     
   - Misses       9686    10246      +560     
   + Partials     1000        0     -1000     
   ```
   
   | Flag | Coverage Δ | |
   |---|---|---|
   | python | `83.63% <96.96%> (?)` | |
   
   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/16917?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
   |---|---|---|
   | [sdks/python/apache\_beam/ml/inference/api.py](https://codecov.io/gh/apache/beam/pull/16917/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-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vbWwvaW5mZXJlbmNlL2FwaS5weQ==) | `96.96% <96.96%> (ø)` | |
   | [sdks/go/pkg/beam/core/runtime/exec/reshuffle.go](https://codecov.io/gh/apache/beam/pull/16917/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-c2Rrcy9nby9wa2cvYmVhbS9jb3JlL3J1bnRpbWUvZXhlYy9yZXNodWZmbGUuZ28=) | | |
   | [sdks/go/pkg/beam/core/runtime/metricsx/urns.go](https://codecov.io/gh/apache/beam/pull/16917/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-c2Rrcy9nby9wa2cvYmVhbS9jb3JlL3J1bnRpbWUvbWV0cmljc3gvdXJucy5nbw==) | | |
   | [sdks/go/pkg/beam/transforms/stats/count.go](https://codecov.io/gh/apache/beam/pull/16917/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-c2Rrcy9nby9wa2cvYmVhbS90cmFuc2Zvcm1zL3N0YXRzL2NvdW50Lmdv) | | |
   | [sdks/go/pkg/beam/core/funcx/sideinput.go](https://codecov.io/gh/apache/beam/pull/16917/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-c2Rrcy9nby9wa2cvYmVhbS9jb3JlL2Z1bmN4L3NpZGVpbnB1dC5nbw==) | | |
   | [sdks/go/pkg/beam/core/runtime/options.go](https://codecov.io/gh/apache/beam/pull/16917/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-c2Rrcy9nby9wa2cvYmVhbS9jb3JlL3J1bnRpbWUvb3B0aW9ucy5nbw==) | | |
   | [sdks/go/pkg/beam/core/runtime/exec/status.go](https://codecov.io/gh/apache/beam/pull/16917/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-c2Rrcy9nby9wa2cvYmVhbS9jb3JlL3J1bnRpbWUvZXhlYy9zdGF0dXMuZ28=) | | |
   | [sdks/go/pkg/beam/core/typex/class.go](https://codecov.io/gh/apache/beam/pull/16917/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-c2Rrcy9nby9wa2cvYmVhbS9jb3JlL3R5cGV4L2NsYXNzLmdv) | | |
   | [sdks/go/pkg/beam/core/runtime/exec/decode.go](https://codecov.io/gh/apache/beam/pull/16917/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-c2Rrcy9nby9wa2cvYmVhbS9jb3JlL3J1bnRpbWUvZXhlYy9kZWNvZGUuZ28=) | | |
   | [sdks/go/pkg/beam/impulse.go](https://codecov.io/gh/apache/beam/pull/16917/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-c2Rrcy9nby9wa2cvYmVhbS9pbXB1bHNlLmdv) | | |
   | ... and [648 more](https://codecov.io/gh/apache/beam/pull/16917/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/16917?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/16917?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 [b2f2128...71e2f8c](https://codecov.io/gh/apache/beam/pull/16917?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 pull request #16917: [BEAM-13972] Add RunInference interface

Posted by GitBox <gi...@apache.org>.
yeandy commented on pull request #16917:
URL: https://github.com/apache/beam/pull/16917#issuecomment-1057209248


   R: @TheNeuralBit and @tvalentyn 


-- 
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 change in pull request #16917: [BEAM-13972] Add RunInference interface

Posted by GitBox <gi...@apache.org>.
yeandy commented on a change in pull request #16917:
URL: https://github.com/apache/beam/pull/16917#discussion_r818141119



##########
File path: sdks/python/apache_beam/ml/inference/api_test.py
##########
@@ -0,0 +1,54 @@
+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements.  See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License.  You may obtain a copy of the License at
+#
+#    http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+
+import unittest
+from apache_beam.ml.inference.api import PyTorchDevice
+from apache_beam.ml.inference.api import PyTorchModelSpec
+from apache_beam.ml.inference.api import SklearnSerializationType
+from apache_beam.ml.inference.api import SklearnModelSpec
+import pytest  # pylint: disable=unused-import
+
+
+class RunInferenceTest(unittest.TestCase):
+  def test_valid_pytorch_model(self):
+    model = PyTorchModelSpec(
+        model_url='pytorch_model.pth', device=PyTorchDevice.GPU)
+    assert model.model_url == 'pytorch_model.pth'
+    assert model.device == PyTorchDevice.GPU
+
+  def test_invalid_pytorch_model(self):
+    with pytest.raises(TypeError, match='device'):
+      PyTorchModelSpec(model_url='pytorch_model.pth')
+    with pytest.raises(TypeError, match='model_url'):
+      PyTorchModelSpec(device=PyTorchDevice.CPU)
+
+  def test_valid_sklearn_model(self):
+    model = SklearnModelSpec(
+        model_url='sklearn_model.pickle',
+        serialization_type=SklearnSerializationType.PICKLE)
+    assert model.model_url == 'sklearn_model.pickle'

Review comment:
       Thanks, fixed!




-- 
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 change in pull request #16917: [BEAM-13972] Add RunInference interface

Posted by GitBox <gi...@apache.org>.
yeandy commented on a change in pull request #16917:
URL: https://github.com/apache/beam/pull/16917#discussion_r817201017



##########
File path: sdks/python/apache_beam/ml/inference/api.py
##########
@@ -0,0 +1,84 @@
+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements.  See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License.  You may obtain a copy of the License at
+#
+#    http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+
+from dataclasses import dataclass
+import apache_beam as beam
+from typing import Tuple, TypeVar, Union
+# TODO: implement RunInferenceImpl
+# from apache_beam.ml.inference.base import RunInferenceImpl
+
+
+@dataclass
+class BaseModelSpec:
+  model_url: str
+
+
+@dataclass
+class PyTorchModel(BaseModelSpec):
+  device: str
+
+  def __post_init__(self):
+    self.device = self.device.upper()
+
+
+@dataclass
+class SklearnModel(BaseModelSpec):
+  serialization_type: str
+
+  def __post_init__(self):
+    self.serialization_type = self.serialization_type.upper()
+
+
+_K = TypeVar('_K')
+_INPUT_TYPE = TypeVar('_INPUT_TYPE')
+_OUTPUT_TYPE = TypeVar('_OUTPUT_TYPE')
+
+
+@dataclass
+class PredictionResult:
+  key: _K
+  example: _INPUT_TYPE
+  inference: _OUTPUT_TYPE
+
+
+@beam.ptransform_fn
+@beam.typehints.with_input_types(Union[_INPUT_TYPE, Tuple[_K, _INPUT_TYPE]])
+@beam.typehints.with_output_types(PredictionResult)

Review comment:
       How should we deal with typing? TFX [explicitly](https://github.com/tensorflow/tfx-bsl/blob/v1.6.0/tfx_bsl/public/beam/run_inference.py#L34) defines output as `Union[_OUTPUT_TYPE, Tuple[_K, _OUTPUT_TYPE]]`




-- 
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 change in pull request #16917: [BEAM-13972] Add RunInference interface

Posted by GitBox <gi...@apache.org>.
yeandy commented on a change in pull request #16917:
URL: https://github.com/apache/beam/pull/16917#discussion_r821021833



##########
File path: sdks/python/apache_beam/ml/inference/api.py
##########
@@ -0,0 +1,84 @@
+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements.  See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License.  You may obtain a copy of the License at
+#
+#    http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+
+from dataclasses import dataclass
+import apache_beam as beam
+from typing import Tuple, TypeVar, Union
+# TODO: implement RunInferenceImpl
+# from apache_beam.ml.inference.base import RunInferenceImpl
+
+
+@dataclass
+class BaseModelSpec:
+  model_url: str
+
+
+@dataclass
+class PyTorchModelSpec(BaseModelSpec):

Review comment:
       Sounds good, 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 change in pull request #16917: [BEAM-13972] Add RunInference interface

Posted by GitBox <gi...@apache.org>.
yeandy commented on a change in pull request #16917:
URL: https://github.com/apache/beam/pull/16917#discussion_r818123437



##########
File path: sdks/python/apache_beam/ml/inference/api.py
##########
@@ -0,0 +1,84 @@
+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements.  See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License.  You may obtain a copy of the License at
+#
+#    http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+
+from dataclasses import dataclass
+import apache_beam as beam
+from typing import Tuple, TypeVar, Union
+# TODO: implement RunInferenceImpl
+# from apache_beam.ml.inference.base import RunInferenceImpl
+
+
+@dataclass
+class BaseModelSpec:
+  model_url: str
+
+
+@dataclass
+class PyTorchModel(BaseModelSpec):
+  device: str
+
+  def __post_init__(self):
+    self.device = self.device.upper()
+
+
+@dataclass
+class SklearnModel(BaseModelSpec):
+  serialization_type: str
+
+  def __post_init__(self):
+    self.serialization_type = self.serialization_type.upper()
+
+
+_K = TypeVar('_K')
+_INPUT_TYPE = TypeVar('_INPUT_TYPE')
+_OUTPUT_TYPE = TypeVar('_OUTPUT_TYPE')
+
+
+@dataclass
+class PredictionResult:
+  key: _K
+  example: _INPUT_TYPE
+  inference: _OUTPUT_TYPE
+
+
+@beam.ptransform_fn
+@beam.typehints.with_input_types(Union[_INPUT_TYPE, Tuple[_K, _INPUT_TYPE]])
+@beam.typehints.with_output_types(PredictionResult)

Review comment:
       I've modified the output type and pushed the change. I feel like we should try to get typing as accurate as possible early on, especially if we want to ensure that we can accommodate TFX's implementation.
   
   Could we have, instead of a bare `PredictionResult` type, a parent class from which `PredictionResult` and `prediction_log_pb2.PredictionLog` inherit? Or use a `Union`? (but this may get messy).
   
   This definitely requires input from someone with more experience with typing.




-- 
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] edited a comment on pull request #16917: [BEAM-13972] Add RunInference interface

Posted by GitBox <gi...@apache.org>.
codecov[bot] edited a comment on pull request #16917:
URL: https://github.com/apache/beam/pull/16917#issuecomment-1047862344


   # [Codecov](https://codecov.io/gh/apache/beam/pull/16917?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 [#16917](https://codecov.io/gh/apache/beam/pull/16917?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (4280dfc) into [master](https://codecov.io/gh/apache/beam/commit/b2f21281b94dc7182d15611a3e6bf1d872ea3205?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (b2f2128) will **increase** coverage by `36.74%`.
   > The diff coverage is `96.77%`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/beam/pull/16917/graphs/tree.svg?width=650&height=150&src=pr&token=qcbbAh8Fj1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)](https://codecov.io/gh/apache/beam/pull/16917?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   
   ```diff
   @@             Coverage Diff             @@
   ##           master   #16917       +/-   ##
   ===========================================
   + Coverage   46.89%   83.63%   +36.74%     
   ===========================================
     Files         204      454      +250     
     Lines       20122    62591    +42469     
   ===========================================
   + Hits         9436    52348    +42912     
   - Misses       9686    10243      +557     
   + Partials     1000        0     -1000     
   ```
   
   | Flag | Coverage Δ | |
   |---|---|---|
   | python | `83.63% <96.77%> (?)` | |
   
   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/16917?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
   |---|---|---|
   | [sdks/python/apache\_beam/ml/inference/api.py](https://codecov.io/gh/apache/beam/pull/16917/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-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vbWwvaW5mZXJlbmNlL2FwaS5weQ==) | `96.77% <96.77%> (ø)` | |
   | [sdks/go/pkg/beam/core/graph/coder/iterable.go](https://codecov.io/gh/apache/beam/pull/16917/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-c2Rrcy9nby9wa2cvYmVhbS9jb3JlL2dyYXBoL2NvZGVyL2l0ZXJhYmxlLmdv) | | |
   | [sdks/go/pkg/beam/io/datastoreio/datastore.go](https://codecov.io/gh/apache/beam/pull/16917/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-c2Rrcy9nby9wa2cvYmVhbS9pby9kYXRhc3RvcmVpby9kYXRhc3RvcmUuZ28=) | | |
   | [sdks/go/pkg/beam/core/graph/coder/registry.go](https://codecov.io/gh/apache/beam/pull/16917/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-c2Rrcy9nby9wa2cvYmVhbS9jb3JlL2dyYXBoL2NvZGVyL3JlZ2lzdHJ5Lmdv) | | |
   | [sdks/go/pkg/beam/core/metrics/metrics.go](https://codecov.io/gh/apache/beam/pull/16917/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-c2Rrcy9nby9wa2cvYmVhbS9jb3JlL21ldHJpY3MvbWV0cmljcy5nbw==) | | |
   | [...s/go/pkg/beam/core/runtime/graphx/schema/schema.go](https://codecov.io/gh/apache/beam/pull/16917/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-c2Rrcy9nby9wa2cvYmVhbS9jb3JlL3J1bnRpbWUvZ3JhcGh4L3NjaGVtYS9zY2hlbWEuZ28=) | | |
   | [sdks/go/pkg/beam/core/graph/coder/bytes.go](https://codecov.io/gh/apache/beam/pull/16917/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-c2Rrcy9nby9wa2cvYmVhbS9jb3JlL2dyYXBoL2NvZGVyL2J5dGVzLmdv) | | |
   | [sdks/go/pkg/beam/core/util/reflectx/util.go](https://codecov.io/gh/apache/beam/pull/16917/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-c2Rrcy9nby9wa2cvYmVhbS9jb3JlL3V0aWwvcmVmbGVjdHgvdXRpbC5nbw==) | | |
   | [sdks/go/pkg/beam/core/runtime/exec/translate.go](https://codecov.io/gh/apache/beam/pull/16917/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-c2Rrcy9nby9wa2cvYmVhbS9jb3JlL3J1bnRpbWUvZXhlYy90cmFuc2xhdGUuZ28=) | | |
   | [sdks/go/pkg/beam/core/graph/coder/panes.go](https://codecov.io/gh/apache/beam/pull/16917/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-c2Rrcy9nby9wa2cvYmVhbS9jb3JlL2dyYXBoL2NvZGVyL3BhbmVzLmdv) | | |
   | ... and [648 more](https://codecov.io/gh/apache/beam/pull/16917/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/16917?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/16917?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 [b2f2128...4280dfc](https://codecov.io/gh/apache/beam/pull/16917?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] codecov[bot] edited a comment on pull request #16917: [BEAM-13972] Add RunInference interface

Posted by GitBox <gi...@apache.org>.
codecov[bot] edited a comment on pull request #16917:
URL: https://github.com/apache/beam/pull/16917#issuecomment-1047862344


   # [Codecov](https://codecov.io/gh/apache/beam/pull/16917?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 [#16917](https://codecov.io/gh/apache/beam/pull/16917?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (f73b890) into [master](https://codecov.io/gh/apache/beam/commit/b2f21281b94dc7182d15611a3e6bf1d872ea3205?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (b2f2128) will **increase** coverage by `36.74%`.
   > The diff coverage is `n/a`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/beam/pull/16917/graphs/tree.svg?width=650&height=150&src=pr&token=qcbbAh8Fj1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)](https://codecov.io/gh/apache/beam/pull/16917?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   
   ```diff
   @@             Coverage Diff             @@
   ##           master   #16917       +/-   ##
   ===========================================
   + Coverage   46.89%   83.64%   +36.74%     
   ===========================================
     Files         204      453      +249     
     Lines       20122    62471    +42349     
   ===========================================
   + Hits         9436    52251    +42815     
   - Misses       9686    10220      +534     
   + Partials     1000        0     -1000     
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/beam/pull/16917?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
   |---|---|---|
   | [sdks/go/pkg/beam/create.go](https://codecov.io/gh/apache/beam/pull/16917/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-c2Rrcy9nby9wa2cvYmVhbS9jcmVhdGUuZ28=) | | |
   | [sdks/go/pkg/beam/io/datastoreio/datastore.go](https://codecov.io/gh/apache/beam/pull/16917/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-c2Rrcy9nby9wa2cvYmVhbS9pby9kYXRhc3RvcmVpby9kYXRhc3RvcmUuZ28=) | | |
   | [sdks/go/pkg/beam/testing/passert/sum.go](https://codecov.io/gh/apache/beam/pull/16917/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-c2Rrcy9nby9wa2cvYmVhbS90ZXN0aW5nL3Bhc3NlcnQvc3VtLmdv) | | |
   | [sdks/go/pkg/beam/core/graph/coder/map.go](https://codecov.io/gh/apache/beam/pull/16917/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-c2Rrcy9nby9wa2cvYmVhbS9jb3JlL2dyYXBoL2NvZGVyL21hcC5nbw==) | | |
   | [sdks/go/pkg/beam/core/funcx/fn.go](https://codecov.io/gh/apache/beam/pull/16917/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-c2Rrcy9nby9wa2cvYmVhbS9jb3JlL2Z1bmN4L2ZuLmdv) | | |
   | [sdks/go/pkg/beam/util/gcsx/gcs.go](https://codecov.io/gh/apache/beam/pull/16917/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-c2Rrcy9nby9wa2cvYmVhbS91dGlsL2djc3gvZ2NzLmdv) | | |
   | [sdks/go/pkg/beam/transforms/filter/filter.go](https://codecov.io/gh/apache/beam/pull/16917/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-c2Rrcy9nby9wa2cvYmVhbS90cmFuc2Zvcm1zL2ZpbHRlci9maWx0ZXIuZ28=) | | |
   | [sdks/go/pkg/beam/core/graph/coder/registry.go](https://codecov.io/gh/apache/beam/pull/16917/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-c2Rrcy9nby9wa2cvYmVhbS9jb3JlL2dyYXBoL2NvZGVyL3JlZ2lzdHJ5Lmdv) | | |
   | [sdks/go/pkg/beam/core/graph/scope.go](https://codecov.io/gh/apache/beam/pull/16917/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-c2Rrcy9nby9wa2cvYmVhbS9jb3JlL2dyYXBoL3Njb3BlLmdv) | | |
   | [sdks/go/pkg/beam/core/runtime/harness/datamgr.go](https://codecov.io/gh/apache/beam/pull/16917/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-c2Rrcy9nby9wa2cvYmVhbS9jb3JlL3J1bnRpbWUvaGFybmVzcy9kYXRhbWdyLmdv) | | |
   | ... and [647 more](https://codecov.io/gh/apache/beam/pull/16917/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/16917?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/16917?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 [b2f2128...f73b890](https://codecov.io/gh/apache/beam/pull/16917?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] AnandInguva commented on a change in pull request #16917: [BEAM-13972] Add RunInference interface

Posted by GitBox <gi...@apache.org>.
AnandInguva commented on a change in pull request #16917:
URL: https://github.com/apache/beam/pull/16917#discussion_r812188685



##########
File path: sdks/python/apache_beam/ml/inference/run_inference.py
##########
@@ -0,0 +1,43 @@
+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements.  See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License.  You may obtain a copy of the License at
+#
+#    http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+
+import apache_beam as beam
+from apache_beam.ml.inference.model import (
+    PytorchModel, RunInferenceModel, SklearnModel)

Review comment:
       I think 3 import statements should be written individually to remove the Lint error.

##########
File path: sdks/python/apache_beam/ml/inference/model.py
##########
@@ -0,0 +1,74 @@
+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements.  See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License.  You may obtain a copy of the License at
+#
+#    http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+
+import abc
+
+VALID_PYTORCH_DEVICE_TYPES = ['CPU', 'GPU']
+VALID_SKLEARN_SERIALIZATION_TYPES = ['JOBLIB', 'PICKLE']
+
+
+class RunInferenceModel(object):
+  '''
+  Base class for the RunInference Model.
+  This class contains information about
+  a model that is needed
+  '''
+  def __init__(self, model_url):
+    self._model_url = model_url
+    self._validate_model()
+
+  @abc.abstractmethod
+  def _validate_model(self):
+    raise NotImplementedError("Please implement _validate_model")
+
+
+class PytorchModel(RunInferenceModel):
+  '''
+  This class wraps the PyTorch model, and other
+  PyTorch-specific parameters
+  '''
+  def __init__(self, model_url: str, device: str):
+    super().__init__(model_url)
+    self._device = device
+    self._validate_device()
+
+  def _validate_model(self):
+    pass
+
+  def _validate_device(self):
+    if self._device not in VALID_PYTORCH_DEVICE_TYPES:

Review comment:
       user may provide `device=cpu`. 
   1. We can convert `self._device` to `self._device.upper()` to match the names in the device list
   




-- 
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 #16917: [BEAM-13972] Add RunInference interface

Posted by GitBox <gi...@apache.org>.
yeandy commented on pull request #16917:
URL: https://github.com/apache/beam/pull/16917#issuecomment-1072515077


   @tvalentyn could you please review/merge?


-- 
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] edited a comment on pull request #16917: [BEAM-13972] Add RunInference interface

Posted by GitBox <gi...@apache.org>.
codecov[bot] edited a comment on pull request #16917:
URL: https://github.com/apache/beam/pull/16917#issuecomment-1047862344


   # [Codecov](https://codecov.io/gh/apache/beam/pull/16917?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 [#16917](https://codecov.io/gh/apache/beam/pull/16917?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (71e2f8c) into [master](https://codecov.io/gh/apache/beam/commit/b2f21281b94dc7182d15611a3e6bf1d872ea3205?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (b2f2128) will **increase** coverage by `36.74%`.
   > The diff coverage is `n/a`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/beam/pull/16917/graphs/tree.svg?width=650&height=150&src=pr&token=qcbbAh8Fj1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)](https://codecov.io/gh/apache/beam/pull/16917?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   
   ```diff
   @@             Coverage Diff             @@
   ##           master   #16917       +/-   ##
   ===========================================
   + Coverage   46.89%   83.63%   +36.74%     
   ===========================================
     Files         204      453      +249     
     Lines       20122    62528    +42406     
   ===========================================
   + Hits         9436    52295    +42859     
   - Misses       9686    10233      +547     
   + Partials     1000        0     -1000     
   ```
   
   | Flag | Coverage Δ | |
   |---|---|---|
   | python | `83.63% <ø> (?)` | |
   
   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/16917?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
   |---|---|---|
   | [sdks/go/pkg/beam/runners/direct/impulse.go](https://codecov.io/gh/apache/beam/pull/16917/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-c2Rrcy9nby9wa2cvYmVhbS9ydW5uZXJzL2RpcmVjdC9pbXB1bHNlLmdv) | | |
   | [sdks/go/pkg/beam/io/databaseio/writer.go](https://codecov.io/gh/apache/beam/pull/16917/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-c2Rrcy9nby9wa2cvYmVhbS9pby9kYXRhYmFzZWlvL3dyaXRlci5nbw==) | | |
   | [...ks/go/pkg/beam/runners/dataflow/dataflowlib/job.go](https://codecov.io/gh/apache/beam/pull/16917/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-c2Rrcy9nby9wa2cvYmVhbS9ydW5uZXJzL2RhdGFmbG93L2RhdGFmbG93bGliL2pvYi5nbw==) | | |
   | [sdks/go/pkg/beam/core/runtime/exec/fn\_arity.go](https://codecov.io/gh/apache/beam/pull/16917/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-c2Rrcy9nby9wa2cvYmVhbS9jb3JlL3J1bnRpbWUvZXhlYy9mbl9hcml0eS5nbw==) | | |
   | [sdks/go/pkg/beam/beam.shims.go](https://codecov.io/gh/apache/beam/pull/16917/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-c2Rrcy9nby9wa2cvYmVhbS9iZWFtLnNoaW1zLmdv) | | |
   | [sdks/go/pkg/beam/core/runtime/symbols.go](https://codecov.io/gh/apache/beam/pull/16917/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-c2Rrcy9nby9wa2cvYmVhbS9jb3JlL3J1bnRpbWUvc3ltYm9scy5nbw==) | | |
   | [sdks/go/pkg/beam/util/shimx/generate.go](https://codecov.io/gh/apache/beam/pull/16917/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-c2Rrcy9nby9wa2cvYmVhbS91dGlsL3NoaW14L2dlbmVyYXRlLmdv) | | |
   | [sdks/go/pkg/beam/runners/direct/buffer.go](https://codecov.io/gh/apache/beam/pull/16917/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-c2Rrcy9nby9wa2cvYmVhbS9ydW5uZXJzL2RpcmVjdC9idWZmZXIuZ28=) | | |
   | [sdks/go/pkg/beam/core/graph/coder/registry.go](https://codecov.io/gh/apache/beam/pull/16917/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-c2Rrcy9nby9wa2cvYmVhbS9jb3JlL2dyYXBoL2NvZGVyL3JlZ2lzdHJ5Lmdv) | | |
   | [sdks/go/pkg/beam/core/runtime/exec/cogbk.go](https://codecov.io/gh/apache/beam/pull/16917/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-c2Rrcy9nby9wa2cvYmVhbS9jb3JlL3J1bnRpbWUvZXhlYy9jb2diay5nbw==) | | |
   | ... and [647 more](https://codecov.io/gh/apache/beam/pull/16917/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/16917?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/16917?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 [b2f2128...71e2f8c](https://codecov.io/gh/apache/beam/pull/16917?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 change in pull request #16917: [BEAM-13972] Add RunInference interface

Posted by GitBox <gi...@apache.org>.
yeandy commented on a change in pull request #16917:
URL: https://github.com/apache/beam/pull/16917#discussion_r814244170



##########
File path: sdks/python/apache_beam/ml/inference/model.py
##########
@@ -0,0 +1,74 @@
+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements.  See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License.  You may obtain a copy of the License at
+#
+#    http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+
+import abc
+
+VALID_PYTORCH_DEVICE_TYPES = ['CPU', 'GPU']
+VALID_SKLEARN_SERIALIZATION_TYPES = ['JOBLIB', 'PICKLE']
+
+
+class RunInferenceModel(object):
+  '''
+  Base class for the RunInference Model.
+  This class contains information about
+  a model that is needed
+  '''
+  def __init__(self, model_url):
+    self._model_url = model_url
+    self._validate_model()
+
+  @abc.abstractmethod
+  def _validate_model(self):
+    raise NotImplementedError("Please implement _validate_model")
+
+
+class PytorchModel(RunInferenceModel):
+  '''
+  This class wraps the PyTorch model, and other
+  PyTorch-specific parameters
+  '''
+  def __init__(self, model_url: str, device: str):
+    super().__init__(model_url)
+    self._device = device
+    self._validate_device()
+
+  def _validate_model(self):
+    pass
+
+  def _validate_device(self):
+    if self._device not in VALID_PYTORCH_DEVICE_TYPES:
+      raise ValueError(
+          'Device type must be one of ' + VALID_PYTORCH_DEVICE_TYPES)
+
+
+class SklearnModel(RunInferenceModel):
+  '''
+  This class wraps the scikit-learn model, and other
+  scikit-learn-specific parameters
+  '''
+  def __init__(self, model_url, serialization_method):
+    super().__init__(model_url)
+    self._serialization_method = serialization_method
+
+  def _validate_model(self):
+    pass
+
+  def _validate_serialization(self):

Review comment:
       Yes, in init




-- 
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] edited a comment on pull request #16917: [BEAM-13972] Add RunInference interface

Posted by GitBox <gi...@apache.org>.
codecov[bot] edited a comment on pull request #16917:
URL: https://github.com/apache/beam/pull/16917#issuecomment-1047862344


   # [Codecov](https://codecov.io/gh/apache/beam/pull/16917?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 [#16917](https://codecov.io/gh/apache/beam/pull/16917?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (b0cea39) into [master](https://codecov.io/gh/apache/beam/commit/b2f21281b94dc7182d15611a3e6bf1d872ea3205?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (b2f2128) will **increase** coverage by `36.75%`.
   > The diff coverage is `96.77%`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/beam/pull/16917/graphs/tree.svg?width=650&height=150&src=pr&token=qcbbAh8Fj1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)](https://codecov.io/gh/apache/beam/pull/16917?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   
   ```diff
   @@             Coverage Diff             @@
   ##           master   #16917       +/-   ##
   ===========================================
   + Coverage   46.89%   83.64%   +36.75%     
   ===========================================
     Files         204      454      +250     
     Lines       20122    62593    +42471     
   ===========================================
   + Hits         9436    52357    +42921     
   - Misses       9686    10236      +550     
   + Partials     1000        0     -1000     
   ```
   
   | Flag | Coverage Δ | |
   |---|---|---|
   | python | `83.64% <96.77%> (?)` | |
   
   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/16917?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
   |---|---|---|
   | [sdks/python/apache\_beam/ml/inference/api.py](https://codecov.io/gh/apache/beam/pull/16917/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-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vbWwvaW5mZXJlbmNlL2FwaS5weQ==) | `96.77% <96.77%> (ø)` | |
   | [sdks/go/pkg/beam/core/typex/class.go](https://codecov.io/gh/apache/beam/pull/16917/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-c2Rrcy9nby9wa2cvYmVhbS9jb3JlL3R5cGV4L2NsYXNzLmdv) | | |
   | [sdks/go/pkg/beam/core/runtime/exec/datasource.go](https://codecov.io/gh/apache/beam/pull/16917/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-c2Rrcy9nby9wa2cvYmVhbS9jb3JlL3J1bnRpbWUvZXhlYy9kYXRhc291cmNlLmdv) | | |
   | [sdks/go/pkg/beam/provision/provision.go](https://codecov.io/gh/apache/beam/pull/16917/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-c2Rrcy9nby9wa2cvYmVhbS9wcm92aXNpb24vcHJvdmlzaW9uLmdv) | | |
   | [sdks/go/pkg/beam/core/runtime/graphx/coder.go](https://codecov.io/gh/apache/beam/pull/16917/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-c2Rrcy9nby9wa2cvYmVhbS9jb3JlL3J1bnRpbWUvZ3JhcGh4L2NvZGVyLmdv) | | |
   | [sdks/go/pkg/beam/transforms/top/top.go](https://codecov.io/gh/apache/beam/pull/16917/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-c2Rrcy9nby9wa2cvYmVhbS90cmFuc2Zvcm1zL3RvcC90b3AuZ28=) | | |
   | [sdks/go/pkg/beam/transforms/stats/sum\_switch.go](https://codecov.io/gh/apache/beam/pull/16917/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-c2Rrcy9nby9wa2cvYmVhbS90cmFuc2Zvcm1zL3N0YXRzL3N1bV9zd2l0Y2guZ28=) | | |
   | [sdks/go/pkg/beam/runners/direct/gbk.go](https://codecov.io/gh/apache/beam/pull/16917/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-c2Rrcy9nby9wa2cvYmVhbS9ydW5uZXJzL2RpcmVjdC9nYmsuZ28=) | | |
   | [...kg/beam/core/runtime/graphx/schema/logicaltypes.go](https://codecov.io/gh/apache/beam/pull/16917/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-c2Rrcy9nby9wa2cvYmVhbS9jb3JlL3J1bnRpbWUvZ3JhcGh4L3NjaGVtYS9sb2dpY2FsdHlwZXMuZ28=) | | |
   | [sdks/go/pkg/beam/core/runtime/xlangx/payload.go](https://codecov.io/gh/apache/beam/pull/16917/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-c2Rrcy9nby9wa2cvYmVhbS9jb3JlL3J1bnRpbWUveGxhbmd4L3BheWxvYWQuZ28=) | | |
   | ... and [648 more](https://codecov.io/gh/apache/beam/pull/16917/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/16917?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/16917?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 [b2f2128...b0cea39](https://codecov.io/gh/apache/beam/pull/16917?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] codecov[bot] edited a comment on pull request #16917: [BEAM-13972] Add RunInference interface

Posted by GitBox <gi...@apache.org>.
codecov[bot] edited a comment on pull request #16917:
URL: https://github.com/apache/beam/pull/16917#issuecomment-1047862344


   # [Codecov](https://codecov.io/gh/apache/beam/pull/16917?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 [#16917](https://codecov.io/gh/apache/beam/pull/16917?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (f73b890) into [master](https://codecov.io/gh/apache/beam/commit/b2f21281b94dc7182d15611a3e6bf1d872ea3205?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (b2f2128) will **increase** coverage by `36.74%`.
   > The diff coverage is `n/a`.
   
   > :exclamation: Current head f73b890 differs from pull request most recent head e802bfe. Consider uploading reports for the commit e802bfe to get more accurate results
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/beam/pull/16917/graphs/tree.svg?width=650&height=150&src=pr&token=qcbbAh8Fj1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)](https://codecov.io/gh/apache/beam/pull/16917?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   
   ```diff
   @@             Coverage Diff             @@
   ##           master   #16917       +/-   ##
   ===========================================
   + Coverage   46.89%   83.64%   +36.74%     
   ===========================================
     Files         204      453      +249     
     Lines       20122    62471    +42349     
   ===========================================
   + Hits         9436    52251    +42815     
   - Misses       9686    10220      +534     
   + Partials     1000        0     -1000     
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/beam/pull/16917?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
   |---|---|---|
   | [sdks/go/pkg/beam/core/runtime/harness/sampler.go](https://codecov.io/gh/apache/beam/pull/16917/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-c2Rrcy9nby9wa2cvYmVhbS9jb3JlL3J1bnRpbWUvaGFybmVzcy9zYW1wbGVyLmdv) | | |
   | [sdks/go/pkg/beam/testing/teststream/teststream.go](https://codecov.io/gh/apache/beam/pull/16917/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-c2Rrcy9nby9wa2cvYmVhbS90ZXN0aW5nL3Rlc3RzdHJlYW0vdGVzdHN0cmVhbS5nbw==) | | |
   | [sdks/go/pkg/beam/transforms/filter/filter.shims.go](https://codecov.io/gh/apache/beam/pull/16917/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-c2Rrcy9nby9wa2cvYmVhbS90cmFuc2Zvcm1zL2ZpbHRlci9maWx0ZXIuc2hpbXMuZ28=) | | |
   | [sdks/go/pkg/beam/provision/provision.go](https://codecov.io/gh/apache/beam/pull/16917/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-c2Rrcy9nby9wa2cvYmVhbS9wcm92aXNpb24vcHJvdmlzaW9uLmdv) | | |
   | [sdks/go/pkg/beam/core/runtime/coderx/string.go](https://codecov.io/gh/apache/beam/pull/16917/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-c2Rrcy9nby9wa2cvYmVhbS9jb3JlL3J1bnRpbWUvY29kZXJ4L3N0cmluZy5nbw==) | | |
   | [sdks/go/pkg/beam/core/runtime/exec/translate.go](https://codecov.io/gh/apache/beam/pull/16917/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-c2Rrcy9nby9wa2cvYmVhbS9jb3JlL3J1bnRpbWUvZXhlYy90cmFuc2xhdGUuZ28=) | | |
   | [...pkg/beam/core/runtime/xlangx/expansionx/process.go](https://codecov.io/gh/apache/beam/pull/16917/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-c2Rrcy9nby9wa2cvYmVhbS9jb3JlL3J1bnRpbWUveGxhbmd4L2V4cGFuc2lvbngvcHJvY2Vzcy5nbw==) | | |
   | [sdks/go/pkg/beam/transforms/stats/min\_switch.go](https://codecov.io/gh/apache/beam/pull/16917/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-c2Rrcy9nby9wa2cvYmVhbS90cmFuc2Zvcm1zL3N0YXRzL21pbl9zd2l0Y2guZ28=) | | |
   | [sdks/go/pkg/beam/util/shimx/generate.go](https://codecov.io/gh/apache/beam/pull/16917/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-c2Rrcy9nby9wa2cvYmVhbS91dGlsL3NoaW14L2dlbmVyYXRlLmdv) | | |
   | [sdks/go/pkg/beam/core/runtime/exec/sideinput.go](https://codecov.io/gh/apache/beam/pull/16917/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-c2Rrcy9nby9wa2cvYmVhbS9jb3JlL3J1bnRpbWUvZXhlYy9zaWRlaW5wdXQuZ28=) | | |
   | ... and [647 more](https://codecov.io/gh/apache/beam/pull/16917/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/16917?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/16917?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 [b2f2128...e802bfe](https://codecov.io/gh/apache/beam/pull/16917?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 change in pull request #16917: [BEAM-13972] Add RunInference interface

Posted by GitBox <gi...@apache.org>.
yeandy commented on a change in pull request #16917:
URL: https://github.com/apache/beam/pull/16917#discussion_r814079004



##########
File path: sdks/python/apache_beam/ml/inference/model.py
##########
@@ -0,0 +1,74 @@
+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements.  See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License.  You may obtain a copy of the License at
+#
+#    http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+
+import abc
+
+VALID_PYTORCH_DEVICE_TYPES = ['CPU', 'GPU']
+VALID_SKLEARN_SERIALIZATION_TYPES = ['JOBLIB', 'PICKLE']
+
+
+class RunInferenceModel(object):
+  '''
+  Base class for the RunInference Model.
+  This class contains information about
+  a model that is needed
+  '''
+  def __init__(self, model_url):
+    self._model_url = model_url
+    self._validate_model()
+
+  @abc.abstractmethod
+  def _validate_model(self):
+    raise NotImplementedError("Please implement _validate_model")
+
+
+class PytorchModel(RunInferenceModel):
+  '''
+  This class wraps the PyTorch model, and other
+  PyTorch-specific parameters
+  '''
+  def __init__(self, model_url: str, device: str):
+    super().__init__(model_url)
+    self._device = device
+    self._validate_device()
+
+  def _validate_model(self):
+    pass
+
+  def _validate_device(self):
+    if self._device not in VALID_PYTORCH_DEVICE_TYPES:
+      raise ValueError(
+          'Device type must be one of ' + VALID_PYTORCH_DEVICE_TYPES)
+
+
+class SklearnModel(RunInferenceModel):
+  '''
+  This class wraps the scikit-learn model, and other
+  scikit-learn-specific parameters
+  '''
+  def __init__(self, model_url, serialization_method):

Review comment:
       Yeah, we can default to `pickle`. Looks like `joblib` can be faster on large numpy arrays. Docs [here](https://scikit-learn.org/stable/modules/model_persistence.html).




-- 
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 #16917: [BEAM-13972] Add RunInference interface

Posted by GitBox <gi...@apache.org>.
yeandy commented on pull request #16917:
URL: https://github.com/apache/beam/pull/16917#issuecomment-1058571061


   R: @robertwb @tvalentyn 
   CC: @kerrydc


-- 
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] edited a comment on pull request #16917: [BEAM-13972] Add RunInference interface

Posted by GitBox <gi...@apache.org>.
codecov[bot] edited a comment on pull request #16917:
URL: https://github.com/apache/beam/pull/16917#issuecomment-1047862344


   # [Codecov](https://codecov.io/gh/apache/beam/pull/16917?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 [#16917](https://codecov.io/gh/apache/beam/pull/16917?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (cf31b18) into [master](https://codecov.io/gh/apache/beam/commit/b2f21281b94dc7182d15611a3e6bf1d872ea3205?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (b2f2128) will **increase** coverage by `36.75%`.
   > The diff coverage is `96.77%`.
   
   > :exclamation: Current head cf31b18 differs from pull request most recent head 4280dfc. Consider uploading reports for the commit 4280dfc to get more accurate results
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/beam/pull/16917/graphs/tree.svg?width=650&height=150&src=pr&token=qcbbAh8Fj1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)](https://codecov.io/gh/apache/beam/pull/16917?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   
   ```diff
   @@             Coverage Diff             @@
   ##           master   #16917       +/-   ##
   ===========================================
   + Coverage   46.89%   83.64%   +36.75%     
   ===========================================
     Files         204      454      +250     
     Lines       20122    62591    +42469     
   ===========================================
   + Hits         9436    52354    +42918     
   - Misses       9686    10237      +551     
   + Partials     1000        0     -1000     
   ```
   
   | Flag | Coverage Δ | |
   |---|---|---|
   | python | `83.64% <96.77%> (?)` | |
   
   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/16917?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
   |---|---|---|
   | [sdks/python/apache\_beam/ml/inference/api.py](https://codecov.io/gh/apache/beam/pull/16917/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-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vbWwvaW5mZXJlbmNlL2FwaS5weQ==) | `96.77% <96.77%> (ø)` | |
   | [sdks/go/pkg/beam/testing/passert/passert.go](https://codecov.io/gh/apache/beam/pull/16917/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-c2Rrcy9nby9wa2cvYmVhbS90ZXN0aW5nL3Bhc3NlcnQvcGFzc2VydC5nbw==) | | |
   | [sdks/go/pkg/beam/core/runtime/exec/status.go](https://codecov.io/gh/apache/beam/pull/16917/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-c2Rrcy9nby9wa2cvYmVhbS9jb3JlL3J1bnRpbWUvZXhlYy9zdGF0dXMuZ28=) | | |
   | [sdks/go/pkg/beam/option.go](https://codecov.io/gh/apache/beam/pull/16917/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-c2Rrcy9nby9wa2cvYmVhbS9vcHRpb24uZ28=) | | |
   | [sdks/go/pkg/beam/core/runtime/graphx/user.go](https://codecov.io/gh/apache/beam/pull/16917/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-c2Rrcy9nby9wa2cvYmVhbS9jb3JlL3J1bnRpbWUvZ3JhcGh4L3VzZXIuZ28=) | | |
   | [...o/pkg/beam/io/rtrackers/offsetrange/offsetrange.go](https://codecov.io/gh/apache/beam/pull/16917/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-c2Rrcy9nby9wa2cvYmVhbS9pby9ydHJhY2tlcnMvb2Zmc2V0cmFuZ2Uvb2Zmc2V0cmFuZ2UuZ28=) | | |
   | [...s/go/pkg/beam/core/runtime/harness/sampler\_hook.go](https://codecov.io/gh/apache/beam/pull/16917/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-c2Rrcy9nby9wa2cvYmVhbS9jb3JlL3J1bnRpbWUvaGFybmVzcy9zYW1wbGVyX2hvb2suZ28=) | | |
   | [sdks/go/pkg/beam/artifact/materialize.go](https://codecov.io/gh/apache/beam/pull/16917/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-c2Rrcy9nby9wa2cvYmVhbS9hcnRpZmFjdC9tYXRlcmlhbGl6ZS5nbw==) | | |
   | [sdks/go/pkg/beam/core/runtime/graphx/xlang.go](https://codecov.io/gh/apache/beam/pull/16917/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-c2Rrcy9nby9wa2cvYmVhbS9jb3JlL3J1bnRpbWUvZ3JhcGh4L3hsYW5nLmdv) | | |
   | [sdks/go/pkg/beam/core/funcx/output.go](https://codecov.io/gh/apache/beam/pull/16917/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-c2Rrcy9nby9wa2cvYmVhbS9jb3JlL2Z1bmN4L291dHB1dC5nbw==) | | |
   | ... and [648 more](https://codecov.io/gh/apache/beam/pull/16917/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/16917?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/16917?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 [b2f2128...4280dfc](https://codecov.io/gh/apache/beam/pull/16917?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] codecov[bot] edited a comment on pull request #16917: [BEAM-13972] Add RunInference interface

Posted by GitBox <gi...@apache.org>.
codecov[bot] edited a comment on pull request #16917:
URL: https://github.com/apache/beam/pull/16917#issuecomment-1047862344


   # [Codecov](https://codecov.io/gh/apache/beam/pull/16917?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 [#16917](https://codecov.io/gh/apache/beam/pull/16917?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (f73b890) into [master](https://codecov.io/gh/apache/beam/commit/b2f21281b94dc7182d15611a3e6bf1d872ea3205?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (b2f2128) will **increase** coverage by `36.74%`.
   > The diff coverage is `n/a`.
   
   > :exclamation: Current head f73b890 differs from pull request most recent head d75631a. Consider uploading reports for the commit d75631a to get more accurate results
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/beam/pull/16917/graphs/tree.svg?width=650&height=150&src=pr&token=qcbbAh8Fj1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)](https://codecov.io/gh/apache/beam/pull/16917?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   
   ```diff
   @@             Coverage Diff             @@
   ##           master   #16917       +/-   ##
   ===========================================
   + Coverage   46.89%   83.64%   +36.74%     
   ===========================================
     Files         204      453      +249     
     Lines       20122    62471    +42349     
   ===========================================
   + Hits         9436    52251    +42815     
   - Misses       9686    10220      +534     
   + Partials     1000        0     -1000     
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/beam/pull/16917?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
   |---|---|---|
   | [sdks/go/pkg/beam/testing/passert/equals.go](https://codecov.io/gh/apache/beam/pull/16917/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-c2Rrcy9nby9wa2cvYmVhbS90ZXN0aW5nL3Bhc3NlcnQvZXF1YWxzLmdv) | | |
   | [sdks/go/pkg/beam/core/runtime/graphx/serialize.go](https://codecov.io/gh/apache/beam/pull/16917/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-c2Rrcy9nby9wa2cvYmVhbS9jb3JlL3J1bnRpbWUvZ3JhcGh4L3NlcmlhbGl6ZS5nbw==) | | |
   | [sdks/go/pkg/beam/core/metrics/metrics.go](https://codecov.io/gh/apache/beam/pull/16917/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-c2Rrcy9nby9wa2cvYmVhbS9jb3JlL21ldHJpY3MvbWV0cmljcy5nbw==) | | |
   | [sdks/go/pkg/beam/core/util/reflectx/types.go](https://codecov.io/gh/apache/beam/pull/16917/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-c2Rrcy9nby9wa2cvYmVhbS9jb3JlL3V0aWwvcmVmbGVjdHgvdHlwZXMuZ28=) | | |
   | [sdks/go/pkg/beam/create.go](https://codecov.io/gh/apache/beam/pull/16917/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-c2Rrcy9nby9wa2cvYmVhbS9jcmVhdGUuZ28=) | | |
   | [sdks/go/pkg/beam/io/filesystem/local/local.go](https://codecov.io/gh/apache/beam/pull/16917/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-c2Rrcy9nby9wa2cvYmVhbS9pby9maWxlc3lzdGVtL2xvY2FsL2xvY2FsLmdv) | | |
   | [sdks/go/pkg/beam/core/graph/graph.go](https://codecov.io/gh/apache/beam/pull/16917/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-c2Rrcy9nby9wa2cvYmVhbS9jb3JlL2dyYXBoL2dyYXBoLmdv) | | |
   | [sdks/go/pkg/beam/core/graph/coder/row\_decoder.go](https://codecov.io/gh/apache/beam/pull/16917/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-c2Rrcy9nby9wa2cvYmVhbS9jb3JlL2dyYXBoL2NvZGVyL3Jvd19kZWNvZGVyLmdv) | | |
   | [sdks/go/pkg/beam/core/runtime/harness/session.go](https://codecov.io/gh/apache/beam/pull/16917/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-c2Rrcy9nby9wa2cvYmVhbS9jb3JlL3J1bnRpbWUvaGFybmVzcy9zZXNzaW9uLmdv) | | |
   | [sdks/go/pkg/beam/io/databaseio/util.go](https://codecov.io/gh/apache/beam/pull/16917/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-c2Rrcy9nby9wa2cvYmVhbS9pby9kYXRhYmFzZWlvL3V0aWwuZ28=) | | |
   | ... and [647 more](https://codecov.io/gh/apache/beam/pull/16917/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/16917?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/16917?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 [b2f2128...d75631a](https://codecov.io/gh/apache/beam/pull/16917?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 change in pull request #16917: [BEAM-13972] Add RunInference interface

Posted by GitBox <gi...@apache.org>.
yeandy commented on a change in pull request #16917:
URL: https://github.com/apache/beam/pull/16917#discussion_r817201371



##########
File path: sdks/python/apache_beam/ml/inference/api.py
##########
@@ -0,0 +1,84 @@
+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements.  See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License.  You may obtain a copy of the License at
+#
+#    http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+
+from dataclasses import dataclass
+import apache_beam as beam
+from typing import Tuple, TypeVar, Union
+# TODO: implement RunInferenceImpl
+# from apache_beam.ml.inference.base import RunInferenceImpl
+
+
+@dataclass
+class BaseModelSpec:
+  model_url: str
+
+
+@dataclass
+class PyTorchModel(BaseModelSpec):
+  device: str
+
+  def __post_init__(self):
+    self.device = self.device.upper()
+
+
+@dataclass
+class SklearnModel(BaseModelSpec):
+  serialization_type: str
+
+  def __post_init__(self):
+    self.serialization_type = self.serialization_type.upper()
+
+
+_K = TypeVar('_K')
+_INPUT_TYPE = TypeVar('_INPUT_TYPE')
+_OUTPUT_TYPE = TypeVar('_OUTPUT_TYPE')
+
+
+@dataclass
+class PredictionResult:
+  key: _K
+  example: _INPUT_TYPE
+  inference: _OUTPUT_TYPE
+
+
+@beam.ptransform_fn
+@beam.typehints.with_input_types(Union[_INPUT_TYPE, Tuple[_K, _INPUT_TYPE]])
+@beam.typehints.with_output_types(PredictionResult)
+def RunInference(
+    examples: beam.pvalue.PCollection,
+    model: BaseModelSpec) -> beam.pvalue.PCollection:
+  """Run inference with a model.
+
+  There one type of inference you can perform using this PTransform:
+    1. In-process inference from a SavedModel instance.
+    TODO: Add remote inference by using a service endpoint.
+
+  Args:
+    examples: A PCollection containing examples of the following possible kinds,
+      each with their corresponding return type.
+        - PCollection[Example]                 -> PCollection[PredictionResult]

Review comment:
       Will change `Example` to the appropriate name when we finalize the type




-- 
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] edited a comment on pull request #16917: [BEAM-13972] Add RunInference interface

Posted by GitBox <gi...@apache.org>.
codecov[bot] edited a comment on pull request #16917:
URL: https://github.com/apache/beam/pull/16917#issuecomment-1047862344


   # [Codecov](https://codecov.io/gh/apache/beam/pull/16917?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 [#16917](https://codecov.io/gh/apache/beam/pull/16917?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (71e2f8c) into [master](https://codecov.io/gh/apache/beam/commit/b2f21281b94dc7182d15611a3e6bf1d872ea3205?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (b2f2128) will **increase** coverage by `36.74%`.
   > The diff coverage is `n/a`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/beam/pull/16917/graphs/tree.svg?width=650&height=150&src=pr&token=qcbbAh8Fj1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)](https://codecov.io/gh/apache/beam/pull/16917?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   
   ```diff
   @@             Coverage Diff             @@
   ##           master   #16917       +/-   ##
   ===========================================
   + Coverage   46.89%   83.63%   +36.74%     
   ===========================================
     Files         204      453      +249     
     Lines       20122    62528    +42406     
   ===========================================
   + Hits         9436    52295    +42859     
   - Misses       9686    10233      +547     
   + Partials     1000        0     -1000     
   ```
   
   | Flag | Coverage Δ | |
   |---|---|---|
   | python | `83.63% <ø> (?)` | |
   
   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/16917?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
   |---|---|---|
   | [sdks/go/pkg/beam/core/runtime/exec/fullvalue.go](https://codecov.io/gh/apache/beam/pull/16917/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-c2Rrcy9nby9wa2cvYmVhbS9jb3JlL3J1bnRpbWUvZXhlYy9mdWxsdmFsdWUuZ28=) | | |
   | [sdks/go/pkg/beam/core/metrics/store.go](https://codecov.io/gh/apache/beam/pull/16917/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-c2Rrcy9nby9wa2cvYmVhbS9jb3JlL21ldHJpY3Mvc3RvcmUuZ28=) | | |
   | [sdks/go/pkg/beam/core/runtime/pipelinex/util.go](https://codecov.io/gh/apache/beam/pull/16917/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-c2Rrcy9nby9wa2cvYmVhbS9jb3JlL3J1bnRpbWUvcGlwZWxpbmV4L3V0aWwuZ28=) | | |
   | [sdks/go/pkg/beam/core/funcx/sideinput.go](https://codecov.io/gh/apache/beam/pull/16917/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-c2Rrcy9nby9wa2cvYmVhbS9jb3JlL2Z1bmN4L3NpZGVpbnB1dC5nbw==) | | |
   | [sdks/go/pkg/beam/core/runtime/exec/coder.go](https://codecov.io/gh/apache/beam/pull/16917/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-c2Rrcy9nby9wa2cvYmVhbS9jb3JlL3J1bnRpbWUvZXhlYy9jb2Rlci5nbw==) | | |
   | [sdks/go/pkg/beam/testing/passert/floats.go](https://codecov.io/gh/apache/beam/pull/16917/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-c2Rrcy9nby9wa2cvYmVhbS90ZXN0aW5nL3Bhc3NlcnQvZmxvYXRzLmdv) | | |
   | [sdks/go/pkg/beam/util.go](https://codecov.io/gh/apache/beam/pull/16917/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-c2Rrcy9nby9wa2cvYmVhbS91dGlsLmdv) | | |
   | [sdks/go/pkg/beam/transforms/stats/sum\_switch.go](https://codecov.io/gh/apache/beam/pull/16917/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-c2Rrcy9nby9wa2cvYmVhbS90cmFuc2Zvcm1zL3N0YXRzL3N1bV9zd2l0Y2guZ28=) | | |
   | [sdks/go/pkg/beam/core/runtime/xlangx/namespace.go](https://codecov.io/gh/apache/beam/pull/16917/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-c2Rrcy9nby9wa2cvYmVhbS9jb3JlL3J1bnRpbWUveGxhbmd4L25hbWVzcGFjZS5nbw==) | | |
   | [sdks/go/pkg/beam/core/runtime/types.go](https://codecov.io/gh/apache/beam/pull/16917/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-c2Rrcy9nby9wa2cvYmVhbS9jb3JlL3J1bnRpbWUvdHlwZXMuZ28=) | | |
   | ... and [647 more](https://codecov.io/gh/apache/beam/pull/16917/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/16917?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/16917?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 [b2f2128...71e2f8c](https://codecov.io/gh/apache/beam/pull/16917?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 pull request #16917: [BEAM-13972] Add RunInference interface

Posted by GitBox <gi...@apache.org>.
yeandy commented on pull request #16917:
URL: https://github.com/apache/beam/pull/16917#issuecomment-1072829832


   Thanks. They will be reconciled once the change to TFX is finished.


-- 
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 #16917: [BEAM-13972] Add RunInference interface

Posted by GitBox <gi...@apache.org>.
codecov[bot] commented on pull request #16917:
URL: https://github.com/apache/beam/pull/16917#issuecomment-1047862344


   # [Codecov](https://codecov.io/gh/apache/beam/pull/16917?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 [#16917](https://codecov.io/gh/apache/beam/pull/16917?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (f73b890) into [master](https://codecov.io/gh/apache/beam/commit/b2f21281b94dc7182d15611a3e6bf1d872ea3205?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (b2f2128) will **increase** coverage by `36.74%`.
   > The diff coverage is `n/a`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/beam/pull/16917/graphs/tree.svg?width=650&height=150&src=pr&token=qcbbAh8Fj1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)](https://codecov.io/gh/apache/beam/pull/16917?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   
   ```diff
   @@             Coverage Diff             @@
   ##           master   #16917       +/-   ##
   ===========================================
   + Coverage   46.89%   83.64%   +36.74%     
   ===========================================
     Files         204      453      +249     
     Lines       20122    62471    +42349     
   ===========================================
   + Hits         9436    52251    +42815     
   - Misses       9686    10220      +534     
   + Partials     1000        0     -1000     
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/beam/pull/16917?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
   |---|---|---|
   | [sdks/go/pkg/beam/runners/vet/vet.go](https://codecov.io/gh/apache/beam/pull/16917/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-c2Rrcy9nby9wa2cvYmVhbS9ydW5uZXJzL3ZldC92ZXQuZ28=) | | |
   | [sdks/go/pkg/beam/io/databaseio/util.go](https://codecov.io/gh/apache/beam/pull/16917/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-c2Rrcy9nby9wa2cvYmVhbS9pby9kYXRhYmFzZWlvL3V0aWwuZ28=) | | |
   | [sdks/go/pkg/beam/core/runtime/graphx/dataflow.go](https://codecov.io/gh/apache/beam/pull/16917/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-c2Rrcy9nby9wa2cvYmVhbS9jb3JlL3J1bnRpbWUvZ3JhcGh4L2RhdGFmbG93Lmdv) | | |
   | [sdks/go/pkg/beam/pcollection.go](https://codecov.io/gh/apache/beam/pull/16917/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-c2Rrcy9nby9wa2cvYmVhbS9wY29sbGVjdGlvbi5nbw==) | | |
   | [sdks/go/pkg/beam/io/filesystem/local/local.go](https://codecov.io/gh/apache/beam/pull/16917/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-c2Rrcy9nby9wa2cvYmVhbS9pby9maWxlc3lzdGVtL2xvY2FsL2xvY2FsLmdv) | | |
   | [sdks/go/pkg/beam/io/synthetic/step.go](https://codecov.io/gh/apache/beam/pull/16917/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-c2Rrcy9nby9wa2cvYmVhbS9pby9zeW50aGV0aWMvc3RlcC5nbw==) | | |
   | [sdks/go/pkg/beam/core/graph/graph.go](https://codecov.io/gh/apache/beam/pull/16917/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-c2Rrcy9nby9wa2cvYmVhbS9jb3JlL2dyYXBoL2dyYXBoLmdv) | | |
   | [sdks/go/pkg/beam/core/graph/coder/map.go](https://codecov.io/gh/apache/beam/pull/16917/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-c2Rrcy9nby9wa2cvYmVhbS9jb3JlL2dyYXBoL2NvZGVyL21hcC5nbw==) | | |
   | [sdks/go/pkg/beam/core/util/reflectx/tags.go](https://codecov.io/gh/apache/beam/pull/16917/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-c2Rrcy9nby9wa2cvYmVhbS9jb3JlL3V0aWwvcmVmbGVjdHgvdGFncy5nbw==) | | |
   | [sdks/go/pkg/beam/external.go](https://codecov.io/gh/apache/beam/pull/16917/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-c2Rrcy9nby9wa2cvYmVhbS9leHRlcm5hbC5nbw==) | | |
   | ... and [647 more](https://codecov.io/gh/apache/beam/pull/16917/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/16917?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/16917?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 [b2f2128...f73b890](https://codecov.io/gh/apache/beam/pull/16917?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 pull request #16917: [BEAM-13972] Add RunInference interface

Posted by GitBox <gi...@apache.org>.
yeandy commented on pull request #16917:
URL: https://github.com/apache/beam/pull/16917#issuecomment-1063165137


   Updated the interface per our discussion, PTAL. @robertwb


-- 
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 change in pull request #16917: [BEAM-13972] Add RunInference interface

Posted by GitBox <gi...@apache.org>.
yeandy commented on a change in pull request #16917:
URL: https://github.com/apache/beam/pull/16917#discussion_r814115504



##########
File path: sdks/python/apache_beam/ml/inference/model.py
##########
@@ -0,0 +1,74 @@
+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements.  See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License.  You may obtain a copy of the License at
+#
+#    http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+
+import abc
+
+VALID_PYTORCH_DEVICE_TYPES = ['CPU', 'GPU']

Review comment:
       Yeah, probably. The constants should never be used outside this module (no need to import), so it probably makes more sense to put it in the class. 
   
   Curious, is this more of a stylistic choice?
   According to [PEP8](https://www.python.org/dev/peps/pep-0008/#constants), constants are _usually_ defined on a module level, but doesn't say that it needs to be class level. 
   
   I suppose some downsides to class-level variables is the possibility of them be shadowed by subclasses.
   
   




-- 
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 change in pull request #16917: [BEAM-13972] Add RunInference interface

Posted by GitBox <gi...@apache.org>.
AnandInguva commented on a change in pull request #16917:
URL: https://github.com/apache/beam/pull/16917#discussion_r818137359



##########
File path: sdks/python/apache_beam/ml/inference/api_test.py
##########
@@ -0,0 +1,54 @@
+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements.  See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License.  You may obtain a copy of the License at
+#
+#    http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+
+import unittest
+from apache_beam.ml.inference.api import PyTorchDevice
+from apache_beam.ml.inference.api import PyTorchModelSpec
+from apache_beam.ml.inference.api import SklearnSerializationType
+from apache_beam.ml.inference.api import SklearnModelSpec
+import pytest  # pylint: disable=unused-import
+
+
+class RunInferenceTest(unittest.TestCase):
+  def test_valid_pytorch_model(self):
+    model = PyTorchModelSpec(
+        model_url='pytorch_model.pth', device=PyTorchDevice.GPU)
+    assert model.model_url == 'pytorch_model.pth'
+    assert model.device == PyTorchDevice.GPU
+
+  def test_invalid_pytorch_model(self):
+    with pytest.raises(TypeError, match='device'):
+      PyTorchModelSpec(model_url='pytorch_model.pth')
+    with pytest.raises(TypeError, match='model_url'):
+      PyTorchModelSpec(device=PyTorchDevice.CPU)
+
+  def test_valid_sklearn_model(self):
+    model = SklearnModelSpec(
+        model_url='sklearn_model.pickle',
+        serialization_type=SklearnSerializationType.PICKLE)
+    assert model.model_url == 'sklearn_model.pickle'

Review comment:
       It was suggested to me to use the class method `self.assertEqual` instead of assert. Making a note 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 change in pull request #16917: [BEAM-13972] Add RunInference interface

Posted by GitBox <gi...@apache.org>.
yeandy commented on a change in pull request #16917:
URL: https://github.com/apache/beam/pull/16917#discussion_r818119046



##########
File path: sdks/python/apache_beam/ml/inference/api.py
##########
@@ -0,0 +1,84 @@
+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements.  See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License.  You may obtain a copy of the License at
+#
+#    http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+
+from dataclasses import dataclass
+import apache_beam as beam
+from typing import Tuple, TypeVar, Union
+# TODO: implement RunInferenceImpl
+# from apache_beam.ml.inference.base import RunInferenceImpl
+
+
+@dataclass
+class BaseModelSpec:
+  model_url: str
+
+
+@dataclass
+class PyTorchModelSpec(BaseModelSpec):
+  device: str
+
+  def __post_init__(self):
+    self.device = self.device.upper()
+
+
+@dataclass
+class SklearnModelSpec(BaseModelSpec):
+  serialization_type: str
+
+  def __post_init__(self):

Review comment:
       True. Fixed!




-- 
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] edited a comment on pull request #16917: [BEAM-13972] Add RunInference interface

Posted by GitBox <gi...@apache.org>.
codecov[bot] edited a comment on pull request #16917:
URL: https://github.com/apache/beam/pull/16917#issuecomment-1047862344


   # [Codecov](https://codecov.io/gh/apache/beam/pull/16917?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 [#16917](https://codecov.io/gh/apache/beam/pull/16917?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (4280dfc) into [master](https://codecov.io/gh/apache/beam/commit/b2f21281b94dc7182d15611a3e6bf1d872ea3205?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (b2f2128) will **increase** coverage by `36.74%`.
   > The diff coverage is `96.77%`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/beam/pull/16917/graphs/tree.svg?width=650&height=150&src=pr&token=qcbbAh8Fj1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)](https://codecov.io/gh/apache/beam/pull/16917?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   
   ```diff
   @@             Coverage Diff             @@
   ##           master   #16917       +/-   ##
   ===========================================
   + Coverage   46.89%   83.63%   +36.74%     
   ===========================================
     Files         204      454      +250     
     Lines       20122    62591    +42469     
   ===========================================
   + Hits         9436    52348    +42912     
   - Misses       9686    10243      +557     
   + Partials     1000        0     -1000     
   ```
   
   | Flag | Coverage Δ | |
   |---|---|---|
   | python | `83.63% <96.77%> (?)` | |
   
   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/16917?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
   |---|---|---|
   | [sdks/python/apache\_beam/ml/inference/api.py](https://codecov.io/gh/apache/beam/pull/16917/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-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vbWwvaW5mZXJlbmNlL2FwaS5weQ==) | `96.77% <96.77%> (ø)` | |
   | [sdks/go/pkg/beam/beam.shims.go](https://codecov.io/gh/apache/beam/pull/16917/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-c2Rrcy9nby9wa2cvYmVhbS9iZWFtLnNoaW1zLmdv) | | |
   | [sdks/go/pkg/beam/runners/vet/vet.go](https://codecov.io/gh/apache/beam/pull/16917/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-c2Rrcy9nby9wa2cvYmVhbS9ydW5uZXJzL3ZldC92ZXQuZ28=) | | |
   | [sdks/go/pkg/beam/core/runtime/harness/statemgr.go](https://codecov.io/gh/apache/beam/pull/16917/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-c2Rrcy9nby9wa2cvYmVhbS9jb3JlL3J1bnRpbWUvaGFybmVzcy9zdGF0ZW1nci5nbw==) | | |
   | [sdks/go/pkg/beam/core/runtime/exec/translate.go](https://codecov.io/gh/apache/beam/pull/16917/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-c2Rrcy9nby9wa2cvYmVhbS9jb3JlL3J1bnRpbWUvZXhlYy90cmFuc2xhdGUuZ28=) | | |
   | [sdks/go/pkg/beam/core/graph/coder/row\_decoder.go](https://codecov.io/gh/apache/beam/pull/16917/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-c2Rrcy9nby9wa2cvYmVhbS9jb3JlL2dyYXBoL2NvZGVyL3Jvd19kZWNvZGVyLmdv) | | |
   | [sdks/go/pkg/beam/forward.go](https://codecov.io/gh/apache/beam/pull/16917/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-c2Rrcy9nby9wa2cvYmVhbS9mb3J3YXJkLmdv) | | |
   | [sdks/go/pkg/beam/flatten.go](https://codecov.io/gh/apache/beam/pull/16917/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-c2Rrcy9nby9wa2cvYmVhbS9mbGF0dGVuLmdv) | | |
   | [sdks/go/pkg/beam/core/runtime/coderx/int.go](https://codecov.io/gh/apache/beam/pull/16917/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-c2Rrcy9nby9wa2cvYmVhbS9jb3JlL3J1bnRpbWUvY29kZXJ4L2ludC5nbw==) | | |
   | [sdks/go/pkg/beam/core/graph/coder/bool.go](https://codecov.io/gh/apache/beam/pull/16917/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-c2Rrcy9nby9wa2cvYmVhbS9jb3JlL2dyYXBoL2NvZGVyL2Jvb2wuZ28=) | | |
   | ... and [648 more](https://codecov.io/gh/apache/beam/pull/16917/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/16917?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/16917?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 [b2f2128...4280dfc](https://codecov.io/gh/apache/beam/pull/16917?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] robertwb commented on a change in pull request #16917: [BEAM-13972] Add RunInference interface

Posted by GitBox <gi...@apache.org>.
robertwb commented on a change in pull request #16917:
URL: https://github.com/apache/beam/pull/16917#discussion_r814167708



##########
File path: sdks/python/apache_beam/ml/inference/model.py
##########
@@ -0,0 +1,74 @@
+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements.  See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License.  You may obtain a copy of the License at
+#
+#    http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+
+import abc
+
+VALID_PYTORCH_DEVICE_TYPES = ['CPU', 'GPU']

Review comment:
       This is a stylistic choice, a question of trying to keep things local. One way to think about this is taking things to the limit, e.g. if one were to add hundreds of model types, would it make sense to enumerate all their constants here (and the classes below) or keep the constants local to the class. There's also the question of namespacing--the fact that you have one constant named _PYTORCH_ and another named _SKLEARN_ make it feel like there should be namespaces here. (Not hard and fast rules of course, but food for thought.) 
   
   The possibility of shadowing by a subclass is actually a feature--if self.VALUE is used a subclass could decide to extend this list, if cls.VALUE is used one always gets the unshadowed version. 
   
   All of this is probably moot if they are moved to their own modules. 




-- 
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 change in pull request #16917: [BEAM-13972] Add RunInference interface

Posted by GitBox <gi...@apache.org>.
yeandy commented on a change in pull request #16917:
URL: https://github.com/apache/beam/pull/16917#discussion_r814231640



##########
File path: sdks/python/apache_beam/ml/inference/model.py
##########
@@ -0,0 +1,74 @@
+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements.  See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License.  You may obtain a copy of the License at
+#
+#    http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+
+import abc
+
+VALID_PYTORCH_DEVICE_TYPES = ['CPU', 'GPU']

Review comment:
       Makes sense! and yeah I'll end up refactoring to their own modules :) 




-- 
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] edited a comment on pull request #16917: [BEAM-13972] Add RunInference interface

Posted by GitBox <gi...@apache.org>.
codecov[bot] edited a comment on pull request #16917:
URL: https://github.com/apache/beam/pull/16917#issuecomment-1047862344


   # [Codecov](https://codecov.io/gh/apache/beam/pull/16917?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 [#16917](https://codecov.io/gh/apache/beam/pull/16917?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (f73b890) into [master](https://codecov.io/gh/apache/beam/commit/b2f21281b94dc7182d15611a3e6bf1d872ea3205?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (b2f2128) will **increase** coverage by `36.74%`.
   > The diff coverage is `n/a`.
   
   > :exclamation: Current head f73b890 differs from pull request most recent head cf31b18. Consider uploading reports for the commit cf31b18 to get more accurate results
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/beam/pull/16917/graphs/tree.svg?width=650&height=150&src=pr&token=qcbbAh8Fj1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)](https://codecov.io/gh/apache/beam/pull/16917?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   
   ```diff
   @@             Coverage Diff             @@
   ##           master   #16917       +/-   ##
   ===========================================
   + Coverage   46.89%   83.64%   +36.74%     
   ===========================================
     Files         204      453      +249     
     Lines       20122    62471    +42349     
   ===========================================
   + Hits         9436    52251    +42815     
   - Misses       9686    10220      +534     
   + Partials     1000        0     -1000     
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/beam/pull/16917?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
   |---|---|---|
   | [sdks/go/pkg/beam/testing/passert/passert.go](https://codecov.io/gh/apache/beam/pull/16917/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-c2Rrcy9nby9wa2cvYmVhbS90ZXN0aW5nL3Bhc3NlcnQvcGFzc2VydC5nbw==) | | |
   | [sdks/go/pkg/beam/core/runtime/exec/status.go](https://codecov.io/gh/apache/beam/pull/16917/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-c2Rrcy9nby9wa2cvYmVhbS9jb3JlL3J1bnRpbWUvZXhlYy9zdGF0dXMuZ28=) | | |
   | [sdks/go/pkg/beam/option.go](https://codecov.io/gh/apache/beam/pull/16917/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-c2Rrcy9nby9wa2cvYmVhbS9vcHRpb24uZ28=) | | |
   | [sdks/go/pkg/beam/core/runtime/graphx/user.go](https://codecov.io/gh/apache/beam/pull/16917/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-c2Rrcy9nby9wa2cvYmVhbS9jb3JlL3J1bnRpbWUvZ3JhcGh4L3VzZXIuZ28=) | | |
   | [...o/pkg/beam/io/rtrackers/offsetrange/offsetrange.go](https://codecov.io/gh/apache/beam/pull/16917/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-c2Rrcy9nby9wa2cvYmVhbS9pby9ydHJhY2tlcnMvb2Zmc2V0cmFuZ2Uvb2Zmc2V0cmFuZ2UuZ28=) | | |
   | [...s/go/pkg/beam/core/runtime/harness/sampler\_hook.go](https://codecov.io/gh/apache/beam/pull/16917/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-c2Rrcy9nby9wa2cvYmVhbS9jb3JlL3J1bnRpbWUvaGFybmVzcy9zYW1wbGVyX2hvb2suZ28=) | | |
   | [sdks/go/pkg/beam/artifact/materialize.go](https://codecov.io/gh/apache/beam/pull/16917/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-c2Rrcy9nby9wa2cvYmVhbS9hcnRpZmFjdC9tYXRlcmlhbGl6ZS5nbw==) | | |
   | [sdks/go/pkg/beam/core/runtime/graphx/xlang.go](https://codecov.io/gh/apache/beam/pull/16917/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-c2Rrcy9nby9wa2cvYmVhbS9jb3JlL3J1bnRpbWUvZ3JhcGh4L3hsYW5nLmdv) | | |
   | [sdks/go/pkg/beam/core/funcx/output.go](https://codecov.io/gh/apache/beam/pull/16917/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-c2Rrcy9nby9wa2cvYmVhbS9jb3JlL2Z1bmN4L291dHB1dC5nbw==) | | |
   | [sdks/go/pkg/beam/core/runtime/graphx/coder.go](https://codecov.io/gh/apache/beam/pull/16917/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-c2Rrcy9nby9wa2cvYmVhbS9jb3JlL3J1bnRpbWUvZ3JhcGh4L2NvZGVyLmdv) | | |
   | ... and [647 more](https://codecov.io/gh/apache/beam/pull/16917/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/16917?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/16917?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 [b2f2128...cf31b18](https://codecov.io/gh/apache/beam/pull/16917?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] robertwb commented on a change in pull request #16917: [BEAM-13972] Add RunInference interface

Posted by GitBox <gi...@apache.org>.
robertwb commented on a change in pull request #16917:
URL: https://github.com/apache/beam/pull/16917#discussion_r819991867



##########
File path: sdks/python/apache_beam/ml/inference/api.py
##########
@@ -0,0 +1,84 @@
+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements.  See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License.  You may obtain a copy of the License at
+#
+#    http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+
+from dataclasses import dataclass
+import apache_beam as beam
+from typing import Tuple, TypeVar, Union
+# TODO: implement RunInferenceImpl
+# from apache_beam.ml.inference.base import RunInferenceImpl
+
+
+@dataclass
+class BaseModelSpec:
+  model_url: str
+
+
+@dataclass
+class PyTorchModelSpec(BaseModelSpec):

Review comment:
       Personally, I'm fine with Spec or Loader or whatever. (Agree that calling it the Model makes it feel like it's the thing that can actually do the prediction, which we don't want to instantiate until we get to the workers). 




-- 
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] robertwb commented on a change in pull request #16917: [BEAM-13972] Add RunInference interface

Posted by GitBox <gi...@apache.org>.
robertwb commented on a change in pull request #16917:
URL: https://github.com/apache/beam/pull/16917#discussion_r819984291



##########
File path: sdks/python/apache_beam/ml/inference/model.py
##########
@@ -0,0 +1,74 @@
+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements.  See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License.  You may obtain a copy of the License at
+#
+#    http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+
+import abc
+
+VALID_PYTORCH_DEVICE_TYPES = ['CPU', 'GPU']
+VALID_SKLEARN_SERIALIZATION_TYPES = ['JOBLIB', 'PICKLE']
+
+
+class RunInferenceModel(object):
+  '''
+  Base class for the RunInference Model.
+  This class contains information about
+  a model that is needed
+  '''
+  def __init__(self, model_url):
+    self._model_url = model_url
+    self._validate_model()
+
+  @abc.abstractmethod
+  def _validate_model(self):
+    raise NotImplementedError("Please implement _validate_model")
+
+
+class PytorchModel(RunInferenceModel):

Review comment:
       Let's keep comments like this open until they're done for easier tracking.

##########
File path: sdks/python/apache_beam/ml/inference/api.py
##########
@@ -0,0 +1,81 @@
+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements.  See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License.  You may obtain a copy of the License at
+#
+#    http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+
+from dataclasses import dataclass
+from enum import Enum
+from typing import Tuple, TypeVar, Union
+
+import apache_beam as beam
+
+
+class PyTorchDevice(Enum):
+  CPU = 1
+  GPU = 2
+
+
+class SklearnSerializationType(Enum):
+  PICKLE = 1
+  JOBLIB = 2
+
+
+@dataclass
+class BaseModelSpec:
+  model_uri: str

Review comment:
       Is there a need for any prescription for specific data in the baseclass? The key property of the base model spec is the methods it provides (to instantiate a runnable model) but it shouldn't dictate how that model is defined/referenced. 




-- 
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] robertwb commented on pull request #16917: [BEAM-13972] Add RunInference interface

Posted by GitBox <gi...@apache.org>.
robertwb commented on pull request #16917:
URL: https://github.com/apache/beam/pull/16917#issuecomment-1072868259


   No problem. This is a rapidly developing space.
   
   On Fri, Mar 18, 2022 at 2:43 PM tvalentyn ***@***.***> wrote:
   
   > Sorry, I misunderstood @yeandy <https://github.com/yeandy> in offline
   > conversation, I thought you already reviewed this change, @robertwb
   > <https://github.com/robertwb> .
   >
   > —
   > Reply to this email directly, view it on GitHub
   > <https://github.com/apache/beam/pull/16917#issuecomment-1072839028>, or
   > unsubscribe
   > <https://github.com/notifications/unsubscribe-auth/AADWVAP2U5IJHBLPBSIDTBLVAT2JBANCNFSM5PBQUWJQ>
   > .
   > Triage notifications on the go with GitHub Mobile for iOS
   > <https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675>
   > or Android
   > <https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub>.
   >
   > You are receiving this because you were mentioned.Message ID:
   > ***@***.***>
   >
   


-- 
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 change in pull request #16917: [BEAM-13972] Add RunInference interface

Posted by GitBox <gi...@apache.org>.
yeandy commented on a change in pull request #16917:
URL: https://github.com/apache/beam/pull/16917#discussion_r819105720



##########
File path: sdks/python/apache_beam/ml/inference/api.py
##########
@@ -0,0 +1,84 @@
+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements.  See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License.  You may obtain a copy of the License at
+#
+#    http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+
+from dataclasses import dataclass
+from enum import Enum
+from typing import Tuple, TypeVar, Union
+
+import apache_beam as beam
+
+
+class PyTorchDevice(Enum):
+  CPU = 1
+  GPU = 2
+
+
+class SklearnSerializationType(Enum):
+  PICKLE = 1
+  JOBLIB = 2
+
+
+@dataclass
+class BaseModelSpec:
+  model_url: str
+
+
+@dataclass
+class PyTorchModelSpec(BaseModelSpec):
+  device: PyTorchDevice
+
+
+@dataclass
+class SklearnModelSpec(BaseModelSpec):
+  serialization_type: SklearnSerializationType
+
+
+_K = TypeVar('_K')
+_INPUT_TYPE = TypeVar('_INPUT_TYPE')
+_OUTPUT_TYPE = TypeVar('_OUTPUT_TYPE')
+
+
+@dataclass
+class PredictionResult:
+  example: _INPUT_TYPE
+  inference: _OUTPUT_TYPE
+
+
+@beam.ptransform_fn
+@beam.typehints.with_input_types(Union[_INPUT_TYPE, Tuple[_K, _INPUT_TYPE]])
+@beam.typehints.with_output_types(
+    Union[PredictionResult, Tuple[_K, PredictionResult]])
+def RunInference(
+    examples: beam.pvalue.PCollection,
+    model: BaseModelSpec) -> beam.pvalue.PCollection:
+  """Run inference with a model.
+
+  There one type of inference you can perform using this PTransform:
+    1. In-process inference from a SavedModel instance.

Review comment:
       I've removed this 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] codecov[bot] edited a comment on pull request #16917: [BEAM-13972] Add RunInference interface

Posted by GitBox <gi...@apache.org>.
codecov[bot] edited a comment on pull request #16917:
URL: https://github.com/apache/beam/pull/16917#issuecomment-1047862344


   # [Codecov](https://codecov.io/gh/apache/beam/pull/16917?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 [#16917](https://codecov.io/gh/apache/beam/pull/16917?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (b0cea39) into [master](https://codecov.io/gh/apache/beam/commit/b2f21281b94dc7182d15611a3e6bf1d872ea3205?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (b2f2128) will **increase** coverage by `36.75%`.
   > The diff coverage is `96.77%`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/beam/pull/16917/graphs/tree.svg?width=650&height=150&src=pr&token=qcbbAh8Fj1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)](https://codecov.io/gh/apache/beam/pull/16917?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   
   ```diff
   @@             Coverage Diff             @@
   ##           master   #16917       +/-   ##
   ===========================================
   + Coverage   46.89%   83.64%   +36.75%     
   ===========================================
     Files         204      454      +250     
     Lines       20122    62593    +42471     
   ===========================================
   + Hits         9436    52357    +42921     
   - Misses       9686    10236      +550     
   + Partials     1000        0     -1000     
   ```
   
   | Flag | Coverage Δ | |
   |---|---|---|
   | python | `83.64% <96.77%> (?)` | |
   
   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/16917?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
   |---|---|---|
   | [sdks/python/apache\_beam/ml/inference/api.py](https://codecov.io/gh/apache/beam/pull/16917/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-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vbWwvaW5mZXJlbmNlL2FwaS5weQ==) | `96.77% <96.77%> (ø)` | |
   | [sdks/go/pkg/beam/core/typex/class.go](https://codecov.io/gh/apache/beam/pull/16917/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-c2Rrcy9nby9wa2cvYmVhbS9jb3JlL3R5cGV4L2NsYXNzLmdv) | | |
   | [sdks/go/pkg/beam/core/runtime/exec/datasource.go](https://codecov.io/gh/apache/beam/pull/16917/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-c2Rrcy9nby9wa2cvYmVhbS9jb3JlL3J1bnRpbWUvZXhlYy9kYXRhc291cmNlLmdv) | | |
   | [sdks/go/pkg/beam/provision/provision.go](https://codecov.io/gh/apache/beam/pull/16917/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-c2Rrcy9nby9wa2cvYmVhbS9wcm92aXNpb24vcHJvdmlzaW9uLmdv) | | |
   | [sdks/go/pkg/beam/core/runtime/graphx/coder.go](https://codecov.io/gh/apache/beam/pull/16917/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-c2Rrcy9nby9wa2cvYmVhbS9jb3JlL3J1bnRpbWUvZ3JhcGh4L2NvZGVyLmdv) | | |
   | [sdks/go/pkg/beam/transforms/top/top.go](https://codecov.io/gh/apache/beam/pull/16917/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-c2Rrcy9nby9wa2cvYmVhbS90cmFuc2Zvcm1zL3RvcC90b3AuZ28=) | | |
   | [sdks/go/pkg/beam/transforms/stats/sum\_switch.go](https://codecov.io/gh/apache/beam/pull/16917/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-c2Rrcy9nby9wa2cvYmVhbS90cmFuc2Zvcm1zL3N0YXRzL3N1bV9zd2l0Y2guZ28=) | | |
   | [sdks/go/pkg/beam/runners/direct/gbk.go](https://codecov.io/gh/apache/beam/pull/16917/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-c2Rrcy9nby9wa2cvYmVhbS9ydW5uZXJzL2RpcmVjdC9nYmsuZ28=) | | |
   | [...kg/beam/core/runtime/graphx/schema/logicaltypes.go](https://codecov.io/gh/apache/beam/pull/16917/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-c2Rrcy9nby9wa2cvYmVhbS9jb3JlL3J1bnRpbWUvZ3JhcGh4L3NjaGVtYS9sb2dpY2FsdHlwZXMuZ28=) | | |
   | [sdks/go/pkg/beam/core/runtime/xlangx/payload.go](https://codecov.io/gh/apache/beam/pull/16917/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-c2Rrcy9nby9wa2cvYmVhbS9jb3JlL3J1bnRpbWUveGxhbmd4L3BheWxvYWQuZ28=) | | |
   | ... and [648 more](https://codecov.io/gh/apache/beam/pull/16917/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/16917?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/16917?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 [b2f2128...b0cea39](https://codecov.io/gh/apache/beam/pull/16917?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] ryanthompson591 commented on a change in pull request #16917: [BEAM-13972] Add RunInference interface

Posted by GitBox <gi...@apache.org>.
ryanthompson591 commented on a change in pull request #16917:
URL: https://github.com/apache/beam/pull/16917#discussion_r819685265



##########
File path: sdks/python/apache_beam/ml/inference/api.py
##########
@@ -0,0 +1,81 @@
+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements.  See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License.  You may obtain a copy of the License at
+#
+#    http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+
+from dataclasses import dataclass
+from enum import Enum
+from typing import Tuple, TypeVar, Union
+
+import apache_beam as beam
+
+
+class PyTorchDevice(Enum):
+  CPU = 1
+  GPU = 2
+
+
+class SklearnSerializationType(Enum):
+  PICKLE = 1
+  JOBLIB = 2
+
+
+@dataclass
+class BaseModelSpec:
+  model_uri: str

Review comment:
       I've been thinking about this.  What if the model is hosted on some service that requires a more complex loading strategy, like gcs.
   
   https://stackoverflow.com/questions/45920895/read-a-file-from-gcs-in-apache-beam
   
   Will the model_uri be adequate for us?




-- 
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] edited a comment on pull request #16917: [BEAM-13972] Add RunInference interface

Posted by GitBox <gi...@apache.org>.
codecov[bot] edited a comment on pull request #16917:
URL: https://github.com/apache/beam/pull/16917#issuecomment-1047862344


   # [Codecov](https://codecov.io/gh/apache/beam/pull/16917?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 [#16917](https://codecov.io/gh/apache/beam/pull/16917?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (e5c18d9) into [master](https://codecov.io/gh/apache/beam/commit/b2f21281b94dc7182d15611a3e6bf1d872ea3205?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (b2f2128) will **increase** coverage by `36.73%`.
   > The diff coverage is `96.96%`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/beam/pull/16917/graphs/tree.svg?width=650&height=150&src=pr&token=qcbbAh8Fj1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)](https://codecov.io/gh/apache/beam/pull/16917?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   
   ```diff
   @@             Coverage Diff             @@
   ##           master   #16917       +/-   ##
   ===========================================
   + Coverage   46.89%   83.63%   +36.73%     
   ===========================================
     Files         204      454      +250     
     Lines       20122    62595    +42473     
   ===========================================
   + Hits         9436    52349    +42913     
   - Misses       9686    10246      +560     
   + Partials     1000        0     -1000     
   ```
   
   | Flag | Coverage Δ | |
   |---|---|---|
   | python | `83.63% <96.96%> (?)` | |
   
   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/16917?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
   |---|---|---|
   | [sdks/python/apache\_beam/ml/inference/api.py](https://codecov.io/gh/apache/beam/pull/16917/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-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vbWwvaW5mZXJlbmNlL2FwaS5weQ==) | `96.96% <96.96%> (ø)` | |
   | [...o/pkg/beam/runners/dataflow/dataflowlib/execute.go](https://codecov.io/gh/apache/beam/pull/16917/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-c2Rrcy9nby9wa2cvYmVhbS9ydW5uZXJzL2RhdGFmbG93L2RhdGFmbG93bGliL2V4ZWN1dGUuZ28=) | | |
   | [sdks/go/pkg/beam/runner.go](https://codecov.io/gh/apache/beam/pull/16917/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-c2Rrcy9nby9wa2cvYmVhbS9ydW5uZXIuZ28=) | | |
   | [sdks/go/pkg/beam/pcollection.go](https://codecov.io/gh/apache/beam/pull/16917/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-c2Rrcy9nby9wa2cvYmVhbS9wY29sbGVjdGlvbi5nbw==) | | |
   | [sdks/go/pkg/beam/flatten.go](https://codecov.io/gh/apache/beam/pull/16917/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-c2Rrcy9nby9wa2cvYmVhbS9mbGF0dGVuLmdv) | | |
   | [sdks/go/pkg/beam/core/runtime/exec/coder.go](https://codecov.io/gh/apache/beam/pull/16917/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-c2Rrcy9nby9wa2cvYmVhbS9jb3JlL3J1bnRpbWUvZXhlYy9jb2Rlci5nbw==) | | |
   | [sdks/go/pkg/beam/util/harnessopts/cache.go](https://codecov.io/gh/apache/beam/pull/16917/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-c2Rrcy9nby9wa2cvYmVhbS91dGlsL2hhcm5lc3NvcHRzL2NhY2hlLmdv) | | |
   | [sdks/go/pkg/beam/combine.go](https://codecov.io/gh/apache/beam/pull/16917/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-c2Rrcy9nby9wa2cvYmVhbS9jb21iaW5lLmdv) | | |
   | [sdks/go/pkg/beam/partition.go](https://codecov.io/gh/apache/beam/pull/16917/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-c2Rrcy9nby9wa2cvYmVhbS9wYXJ0aXRpb24uZ28=) | | |
   | [sdks/go/pkg/beam/options/gcpopts/options.go](https://codecov.io/gh/apache/beam/pull/16917/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-c2Rrcy9nby9wa2cvYmVhbS9vcHRpb25zL2djcG9wdHMvb3B0aW9ucy5nbw==) | | |
   | ... and [648 more](https://codecov.io/gh/apache/beam/pull/16917/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/16917?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/16917?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 [b2f2128...e5c18d9](https://codecov.io/gh/apache/beam/pull/16917?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] ryanthompson591 commented on a change in pull request #16917: [BEAM-13972] Add RunInference interface

Posted by GitBox <gi...@apache.org>.
ryanthompson591 commented on a change in pull request #16917:
URL: https://github.com/apache/beam/pull/16917#discussion_r819685265



##########
File path: sdks/python/apache_beam/ml/inference/api.py
##########
@@ -0,0 +1,81 @@
+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements.  See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License.  You may obtain a copy of the License at
+#
+#    http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+
+from dataclasses import dataclass
+from enum import Enum
+from typing import Tuple, TypeVar, Union
+
+import apache_beam as beam
+
+
+class PyTorchDevice(Enum):
+  CPU = 1
+  GPU = 2
+
+
+class SklearnSerializationType(Enum):
+  PICKLE = 1
+  JOBLIB = 2
+
+
+@dataclass
+class BaseModelSpec:
+  model_uri: str

Review comment:
       I've been thinking about this.  What if the model is hosted on some service that requires a more complex loading strategy, like gcs.
   
   https://stackoverflow.com/questions/45920895/read-a-file-from-gcs-in-apache-beam
   
   Will the model_uri as a string alone be adequate for us?  Could there be services that need the URI and a key. Or could there be a case where the model itself could come from a transform.




-- 
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 change in pull request #16917: [BEAM-13972] Add RunInference interface

Posted by GitBox <gi...@apache.org>.
yeandy commented on a change in pull request #16917:
URL: https://github.com/apache/beam/pull/16917#discussion_r817952152



##########
File path: sdks/python/apache_beam/ml/inference/api.py
##########
@@ -0,0 +1,84 @@
+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements.  See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License.  You may obtain a copy of the License at
+#
+#    http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+
+from dataclasses import dataclass
+import apache_beam as beam
+from typing import Tuple, TypeVar, Union
+# TODO: implement RunInferenceImpl
+# from apache_beam.ml.inference.base import RunInferenceImpl
+
+
+@dataclass
+class BaseModelSpec:
+  model_url: str
+
+
+@dataclass
+class PyTorchModelSpec(BaseModelSpec):
+  device: str
+
+  def __post_init__(self):
+    self.device = self.device.upper()
+
+
+@dataclass
+class SklearnModelSpec(BaseModelSpec):
+  serialization_type: str
+
+  def __post_init__(self):
+    self.serialization_type = self.serialization_type.upper()
+
+
+_K = TypeVar('_K')
+_INPUT_TYPE = TypeVar('_INPUT_TYPE')
+_OUTPUT_TYPE = TypeVar('_OUTPUT_TYPE')
+
+
+@dataclass
+class PredictionResult:
+  key: _K

Review comment:
       You're right, my bad!




-- 
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] edited a comment on pull request #16917: [BEAM-13972] Add RunInference interface

Posted by GitBox <gi...@apache.org>.
codecov[bot] edited a comment on pull request #16917:
URL: https://github.com/apache/beam/pull/16917#issuecomment-1047862344


   # [Codecov](https://codecov.io/gh/apache/beam/pull/16917?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 [#16917](https://codecov.io/gh/apache/beam/pull/16917?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (b0cea39) into [master](https://codecov.io/gh/apache/beam/commit/b2f21281b94dc7182d15611a3e6bf1d872ea3205?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (b2f2128) will **increase** coverage by `36.75%`.
   > The diff coverage is `96.77%`.
   
   > :exclamation: Current head b0cea39 differs from pull request most recent head e5c18d9. Consider uploading reports for the commit e5c18d9 to get more accurate results
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/beam/pull/16917/graphs/tree.svg?width=650&height=150&src=pr&token=qcbbAh8Fj1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)](https://codecov.io/gh/apache/beam/pull/16917?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   
   ```diff
   @@             Coverage Diff             @@
   ##           master   #16917       +/-   ##
   ===========================================
   + Coverage   46.89%   83.64%   +36.75%     
   ===========================================
     Files         204      454      +250     
     Lines       20122    62593    +42471     
   ===========================================
   + Hits         9436    52357    +42921     
   - Misses       9686    10236      +550     
   + Partials     1000        0     -1000     
   ```
   
   | Flag | Coverage Δ | |
   |---|---|---|
   | python | `83.64% <96.77%> (?)` | |
   
   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/16917?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
   |---|---|---|
   | [sdks/python/apache\_beam/ml/inference/api.py](https://codecov.io/gh/apache/beam/pull/16917/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-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vbWwvaW5mZXJlbmNlL2FwaS5weQ==) | `96.77% <96.77%> (ø)` | |
   | [sdks/go/pkg/beam/core/runtime/harness/harness.go](https://codecov.io/gh/apache/beam/pull/16917/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-c2Rrcy9nby9wa2cvYmVhbS9jb3JlL3J1bnRpbWUvaGFybmVzcy9oYXJuZXNzLmdv) | | |
   | [sdks/go/pkg/beam/core/runtime/coderx/varint.go](https://codecov.io/gh/apache/beam/pull/16917/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-c2Rrcy9nby9wa2cvYmVhbS9jb3JlL3J1bnRpbWUvY29kZXJ4L3ZhcmludC5nbw==) | | |
   | [sdks/go/pkg/beam/io/filesystem/gcs/gcs.go](https://codecov.io/gh/apache/beam/pull/16917/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-c2Rrcy9nby9wa2cvYmVhbS9pby9maWxlc3lzdGVtL2djcy9nY3MuZ28=) | | |
   | [sdks/go/pkg/beam/util.go](https://codecov.io/gh/apache/beam/pull/16917/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-c2Rrcy9nby9wa2cvYmVhbS91dGlsLmdv) | | |
   | [sdks/go/pkg/beam/core/util/reflectx/util.go](https://codecov.io/gh/apache/beam/pull/16917/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-c2Rrcy9nby9wa2cvYmVhbS9jb3JlL3V0aWwvcmVmbGVjdHgvdXRpbC5nbw==) | | |
   | [sdks/go/pkg/beam/core/runtime/exec/discard.go](https://codecov.io/gh/apache/beam/pull/16917/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-c2Rrcy9nby9wa2cvYmVhbS9jb3JlL3J1bnRpbWUvZXhlYy9kaXNjYXJkLmdv) | | |
   | [sdks/go/pkg/beam/combine.go](https://codecov.io/gh/apache/beam/pull/16917/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-c2Rrcy9nby9wa2cvYmVhbS9jb21iaW5lLmdv) | | |
   | [sdks/go/pkg/beam/core/util/reflectx/functions.go](https://codecov.io/gh/apache/beam/pull/16917/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-c2Rrcy9nby9wa2cvYmVhbS9jb3JlL3V0aWwvcmVmbGVjdHgvZnVuY3Rpb25zLmdv) | | |
   | [sdks/go/pkg/beam/io/filesystem/local/local.go](https://codecov.io/gh/apache/beam/pull/16917/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-c2Rrcy9nby9wa2cvYmVhbS9pby9maWxlc3lzdGVtL2xvY2FsL2xvY2FsLmdv) | | |
   | ... and [648 more](https://codecov.io/gh/apache/beam/pull/16917/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/16917?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/16917?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 [b2f2128...e5c18d9](https://codecov.io/gh/apache/beam/pull/16917?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 change in pull request #16917: [BEAM-13972] Add RunInference interface

Posted by GitBox <gi...@apache.org>.
yeandy commented on a change in pull request #16917:
URL: https://github.com/apache/beam/pull/16917#discussion_r819918468



##########
File path: sdks/python/apache_beam/ml/inference/api.py
##########
@@ -0,0 +1,81 @@
+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements.  See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License.  You may obtain a copy of the License at
+#
+#    http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+
+from dataclasses import dataclass
+from enum import Enum
+from typing import Tuple, TypeVar, Union
+
+import apache_beam as beam
+
+
+class PyTorchDevice(Enum):
+  CPU = 1
+  GPU = 2
+
+
+class SklearnSerializationType(Enum):
+  PICKLE = 1
+  JOBLIB = 2
+
+
+@dataclass
+class BaseModelSpec:
+  model_uri: str

Review comment:
       Could we, as future feature requests, add a wrappers to support these different services? 
   e.g.
   ```
   class RemoteServiceType(enum):
       beam_file_systems = 1
       gcs_client = 2  
   ```




-- 
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 #16917: [BEAM-13972] Add RunInference interface

Posted by GitBox <gi...@apache.org>.
tvalentyn commented on pull request #16917:
URL: https://github.com/apache/beam/pull/16917#issuecomment-1072839028


   Sorry, I misunderstood @yeandy in offline conversation, I thought you already reviewed this change, @robertwb .


-- 
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] robertwb commented on pull request #16917: [BEAM-13972] Add RunInference interface

Posted by GitBox <gi...@apache.org>.
robertwb commented on pull request #16917:
URL: https://github.com/apache/beam/pull/16917#issuecomment-1072812640


   I'm actively involved in the review within the TFX codebase. I'm assuming, however, that the two will be reconciled. (And, generally, +1 to @aaltay 's advice about pining reviews that are lagging.)


-- 
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 merged pull request #16917: [BEAM-13972] Add RunInference interface

Posted by GitBox <gi...@apache.org>.
tvalentyn merged pull request #16917:
URL: https://github.com/apache/beam/pull/16917


   


-- 
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 change in pull request #16917: [BEAM-13972] Add RunInference interface

Posted by GitBox <gi...@apache.org>.
AnandInguva commented on a change in pull request #16917:
URL: https://github.com/apache/beam/pull/16917#discussion_r812187752



##########
File path: sdks/python/apache_beam/ml/inference/model.py
##########
@@ -0,0 +1,74 @@
+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements.  See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License.  You may obtain a copy of the License at
+#
+#    http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+
+import abc
+
+VALID_PYTORCH_DEVICE_TYPES = ['CPU', 'GPU']
+VALID_SKLEARN_SERIALIZATION_TYPES = ['JOBLIB', 'PICKLE']
+
+
+class RunInferenceModel(object):
+  '''
+  Base class for the RunInference Model.
+  This class contains information about
+  a model that is needed
+  '''
+  def __init__(self, model_url):
+    self._model_url = model_url
+    self._validate_model()
+
+  @abc.abstractmethod
+  def _validate_model(self):
+    raise NotImplementedError("Please implement _validate_model")
+
+
+class PytorchModel(RunInferenceModel):
+  '''
+  This class wraps the PyTorch model, and other
+  PyTorch-specific parameters
+  '''
+  def __init__(self, model_url: str, device: str):
+    super().__init__(model_url)
+    self._device = device
+    self._validate_device()
+
+  def _validate_model(self):
+    pass
+
+  def _validate_device(self):
+    if self._device not in VALID_PYTORCH_DEVICE_TYPES:

Review comment:
       user may provide `device=cpu`, all letters in Smalls. 
   1. We can convert `self._device` to `self._device.upper()` to match the names in the device list
   




-- 
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 change in pull request #16917: [BEAM-13972] Add RunInference interface

Posted by GitBox <gi...@apache.org>.
yeandy commented on a change in pull request #16917:
URL: https://github.com/apache/beam/pull/16917#discussion_r814079447



##########
File path: sdks/python/apache_beam/ml/inference/model.py
##########
@@ -0,0 +1,74 @@
+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements.  See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License.  You may obtain a copy of the License at
+#
+#    http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+
+import abc
+
+VALID_PYTORCH_DEVICE_TYPES = ['CPU', 'GPU']
+VALID_SKLEARN_SERIALIZATION_TYPES = ['JOBLIB', 'PICKLE']
+
+
+class RunInferenceModel(object):
+  '''
+  Base class for the RunInference Model.
+  This class contains information about
+  a model that is needed
+  '''
+  def __init__(self, model_url):
+    self._model_url = model_url
+    self._validate_model()
+
+  @abc.abstractmethod
+  def _validate_model(self):
+    raise NotImplementedError("Please implement _validate_model")
+
+
+class PytorchModel(RunInferenceModel):

Review comment:
       Agreed. I'll refactor.




-- 
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 change in pull request #16917: [BEAM-13972] Add RunInference interface

Posted by GitBox <gi...@apache.org>.
ryanthompson591 commented on a change in pull request #16917:
URL: https://github.com/apache/beam/pull/16917#discussion_r817907842



##########
File path: sdks/python/apache_beam/ml/inference/api.py
##########
@@ -0,0 +1,84 @@
+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements.  See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License.  You may obtain a copy of the License at
+#
+#    http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+
+from dataclasses import dataclass
+import apache_beam as beam
+from typing import Tuple, TypeVar, Union
+# TODO: implement RunInferenceImpl

Review comment:
       I would just remove these comments for now. We'll remember them in the next PR.

##########
File path: sdks/python/apache_beam/ml/inference/api.py
##########
@@ -0,0 +1,84 @@
+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements.  See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License.  You may obtain a copy of the License at
+#
+#    http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+
+from dataclasses import dataclass
+import apache_beam as beam
+from typing import Tuple, TypeVar, Union
+# TODO: implement RunInferenceImpl
+# from apache_beam.ml.inference.base import RunInferenceImpl
+
+
+@dataclass
+class BaseModelSpec:
+  model_url: str
+
+
+@dataclass
+class PyTorchModelSpec(BaseModelSpec):
+  device: str
+
+  def __post_init__(self):
+    self.device = self.device.upper()
+
+
+@dataclass
+class SklearnModelSpec(BaseModelSpec):
+  serialization_type: str
+
+  def __post_init__(self):

Review comment:
       I think an enum would remove the need for this code entirely.

##########
File path: sdks/python/apache_beam/ml/inference/api.py
##########
@@ -0,0 +1,84 @@
+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements.  See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License.  You may obtain a copy of the License at
+#
+#    http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+
+from dataclasses import dataclass
+import apache_beam as beam
+from typing import Tuple, TypeVar, Union
+# TODO: implement RunInferenceImpl
+# from apache_beam.ml.inference.base import RunInferenceImpl
+
+
+@dataclass
+class BaseModelSpec:
+  model_url: str
+
+
+@dataclass
+class PyTorchModelSpec(BaseModelSpec):
+  device: str
+
+  def __post_init__(self):
+    self.device = self.device.upper()
+
+
+@dataclass
+class SklearnModelSpec(BaseModelSpec):
+  serialization_type: str
+
+  def __post_init__(self):
+    self.serialization_type = self.serialization_type.upper()
+
+
+_K = TypeVar('_K')
+_INPUT_TYPE = TypeVar('_INPUT_TYPE')
+_OUTPUT_TYPE = TypeVar('_OUTPUT_TYPE')
+
+
+@dataclass
+class PredictionResult:
+  key: _K

Review comment:
       I don't think key should be in prediciton result. If my understanding of beam is correct then keys will be in tuples (since other beam transforms expect that pattern).
   
   So keyed output would look something like:
   [(key1, PredictionResult(example, inference)), (key2, PredictionResult(....)]

##########
File path: sdks/python/apache_beam/ml/inference/api.py
##########
@@ -0,0 +1,84 @@
+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements.  See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License.  You may obtain a copy of the License at
+#
+#    http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+
+from dataclasses import dataclass
+import apache_beam as beam
+from typing import Tuple, TypeVar, Union
+# TODO: implement RunInferenceImpl
+# from apache_beam.ml.inference.base import RunInferenceImpl
+
+
+@dataclass
+class BaseModelSpec:
+  model_url: str
+
+
+@dataclass
+class PyTorchModelSpec(BaseModelSpec):
+  device: str
+
+  def __post_init__(self):
+    self.device = self.device.upper()
+
+
+@dataclass
+class SklearnModelSpec(BaseModelSpec):
+  serialization_type: str
+
+  def __post_init__(self):
+    self.serialization_type = self.serialization_type.upper()
+
+
+_K = TypeVar('_K')
+_INPUT_TYPE = TypeVar('_INPUT_TYPE')
+_OUTPUT_TYPE = TypeVar('_OUTPUT_TYPE')
+
+
+@dataclass
+class PredictionResult:
+  key: _K
+  example: _INPUT_TYPE
+  inference: _OUTPUT_TYPE
+
+
+@beam.ptransform_fn
+@beam.typehints.with_input_types(Union[_INPUT_TYPE, Tuple[_K, _INPUT_TYPE]])
+@beam.typehints.with_output_types(PredictionResult)
+def RunInference(
+    examples: beam.pvalue.PCollection,
+    model: BaseModelSpec) -> beam.pvalue.PCollection:
+  """Run inference with a model.
+
+  There one type of inference you can perform using this PTransform:
+    1. In-process inference from a SavedModel instance.
+    TODO: Add remote inference by using a service endpoint.
+
+  Args:
+    examples: A PCollection containing examples of the following possible kinds,
+      each with their corresponding return type.
+        - PCollection[Example]                 -> PCollection[PredictionResult]
+        - PCollection[Tuple[K, Example]]       -> PCollection[
+                                                    Tuple[K, PredictionResult]]
+    model: Model inference endpoint.
+  Returns:
+    A PCollection (possibly keyed) containing PredictionResults.
+  """
+  pass
+  # TODO: implement RunInferenceImpl

Review comment:
       I would just leave these comments out for now, we know we'll be adding this.
   
   something like
   pass #TODO add implementation.

##########
File path: sdks/python/apache_beam/ml/inference/api.py
##########
@@ -0,0 +1,84 @@
+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements.  See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License.  You may obtain a copy of the License at
+#
+#    http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+
+from dataclasses import dataclass
+import apache_beam as beam
+from typing import Tuple, TypeVar, Union
+# TODO: implement RunInferenceImpl
+# from apache_beam.ml.inference.base import RunInferenceImpl
+
+
+@dataclass
+class BaseModelSpec:
+  model_url: str
+
+
+@dataclass
+class PyTorchModel(BaseModelSpec):
+  device: str
+
+  def __post_init__(self):
+    self.device = self.device.upper()
+
+
+@dataclass
+class SklearnModel(BaseModelSpec):
+  serialization_type: str
+
+  def __post_init__(self):
+    self.serialization_type = self.serialization_type.upper()
+
+
+_K = TypeVar('_K')
+_INPUT_TYPE = TypeVar('_INPUT_TYPE')
+_OUTPUT_TYPE = TypeVar('_OUTPUT_TYPE')
+
+
+@dataclass
+class PredictionResult:
+  key: _K
+  example: _INPUT_TYPE
+  inference: _OUTPUT_TYPE
+
+
+@beam.ptransform_fn
+@beam.typehints.with_input_types(Union[_INPUT_TYPE, Tuple[_K, _INPUT_TYPE]])
+@beam.typehints.with_output_types(PredictionResult)

Review comment:
       yeah, TFX is doing it right I think.  They either have output type or a key with output type.
   
   I'm not the typing expert, IMO its ok to have a typing specific PR that just concentrates on that too, if this gets messy.
   
   We need typing to support TFX's proto use case as well as ours new frameworks use case.




-- 
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 change in pull request #16917: [BEAM-13972] Add RunInference interface

Posted by GitBox <gi...@apache.org>.
yeandy commented on a change in pull request #16917:
URL: https://github.com/apache/beam/pull/16917#discussion_r817934027



##########
File path: sdks/python/apache_beam/ml/inference/api.py
##########
@@ -0,0 +1,84 @@
+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements.  See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License.  You may obtain a copy of the License at
+#
+#    http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+
+from dataclasses import dataclass
+import apache_beam as beam
+from typing import Tuple, TypeVar, Union
+# TODO: implement RunInferenceImpl

Review comment:
       SG

##########
File path: sdks/python/apache_beam/ml/inference/api.py
##########
@@ -0,0 +1,84 @@
+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements.  See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License.  You may obtain a copy of the License at
+#
+#    http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+
+from dataclasses import dataclass
+import apache_beam as beam
+from typing import Tuple, TypeVar, Union
+# TODO: implement RunInferenceImpl
+# from apache_beam.ml.inference.base import RunInferenceImpl
+
+
+@dataclass
+class BaseModelSpec:
+  model_url: str
+
+
+@dataclass
+class PyTorchModelSpec(BaseModelSpec):
+  device: str
+
+  def __post_init__(self):
+    self.device = self.device.upper()
+
+
+@dataclass
+class SklearnModelSpec(BaseModelSpec):
+  serialization_type: str
+
+  def __post_init__(self):
+    self.serialization_type = self.serialization_type.upper()
+
+
+_K = TypeVar('_K')
+_INPUT_TYPE = TypeVar('_INPUT_TYPE')
+_OUTPUT_TYPE = TypeVar('_OUTPUT_TYPE')
+
+
+@dataclass
+class PredictionResult:
+  key: _K
+  example: _INPUT_TYPE
+  inference: _OUTPUT_TYPE
+
+
+@beam.ptransform_fn
+@beam.typehints.with_input_types(Union[_INPUT_TYPE, Tuple[_K, _INPUT_TYPE]])
+@beam.typehints.with_output_types(PredictionResult)
+def RunInference(
+    examples: beam.pvalue.PCollection,
+    model: BaseModelSpec) -> beam.pvalue.PCollection:
+  """Run inference with a model.
+
+  There one type of inference you can perform using this PTransform:
+    1. In-process inference from a SavedModel instance.
+    TODO: Add remote inference by using a service endpoint.
+
+  Args:
+    examples: A PCollection containing examples of the following possible kinds,
+      each with their corresponding return type.
+        - PCollection[Example]                 -> PCollection[PredictionResult]
+        - PCollection[Tuple[K, Example]]       -> PCollection[
+                                                    Tuple[K, PredictionResult]]
+    model: Model inference endpoint.
+  Returns:
+    A PCollection (possibly keyed) containing PredictionResults.
+  """
+  pass
+  # TODO: implement RunInferenceImpl

Review comment:
       SG




-- 
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 change in pull request #16917: [BEAM-13972] Add RunInference interface

Posted by GitBox <gi...@apache.org>.
yeandy commented on a change in pull request #16917:
URL: https://github.com/apache/beam/pull/16917#discussion_r817202478



##########
File path: sdks/python/apache_beam/ml/inference/api.py
##########
@@ -0,0 +1,84 @@
+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements.  See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License.  You may obtain a copy of the License at
+#
+#    http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+
+from dataclasses import dataclass
+import apache_beam as beam
+from typing import Tuple, TypeVar, Union
+# TODO: implement RunInferenceImpl
+# from apache_beam.ml.inference.base import RunInferenceImpl
+
+
+@dataclass
+class BaseModelSpec:
+  model_url: str
+
+
+@dataclass
+class PyTorchModel(BaseModelSpec):

Review comment:
       Renamed `PytorchModel` to `PytorchModelSpec` since we renamed the old `ModelSpec` to `ModelLoader`. What do you think @ryanthompson591 ?




-- 
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 change in pull request #16917: [BEAM-13972] Add RunInference interface

Posted by GitBox <gi...@apache.org>.
yeandy commented on a change in pull request #16917:
URL: https://github.com/apache/beam/pull/16917#discussion_r819105931



##########
File path: sdks/python/apache_beam/ml/inference/api.py
##########
@@ -0,0 +1,84 @@
+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements.  See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License.  You may obtain a copy of the License at
+#
+#    http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+
+from dataclasses import dataclass
+from enum import Enum
+from typing import Tuple, TypeVar, Union
+
+import apache_beam as beam
+
+
+class PyTorchDevice(Enum):
+  CPU = 1
+  GPU = 2
+
+
+class SklearnSerializationType(Enum):
+  PICKLE = 1
+  JOBLIB = 2
+
+
+@dataclass
+class BaseModelSpec:
+  model_url: str
+
+
+@dataclass
+class PyTorchModelSpec(BaseModelSpec):
+  device: PyTorchDevice
+
+
+@dataclass
+class SklearnModelSpec(BaseModelSpec):
+  serialization_type: SklearnSerializationType
+
+
+_K = TypeVar('_K')
+_INPUT_TYPE = TypeVar('_INPUT_TYPE')
+_OUTPUT_TYPE = TypeVar('_OUTPUT_TYPE')
+
+
+@dataclass
+class PredictionResult:
+  example: _INPUT_TYPE
+  inference: _OUTPUT_TYPE
+
+
+@beam.ptransform_fn
+@beam.typehints.with_input_types(Union[_INPUT_TYPE, Tuple[_K, _INPUT_TYPE]])
+@beam.typehints.with_output_types(
+    Union[PredictionResult, Tuple[_K, PredictionResult]])
+def RunInference(
+    examples: beam.pvalue.PCollection,
+    model: BaseModelSpec) -> beam.pvalue.PCollection:
+  """Run inference with a model.

Review comment:
       Thanks, I've updated the comments with your suggestion!

##########
File path: sdks/python/apache_beam/ml/inference/api.py
##########
@@ -0,0 +1,84 @@
+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements.  See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License.  You may obtain a copy of the License at
+#
+#    http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+
+from dataclasses import dataclass
+from enum import Enum
+from typing import Tuple, TypeVar, Union
+
+import apache_beam as beam
+
+
+class PyTorchDevice(Enum):
+  CPU = 1
+  GPU = 2
+
+
+class SklearnSerializationType(Enum):
+  PICKLE = 1
+  JOBLIB = 2
+
+
+@dataclass
+class BaseModelSpec:
+  model_url: str
+
+
+@dataclass
+class PyTorchModelSpec(BaseModelSpec):
+  device: PyTorchDevice
+
+
+@dataclass
+class SklearnModelSpec(BaseModelSpec):
+  serialization_type: SklearnSerializationType
+
+
+_K = TypeVar('_K')
+_INPUT_TYPE = TypeVar('_INPUT_TYPE')
+_OUTPUT_TYPE = TypeVar('_OUTPUT_TYPE')
+
+
+@dataclass
+class PredictionResult:
+  example: _INPUT_TYPE
+  inference: _OUTPUT_TYPE
+
+
+@beam.ptransform_fn
+@beam.typehints.with_input_types(Union[_INPUT_TYPE, Tuple[_K, _INPUT_TYPE]])
+@beam.typehints.with_output_types(
+    Union[PredictionResult, Tuple[_K, PredictionResult]])
+def RunInference(
+    examples: beam.pvalue.PCollection,
+    model: BaseModelSpec) -> beam.pvalue.PCollection:
+  """Run inference with a model.

Review comment:
       Thanks, I've updated the documentation with your suggestion!




-- 
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] aaltay commented on pull request #16917: [BEAM-13972] Add RunInference interface

Posted by GitBox <gi...@apache.org>.
aaltay commented on pull request #16917:
URL: https://github.com/apache/beam/pull/16917#issuecomment-1071912825


   @robertwb - could you please review this PR?
   
   Other folks on the PR, feel free to ping reviewers here or in other places if a review is open for more than a few business days. And where it makes sense you could also ask other committers to review PRs and spread the review load across committers.


-- 
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] edited a comment on pull request #16917: [BEAM-13972] Add RunInference interface

Posted by GitBox <gi...@apache.org>.
codecov[bot] edited a comment on pull request #16917:
URL: https://github.com/apache/beam/pull/16917#issuecomment-1047862344


   # [Codecov](https://codecov.io/gh/apache/beam/pull/16917?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 [#16917](https://codecov.io/gh/apache/beam/pull/16917?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (f73b890) into [master](https://codecov.io/gh/apache/beam/commit/b2f21281b94dc7182d15611a3e6bf1d872ea3205?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (b2f2128) will **increase** coverage by `36.74%`.
   > The diff coverage is `n/a`.
   
   > :exclamation: Current head f73b890 differs from pull request most recent head 18a3db5. Consider uploading reports for the commit 18a3db5 to get more accurate results
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/beam/pull/16917/graphs/tree.svg?width=650&height=150&src=pr&token=qcbbAh8Fj1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)](https://codecov.io/gh/apache/beam/pull/16917?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   
   ```diff
   @@             Coverage Diff             @@
   ##           master   #16917       +/-   ##
   ===========================================
   + Coverage   46.89%   83.64%   +36.74%     
   ===========================================
     Files         204      453      +249     
     Lines       20122    62471    +42349     
   ===========================================
   + Hits         9436    52251    +42815     
   - Misses       9686    10220      +534     
   + Partials     1000        0     -1000     
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/beam/pull/16917?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
   |---|---|---|
   | [sdks/go/pkg/beam/core/runtime/exec/translate.go](https://codecov.io/gh/apache/beam/pull/16917/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-c2Rrcy9nby9wa2cvYmVhbS9jb3JlL3J1bnRpbWUvZXhlYy90cmFuc2xhdGUuZ28=) | | |
   | [sdks/go/pkg/beam/util/harnessopts/cache.go](https://codecov.io/gh/apache/beam/pull/16917/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-c2Rrcy9nby9wa2cvYmVhbS91dGlsL2hhcm5lc3NvcHRzL2NhY2hlLmdv) | | |
   | [sdks/go/pkg/beam/core/graph/graph.go](https://codecov.io/gh/apache/beam/pull/16917/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-c2Rrcy9nby9wa2cvYmVhbS9jb3JlL2dyYXBoL2dyYXBoLmdv) | | |
   | [sdks/go/pkg/beam/core/graph/coder/map.go](https://codecov.io/gh/apache/beam/pull/16917/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-c2Rrcy9nby9wa2cvYmVhbS9jb3JlL2dyYXBoL2NvZGVyL21hcC5nbw==) | | |
   | [sdks/go/pkg/beam/schema.go](https://codecov.io/gh/apache/beam/pull/16917/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-c2Rrcy9nby9wa2cvYmVhbS9zY2hlbWEuZ28=) | | |
   | [sdks/go/pkg/beam/core/graph/fn.go](https://codecov.io/gh/apache/beam/pull/16917/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-c2Rrcy9nby9wa2cvYmVhbS9jb3JlL2dyYXBoL2ZuLmdv) | | |
   | [sdks/go/pkg/beam/core/runtime/xlangx/payload.go](https://codecov.io/gh/apache/beam/pull/16917/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-c2Rrcy9nby9wa2cvYmVhbS9jb3JlL3J1bnRpbWUveGxhbmd4L3BheWxvYWQuZ28=) | | |
   | [sdks/go/pkg/beam/core/runtime/exec/fullvalue.go](https://codecov.io/gh/apache/beam/pull/16917/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-c2Rrcy9nby9wa2cvYmVhbS9jb3JlL3J1bnRpbWUvZXhlYy9mdWxsdmFsdWUuZ28=) | | |
   | [sdks/go/pkg/beam/io/databaseio/writer.go](https://codecov.io/gh/apache/beam/pull/16917/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-c2Rrcy9nby9wa2cvYmVhbS9pby9kYXRhYmFzZWlvL3dyaXRlci5nbw==) | | |
   | [sdks/go/pkg/beam/testing/passert/passert.go](https://codecov.io/gh/apache/beam/pull/16917/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-c2Rrcy9nby9wa2cvYmVhbS90ZXN0aW5nL3Bhc3NlcnQvcGFzc2VydC5nbw==) | | |
   | ... and [647 more](https://codecov.io/gh/apache/beam/pull/16917/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/16917?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/16917?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 [b2f2128...18a3db5](https://codecov.io/gh/apache/beam/pull/16917?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] codecov[bot] edited a comment on pull request #16917: [BEAM-13972] Add RunInference interface

Posted by GitBox <gi...@apache.org>.
codecov[bot] edited a comment on pull request #16917:
URL: https://github.com/apache/beam/pull/16917#issuecomment-1047862344


   # [Codecov](https://codecov.io/gh/apache/beam/pull/16917?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 [#16917](https://codecov.io/gh/apache/beam/pull/16917?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (4280dfc) into [master](https://codecov.io/gh/apache/beam/commit/b2f21281b94dc7182d15611a3e6bf1d872ea3205?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (b2f2128) will **increase** coverage by `36.74%`.
   > The diff coverage is `96.77%`.
   
   > :exclamation: Current head 4280dfc differs from pull request most recent head b0cea39. Consider uploading reports for the commit b0cea39 to get more accurate results
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/beam/pull/16917/graphs/tree.svg?width=650&height=150&src=pr&token=qcbbAh8Fj1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)](https://codecov.io/gh/apache/beam/pull/16917?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   
   ```diff
   @@             Coverage Diff             @@
   ##           master   #16917       +/-   ##
   ===========================================
   + Coverage   46.89%   83.63%   +36.74%     
   ===========================================
     Files         204      454      +250     
     Lines       20122    62591    +42469     
   ===========================================
   + Hits         9436    52348    +42912     
   - Misses       9686    10243      +557     
   + Partials     1000        0     -1000     
   ```
   
   | Flag | Coverage Δ | |
   |---|---|---|
   | python | `83.63% <96.77%> (?)` | |
   
   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/16917?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
   |---|---|---|
   | [sdks/python/apache\_beam/ml/inference/api.py](https://codecov.io/gh/apache/beam/pull/16917/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-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vbWwvaW5mZXJlbmNlL2FwaS5weQ==) | `96.77% <96.77%> (ø)` | |
   | [sdks/go/pkg/beam/testing/teststream/teststream.go](https://codecov.io/gh/apache/beam/pull/16917/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-c2Rrcy9nby9wa2cvYmVhbS90ZXN0aW5nL3Rlc3RzdHJlYW0vdGVzdHN0cmVhbS5nbw==) | | |
   | [sdks/go/pkg/beam/transforms/stats/max\_switch.go](https://codecov.io/gh/apache/beam/pull/16917/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-c2Rrcy9nby9wa2cvYmVhbS90cmFuc2Zvcm1zL3N0YXRzL21heF9zd2l0Y2guZ28=) | | |
   | [sdks/go/pkg/beam/transforms/filter/filter.go](https://codecov.io/gh/apache/beam/pull/16917/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-c2Rrcy9nby9wa2cvYmVhbS90cmFuc2Zvcm1zL2ZpbHRlci9maWx0ZXIuZ28=) | | |
   | [sdks/go/pkg/beam/core/graph/coder/time.go](https://codecov.io/gh/apache/beam/pull/16917/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-c2Rrcy9nby9wa2cvYmVhbS9jb3JlL2dyYXBoL2NvZGVyL3RpbWUuZ28=) | | |
   | [sdks/go/pkg/beam/testing/passert/equals.go](https://codecov.io/gh/apache/beam/pull/16917/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-c2Rrcy9nby9wa2cvYmVhbS90ZXN0aW5nL3Bhc3NlcnQvZXF1YWxzLmdv) | | |
   | [sdks/go/pkg/beam/core/runtime/graphx/dataflow.go](https://codecov.io/gh/apache/beam/pull/16917/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-c2Rrcy9nby9wa2cvYmVhbS9jb3JlL3J1bnRpbWUvZ3JhcGh4L2RhdGFmbG93Lmdv) | | |
   | [sdks/go/pkg/beam/core/runtime/exec/hash.go](https://codecov.io/gh/apache/beam/pull/16917/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-c2Rrcy9nby9wa2cvYmVhbS9jb3JlL3J1bnRpbWUvZXhlYy9oYXNoLmdv) | | |
   | [sdks/go/pkg/beam/core/funcx/fn.go](https://codecov.io/gh/apache/beam/pull/16917/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-c2Rrcy9nby9wa2cvYmVhbS9jb3JlL2Z1bmN4L2ZuLmdv) | | |
   | [...o/pkg/beam/io/rtrackers/offsetrange/offsetrange.go](https://codecov.io/gh/apache/beam/pull/16917/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-c2Rrcy9nby9wa2cvYmVhbS9pby9ydHJhY2tlcnMvb2Zmc2V0cmFuZ2Uvb2Zmc2V0cmFuZ2UuZ28=) | | |
   | ... and [648 more](https://codecov.io/gh/apache/beam/pull/16917/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/16917?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/16917?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 [b2f2128...b0cea39](https://codecov.io/gh/apache/beam/pull/16917?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 change in pull request #16917: [BEAM-13972] Add RunInference interface

Posted by GitBox <gi...@apache.org>.
yeandy commented on a change in pull request #16917:
URL: https://github.com/apache/beam/pull/16917#discussion_r819105567



##########
File path: sdks/python/apache_beam/ml/inference/api.py
##########
@@ -0,0 +1,84 @@
+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements.  See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License.  You may obtain a copy of the License at
+#
+#    http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+
+from dataclasses import dataclass
+import apache_beam as beam
+from typing import Tuple, TypeVar, Union
+# TODO: implement RunInferenceImpl
+# from apache_beam.ml.inference.base import RunInferenceImpl
+
+
+@dataclass
+class BaseModelSpec:
+  model_url: str
+
+
+@dataclass
+class PyTorchModelSpec(BaseModelSpec):

Review comment:
       I know it will be obvious what it can do if you just read the code, but to me, `PyTorchModel` implies that it is the actual model that can do the prediction, instead of an object that holds specs about the 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] codecov[bot] edited a comment on pull request #16917: [BEAM-13972] Add RunInference interface

Posted by GitBox <gi...@apache.org>.
codecov[bot] edited a comment on pull request #16917:
URL: https://github.com/apache/beam/pull/16917#issuecomment-1047862344


   # [Codecov](https://codecov.io/gh/apache/beam/pull/16917?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 [#16917](https://codecov.io/gh/apache/beam/pull/16917?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (e5c18d9) into [master](https://codecov.io/gh/apache/beam/commit/b2f21281b94dc7182d15611a3e6bf1d872ea3205?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (b2f2128) will **increase** coverage by `36.73%`.
   > The diff coverage is `96.96%`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/beam/pull/16917/graphs/tree.svg?width=650&height=150&src=pr&token=qcbbAh8Fj1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)](https://codecov.io/gh/apache/beam/pull/16917?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   
   ```diff
   @@             Coverage Diff             @@
   ##           master   #16917       +/-   ##
   ===========================================
   + Coverage   46.89%   83.63%   +36.73%     
   ===========================================
     Files         204      454      +250     
     Lines       20122    62595    +42473     
   ===========================================
   + Hits         9436    52349    +42913     
   - Misses       9686    10246      +560     
   + Partials     1000        0     -1000     
   ```
   
   | Flag | Coverage Δ | |
   |---|---|---|
   | python | `83.63% <96.96%> (?)` | |
   
   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/16917?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
   |---|---|---|
   | [sdks/python/apache\_beam/ml/inference/api.py](https://codecov.io/gh/apache/beam/pull/16917/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-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vbWwvaW5mZXJlbmNlL2FwaS5weQ==) | `96.96% <96.96%> (ø)` | |
   | [sdks/go/pkg/beam/transforms/stats/min.go](https://codecov.io/gh/apache/beam/pull/16917/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-c2Rrcy9nby9wa2cvYmVhbS90cmFuc2Zvcm1zL3N0YXRzL21pbi5nbw==) | | |
   | [sdks/go/pkg/beam/core/graph/coder/windows.go](https://codecov.io/gh/apache/beam/pull/16917/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-c2Rrcy9nby9wa2cvYmVhbS9jb3JlL2dyYXBoL2NvZGVyL3dpbmRvd3MuZ28=) | | |
   | [sdks/go/pkg/beam/combine.go](https://codecov.io/gh/apache/beam/pull/16917/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-c2Rrcy9nby9wa2cvYmVhbS9jb21iaW5lLmdv) | | |
   | [sdks/go/pkg/beam/core/util/protox/any.go](https://codecov.io/gh/apache/beam/pull/16917/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-c2Rrcy9nby9wa2cvYmVhbS9jb3JlL3V0aWwvcHJvdG94L2FueS5nbw==) | | |
   | [sdks/go/pkg/beam/transforms/stats/sum\_switch.go](https://codecov.io/gh/apache/beam/pull/16917/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-c2Rrcy9nby9wa2cvYmVhbS90cmFuc2Zvcm1zL3N0YXRzL3N1bV9zd2l0Y2guZ28=) | | |
   | [sdks/go/pkg/beam/beam.shims.go](https://codecov.io/gh/apache/beam/pull/16917/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-c2Rrcy9nby9wa2cvYmVhbS9iZWFtLnNoaW1zLmdv) | | |
   | [sdks/go/pkg/beam/io/xlang/schemaio/schemaio.go](https://codecov.io/gh/apache/beam/pull/16917/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-c2Rrcy9nby9wa2cvYmVhbS9pby94bGFuZy9zY2hlbWFpby9zY2hlbWFpby5nbw==) | | |
   | [sdks/go/pkg/beam/core/graph/bind.go](https://codecov.io/gh/apache/beam/pull/16917/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-c2Rrcy9nby9wa2cvYmVhbS9jb3JlL2dyYXBoL2JpbmQuZ28=) | | |
   | [sdks/go/pkg/beam/artifact/materialize.go](https://codecov.io/gh/apache/beam/pull/16917/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-c2Rrcy9nby9wa2cvYmVhbS9hcnRpZmFjdC9tYXRlcmlhbGl6ZS5nbw==) | | |
   | ... and [648 more](https://codecov.io/gh/apache/beam/pull/16917/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/16917?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/16917?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 [b2f2128...e5c18d9](https://codecov.io/gh/apache/beam/pull/16917?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 change in pull request #16917: [BEAM-13972] Add RunInference interface

Posted by GitBox <gi...@apache.org>.
yeandy commented on a change in pull request #16917:
URL: https://github.com/apache/beam/pull/16917#discussion_r821025267



##########
File path: sdks/python/apache_beam/ml/inference/model.py
##########
@@ -0,0 +1,74 @@
+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements.  See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License.  You may obtain a copy of the License at
+#
+#    http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+
+import abc
+
+VALID_PYTORCH_DEVICE_TYPES = ['CPU', 'GPU']
+VALID_SKLEARN_SERIALIZATION_TYPES = ['JOBLIB', 'PICKLE']
+
+
+class RunInferenceModel(object):
+  '''
+  Base class for the RunInference Model.
+  This class contains information about
+  a model that is needed
+  '''
+  def __init__(self, model_url):
+    self._model_url = model_url
+    self._validate_model()
+
+  @abc.abstractmethod
+  def _validate_model(self):
+    raise NotImplementedError("Please implement _validate_model")
+
+
+class PytorchModel(RunInferenceModel):

Review comment:
       These implementations will be done in separate PRs ([pytorch](https://issues.apache.org/jira/browse/BEAM-13984), [sklearn](https://issues.apache.org/jira/browse/BEAM-13983)) after this PR, as well as the PR for the base class #16970 are 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] yeandy commented on a change in pull request #16917: [BEAM-13972] Add RunInference interface

Posted by GitBox <gi...@apache.org>.
yeandy commented on a change in pull request #16917:
URL: https://github.com/apache/beam/pull/16917#discussion_r821026488



##########
File path: sdks/python/apache_beam/ml/inference/api.py
##########
@@ -0,0 +1,81 @@
+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements.  See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License.  You may obtain a copy of the License at
+#
+#    http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+
+from dataclasses import dataclass
+from enum import Enum
+from typing import Tuple, TypeVar, Union
+
+import apache_beam as beam
+
+
+class PyTorchDevice(Enum):
+  CPU = 1
+  GPU = 2
+
+
+class SklearnSerializationType(Enum):
+  PICKLE = 1
+  JOBLIB = 2
+
+
+@dataclass
+class BaseModelSpec:
+  model_uri: str

Review comment:
       I'm not following-- could you clarify? Also, we've scheduled a meeting to sync on this, so perhaps it we can get alignment then.




-- 
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 change in pull request #16917: [BEAM-13972] Add RunInference interface

Posted by GitBox <gi...@apache.org>.
yeandy commented on a change in pull request #16917:
URL: https://github.com/apache/beam/pull/16917#discussion_r822838002



##########
File path: sdks/python/apache_beam/ml/inference/api.py
##########
@@ -0,0 +1,81 @@
+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements.  See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License.  You may obtain a copy of the License at
+#
+#    http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+
+from dataclasses import dataclass
+from enum import Enum
+from typing import Tuple, TypeVar, Union
+
+import apache_beam as beam
+
+
+class PyTorchDevice(Enum):
+  CPU = 1
+  GPU = 2
+
+
+class SklearnSerializationType(Enum):
+  PICKLE = 1
+  JOBLIB = 2
+
+
+@dataclass
+class BaseModelSpec:
+  model_uri: str

Review comment:
       Resolving per our discussion: Only subclasses only need to look at `model_uri`, so we should put it there.




-- 
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 change in pull request #16917: [BEAM-13972] Add RunInference interface

Posted by GitBox <gi...@apache.org>.
yeandy commented on a change in pull request #16917:
URL: https://github.com/apache/beam/pull/16917#discussion_r812443799



##########
File path: sdks/python/apache_beam/ml/inference/run_inference.py
##########
@@ -0,0 +1,43 @@
+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements.  See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License.  You may obtain a copy of the License at
+#
+#    http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+
+import apache_beam as beam
+from apache_beam.ml.inference.model import (
+    PytorchModel, RunInferenceModel, SklearnModel)

Review comment:
       I have a habit of this due to experiences in other, older Python repos 😆  




-- 
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] robertwb commented on a change in pull request #16917: [BEAM-13972] Add RunInference interface

Posted by GitBox <gi...@apache.org>.
robertwb commented on a change in pull request #16917:
URL: https://github.com/apache/beam/pull/16917#discussion_r813233151



##########
File path: sdks/python/apache_beam/ml/inference/model.py
##########
@@ -0,0 +1,74 @@
+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements.  See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License.  You may obtain a copy of the License at
+#
+#    http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+
+import abc
+
+VALID_PYTORCH_DEVICE_TYPES = ['CPU', 'GPU']
+VALID_SKLEARN_SERIALIZATION_TYPES = ['JOBLIB', 'PICKLE']
+
+
+class RunInferenceModel(object):
+  '''
+  Base class for the RunInference Model.
+  This class contains information about
+  a model that is needed
+  '''
+  def __init__(self, model_url):
+    self._model_url = model_url
+    self._validate_model()
+
+  @abc.abstractmethod
+  def _validate_model(self):
+    raise NotImplementedError("Please implement _validate_model")
+
+
+class PytorchModel(RunInferenceModel):
+  '''
+  This class wraps the PyTorch model, and other
+  PyTorch-specific parameters
+  '''
+  def __init__(self, model_url: str, device: str):
+    super().__init__(model_url)
+    self._device = device
+    self._validate_device()
+
+  def _validate_model(self):
+    pass
+
+  def _validate_device(self):
+    if self._device not in VALID_PYTORCH_DEVICE_TYPES:
+      raise ValueError(
+          'Device type must be one of ' + VALID_PYTORCH_DEVICE_TYPES)
+
+
+class SklearnModel(RunInferenceModel):
+  '''
+  This class wraps the scikit-learn model, and other
+  scikit-learn-specific parameters
+  '''
+  def __init__(self, model_url, serialization_method):
+    super().__init__(model_url)
+    self._serialization_method = serialization_method
+
+  def _validate_model(self):
+    pass
+
+  def _validate_serialization(self):

Review comment:
       Did you intend to call this?

##########
File path: sdks/python/apache_beam/ml/inference/model.py
##########
@@ -0,0 +1,74 @@
+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements.  See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License.  You may obtain a copy of the License at
+#
+#    http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+
+import abc
+
+VALID_PYTORCH_DEVICE_TYPES = ['CPU', 'GPU']
+VALID_SKLEARN_SERIALIZATION_TYPES = ['JOBLIB', 'PICKLE']
+
+
+class RunInferenceModel(object):
+  '''
+  Base class for the RunInference Model.
+  This class contains information about
+  a model that is needed
+  '''
+  def __init__(self, model_url):
+    self._model_url = model_url
+    self._validate_model()
+
+  @abc.abstractmethod
+  def _validate_model(self):
+    raise NotImplementedError("Please implement _validate_model")
+
+
+class PytorchModel(RunInferenceModel):
+  '''
+  This class wraps the PyTorch model, and other
+  PyTorch-specific parameters
+  '''
+  def __init__(self, model_url: str, device: str):
+    super().__init__(model_url)
+    self._device = device
+    self._validate_device()
+
+  def _validate_model(self):
+    pass
+
+  def _validate_device(self):
+    if self._device not in VALID_PYTORCH_DEVICE_TYPES:
+      raise ValueError(
+          'Device type must be one of ' + VALID_PYTORCH_DEVICE_TYPES)
+
+
+class SklearnModel(RunInferenceModel):
+  '''
+  This class wraps the scikit-learn model, and other
+  scikit-learn-specific parameters
+  '''
+  def __init__(self, model_url, serialization_method):

Review comment:
       Does it make sense to have a default serialization method? (I'm not sure what that means, docs would be good.)

##########
File path: sdks/python/apache_beam/ml/inference/run_inference.py
##########
@@ -0,0 +1,43 @@
+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements.  See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License.  You may obtain a copy of the License at
+#
+#    http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+
+import apache_beam as beam
+from apache_beam.ml.inference.model import (
+    PytorchModel, RunInferenceModel, SklearnModel)

Review comment:
       Ideally we should not have to import specific implementations here, just RunInferenceModel.

##########
File path: sdks/python/apache_beam/ml/inference/model.py
##########
@@ -0,0 +1,74 @@
+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements.  See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License.  You may obtain a copy of the License at
+#
+#    http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+
+import abc
+
+VALID_PYTORCH_DEVICE_TYPES = ['CPU', 'GPU']

Review comment:
       Should these be class-level variables on the pytorch/sklearn implementations?

##########
File path: sdks/python/apache_beam/ml/inference/model.py
##########
@@ -0,0 +1,74 @@
+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements.  See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License.  You may obtain a copy of the License at
+#
+#    http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+
+import abc
+
+VALID_PYTORCH_DEVICE_TYPES = ['CPU', 'GPU']
+VALID_SKLEARN_SERIALIZATION_TYPES = ['JOBLIB', 'PICKLE']
+
+
+class RunInferenceModel(object):
+  '''
+  Base class for the RunInference Model.
+  This class contains information about
+  a model that is needed
+  '''
+  def __init__(self, model_url):
+    self._model_url = model_url
+    self._validate_model()
+
+  @abc.abstractmethod
+  def _validate_model(self):
+    raise NotImplementedError("Please implement _validate_model")
+
+
+class PytorchModel(RunInferenceModel):

Review comment:
       It feels like PytorchModel and SklearnModel should be in their own file, only imported if one or the other is needed, rather than having a single module that holds all model types. 

##########
File path: sdks/python/apache_beam/ml/inference/run_inference.py
##########
@@ -0,0 +1,43 @@
+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements.  See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License.  You may obtain a copy of the License at
+#
+#    http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+
+import apache_beam as beam
+from apache_beam.ml.inference.model import (
+    PytorchModel, RunInferenceModel, SklearnModel)
+
+
+# TODO
+#@beam.typehints.with_input_types(Union[_INPUT_TYPE, Tuple[_K, _INPUT_TYPE]])
+#@beam.typehints.with_output_types(Union[_OUTPUT_TYPE, Tuple[_K, _OUTPUT_TYPE]])
+class RunInference(beam.PTransform):
+  def __init__(self, model: RunInferenceModel, batch_size=None, **kwargs):
+    self._model = model
+    self._batch_size = batch_size
+
+  def expand(self, examples: beam.PCollection) -> beam.PCollection:
+
+    if isinstance(self._model, PytorchModel):

Review comment:
       Generally isinstance chains like this is an anti-pattern. RunInferenceModel should have a sufficiently expressive interface that the implementation-specific can be fully encapsulated in the various subclass implementations. 




-- 
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] aaltay commented on pull request #16917: [BEAM-13972] Add RunInference interface

Posted by GitBox <gi...@apache.org>.
aaltay commented on pull request #16917:
URL: https://github.com/apache/beam/pull/16917#issuecomment-1071912825


   @robertwb - could you please review this PR?
   
   Other folks on the PR, feel free to ping reviewers here or in other places if a review is open for more than a few business days. And where it makes sense you could also ask other committers to review PRs and spread the review load across committers.


-- 
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 change in pull request #16917: [BEAM-13972] Add RunInference interface

Posted by GitBox <gi...@apache.org>.
yeandy commented on a change in pull request #16917:
URL: https://github.com/apache/beam/pull/16917#discussion_r814242249



##########
File path: sdks/python/apache_beam/ml/inference/run_inference.py
##########
@@ -0,0 +1,43 @@
+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements.  See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License.  You may obtain a copy of the License at
+#
+#    http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+
+import apache_beam as beam
+from apache_beam.ml.inference.model import (
+    PytorchModel, RunInferenceModel, SklearnModel)
+
+
+# TODO
+#@beam.typehints.with_input_types(Union[_INPUT_TYPE, Tuple[_K, _INPUT_TYPE]])
+#@beam.typehints.with_output_types(Union[_OUTPUT_TYPE, Tuple[_K, _OUTPUT_TYPE]])
+class RunInference(beam.PTransform):
+  def __init__(self, model: RunInferenceModel, batch_size=None, **kwargs):
+    self._model = model
+    self._batch_size = batch_size
+
+  def expand(self, examples: beam.PCollection) -> beam.PCollection:
+
+    if isinstance(self._model, PytorchModel):

Review comment:
       👍 




-- 
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 #16917: [BEAM-13972] Add RunInference interface

Posted by GitBox <gi...@apache.org>.
yeandy commented on pull request #16917:
URL: https://github.com/apache/beam/pull/16917#issuecomment-1057292810


   CC: @KevinGG 


-- 
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 edited a comment on pull request #16917: [BEAM-13972] Add RunInference interface

Posted by GitBox <gi...@apache.org>.
yeandy edited a comment on pull request #16917:
URL: https://github.com/apache/beam/pull/16917#issuecomment-1057209248


   CC: @TheNeuralBit and @tvalentyn 


-- 
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] edited a comment on pull request #16917: [BEAM-13972] Add RunInference interface

Posted by GitBox <gi...@apache.org>.
codecov[bot] edited a comment on pull request #16917:
URL: https://github.com/apache/beam/pull/16917#issuecomment-1047862344


   # [Codecov](https://codecov.io/gh/apache/beam/pull/16917?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 [#16917](https://codecov.io/gh/apache/beam/pull/16917?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (cf31b18) into [master](https://codecov.io/gh/apache/beam/commit/b2f21281b94dc7182d15611a3e6bf1d872ea3205?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (b2f2128) will **increase** coverage by `36.75%`.
   > The diff coverage is `96.77%`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/beam/pull/16917/graphs/tree.svg?width=650&height=150&src=pr&token=qcbbAh8Fj1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)](https://codecov.io/gh/apache/beam/pull/16917?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   
   ```diff
   @@             Coverage Diff             @@
   ##           master   #16917       +/-   ##
   ===========================================
   + Coverage   46.89%   83.64%   +36.75%     
   ===========================================
     Files         204      454      +250     
     Lines       20122    62591    +42469     
   ===========================================
   + Hits         9436    52354    +42918     
   - Misses       9686    10237      +551     
   + Partials     1000        0     -1000     
   ```
   
   | Flag | Coverage Δ | |
   |---|---|---|
   | python | `83.64% <96.77%> (?)` | |
   
   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/16917?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
   |---|---|---|
   | [sdks/python/apache\_beam/ml/inference/api.py](https://codecov.io/gh/apache/beam/pull/16917/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-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vbWwvaW5mZXJlbmNlL2FwaS5weQ==) | `96.77% <96.77%> (ø)` | |
   | [sdks/go/pkg/beam/transforms/stats/min\_switch.go](https://codecov.io/gh/apache/beam/pull/16917/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-c2Rrcy9nby9wa2cvYmVhbS90cmFuc2Zvcm1zL3N0YXRzL21pbl9zd2l0Y2guZ28=) | | |
   | [...ks/go/pkg/beam/core/runtime/harness/cache\_hooks.go](https://codecov.io/gh/apache/beam/pull/16917/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-c2Rrcy9nby9wa2cvYmVhbS9jb3JlL3J1bnRpbWUvaGFybmVzcy9jYWNoZV9ob29rcy5nbw==) | | |
   | [sdks/go/pkg/beam/io/filesystem/util.go](https://codecov.io/gh/apache/beam/pull/16917/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-c2Rrcy9nby9wa2cvYmVhbS9pby9maWxlc3lzdGVtL3V0aWwuZ28=) | | |
   | [sdks/go/pkg/beam/core/util/reflectx/functions.go](https://codecov.io/gh/apache/beam/pull/16917/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-c2Rrcy9nby9wa2cvYmVhbS9jb3JlL3V0aWwvcmVmbGVjdHgvZnVuY3Rpb25zLmdv) | | |
   | [sdks/go/pkg/beam/core/util/reflectx/structs.go](https://codecov.io/gh/apache/beam/pull/16917/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-c2Rrcy9nby9wa2cvYmVhbS9jb3JlL3V0aWwvcmVmbGVjdHgvc3RydWN0cy5nbw==) | | |
   | [sdks/go/pkg/beam/core/graph/fn.go](https://codecov.io/gh/apache/beam/pull/16917/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-c2Rrcy9nby9wa2cvYmVhbS9jb3JlL2dyYXBoL2ZuLmdv) | | |
   | [sdks/go/pkg/beam/core/runtime/genx/genx.go](https://codecov.io/gh/apache/beam/pull/16917/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-c2Rrcy9nby9wa2cvYmVhbS9jb3JlL3J1bnRpbWUvZ2VueC9nZW54Lmdv) | | |
   | [sdks/go/pkg/beam/runners/direct/gbk.go](https://codecov.io/gh/apache/beam/pull/16917/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-c2Rrcy9nby9wa2cvYmVhbS9ydW5uZXJzL2RpcmVjdC9nYmsuZ28=) | | |
   | [sdks/go/pkg/beam/core/graph/node.go](https://codecov.io/gh/apache/beam/pull/16917/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-c2Rrcy9nby9wa2cvYmVhbS9jb3JlL2dyYXBoL25vZGUuZ28=) | | |
   | ... and [648 more](https://codecov.io/gh/apache/beam/pull/16917/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/16917?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/16917?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 [b2f2128...cf31b18](https://codecov.io/gh/apache/beam/pull/16917?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] ryanthompson591 commented on a change in pull request #16917: [BEAM-13972] Add RunInference interface

Posted by GitBox <gi...@apache.org>.
ryanthompson591 commented on a change in pull request #16917:
URL: https://github.com/apache/beam/pull/16917#discussion_r822868000



##########
File path: sdks/python/apache_beam/ml/inference/api.py
##########
@@ -15,38 +15,29 @@
 # limitations under the License.
 #
 
+import abc
 from dataclasses import dataclass
-from enum import Enum
 from typing import Tuple
 from typing import TypeVar
 from typing import Union
 
 import apache_beam as beam
 
 
-class PyTorchDevice(Enum):
-  CPU = 1
-  GPU = 2
-
-
-class SklearnSerializationType(Enum):
-  PICKLE = 1
-  JOBLIB = 2
-
-
-@dataclass
 class BaseModelSpec:
-  model_uri: str
-
-
-@dataclass
-class PyTorchModelSpec(BaseModelSpec):
-  device: PyTorchDevice
-
+  """
+  Model factory that returns ModelLoader and
+  InferenceRunner objects to be used
+  """
+  @abc.abstractmethod
+  def get_model_loader(self):
+    "Returns ModelLoader object"

Review comment:
       I think this is good for now.  
   
   Just a note, but I don't think you need to do anything: Going forward it seems to me that there will be an implementation that uses a service (instead of loading model) a that maybe returns none for get_model_loader.  I haven't decided if this should return None by default or if a NotImplementedError is fine.




-- 
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 change in pull request #16917: [BEAM-13972] Add RunInference interface

Posted by GitBox <gi...@apache.org>.
yeandy commented on a change in pull request #16917:
URL: https://github.com/apache/beam/pull/16917#discussion_r812445562



##########
File path: sdks/python/apache_beam/ml/inference/model.py
##########
@@ -0,0 +1,74 @@
+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements.  See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License.  You may obtain a copy of the License at
+#
+#    http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+
+import abc
+
+VALID_PYTORCH_DEVICE_TYPES = ['CPU', 'GPU']
+VALID_SKLEARN_SERIALIZATION_TYPES = ['JOBLIB', 'PICKLE']
+
+
+class RunInferenceModel(object):
+  '''
+  Base class for the RunInference Model.
+  This class contains information about
+  a model that is needed
+  '''
+  def __init__(self, model_url):
+    self._model_url = model_url
+    self._validate_model()
+
+  @abc.abstractmethod
+  def _validate_model(self):
+    raise NotImplementedError("Please implement _validate_model")
+
+
+class PytorchModel(RunInferenceModel):
+  '''
+  This class wraps the PyTorch model, and other
+  PyTorch-specific parameters
+  '''
+  def __init__(self, model_url: str, device: str):
+    super().__init__(model_url)
+    self._device = device
+    self._validate_device()
+
+  def _validate_model(self):
+    pass
+
+  def _validate_device(self):
+    if self._device not in VALID_PYTORCH_DEVICE_TYPES:

Review comment:
       True, 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] codecov[bot] edited a comment on pull request #16917: [BEAM-13972] Add RunInference interface

Posted by GitBox <gi...@apache.org>.
codecov[bot] edited a comment on pull request #16917:
URL: https://github.com/apache/beam/pull/16917#issuecomment-1047862344


   # [Codecov](https://codecov.io/gh/apache/beam/pull/16917?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 [#16917](https://codecov.io/gh/apache/beam/pull/16917?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (f73b890) into [master](https://codecov.io/gh/apache/beam/commit/b2f21281b94dc7182d15611a3e6bf1d872ea3205?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (b2f2128) will **increase** coverage by `36.74%`.
   > The diff coverage is `n/a`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/beam/pull/16917/graphs/tree.svg?width=650&height=150&src=pr&token=qcbbAh8Fj1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)](https://codecov.io/gh/apache/beam/pull/16917?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   
   ```diff
   @@             Coverage Diff             @@
   ##           master   #16917       +/-   ##
   ===========================================
   + Coverage   46.89%   83.64%   +36.74%     
   ===========================================
     Files         204      453      +249     
     Lines       20122    62471    +42349     
   ===========================================
   + Hits         9436    52251    +42815     
   - Misses       9686    10220      +534     
   + Partials     1000        0     -1000     
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/beam/pull/16917?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
   |---|---|---|
   | [sdks/go/pkg/beam/core/runtime/exec/sdf.go](https://codecov.io/gh/apache/beam/pull/16917/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-c2Rrcy9nby9wa2cvYmVhbS9jb3JlL3J1bnRpbWUvZXhlYy9zZGYuZ28=) | | |
   | [sdks/go/pkg/beam/testing/ptest/ptest.go](https://codecov.io/gh/apache/beam/pull/16917/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-c2Rrcy9nby9wa2cvYmVhbS90ZXN0aW5nL3B0ZXN0L3B0ZXN0Lmdv) | | |
   | [sdks/go/pkg/beam/core/graph/coder/row\_decoder.go](https://codecov.io/gh/apache/beam/pull/16917/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-c2Rrcy9nby9wa2cvYmVhbS9jb3JlL2dyYXBoL2NvZGVyL3Jvd19kZWNvZGVyLmdv) | | |
   | [sdks/go/pkg/beam/provision/provision.go](https://codecov.io/gh/apache/beam/pull/16917/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-c2Rrcy9nby9wa2cvYmVhbS9wcm92aXNpb24vcHJvdmlzaW9uLmdv) | | |
   | [sdks/go/pkg/beam/options/gcpopts/options.go](https://codecov.io/gh/apache/beam/pull/16917/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-c2Rrcy9nby9wa2cvYmVhbS9vcHRpb25zL2djcG9wdHMvb3B0aW9ucy5nbw==) | | |
   | [...ks/go/pkg/beam/runners/dataflow/dataflowlib/job.go](https://codecov.io/gh/apache/beam/pull/16917/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-c2Rrcy9nby9wa2cvYmVhbS9ydW5uZXJzL2RhdGFmbG93L2RhdGFmbG93bGliL2pvYi5nbw==) | | |
   | [sdks/go/pkg/beam/io/filesystem/local/local.go](https://codecov.io/gh/apache/beam/pull/16917/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-c2Rrcy9nby9wa2cvYmVhbS9pby9maWxlc3lzdGVtL2xvY2FsL2xvY2FsLmdv) | | |
   | [...ks/go/pkg/beam/core/runtime/coderx/coderx.shims.go](https://codecov.io/gh/apache/beam/pull/16917/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-c2Rrcy9nby9wa2cvYmVhbS9jb3JlL3J1bnRpbWUvY29kZXJ4L2NvZGVyeC5zaGltcy5nbw==) | | |
   | [sdks/go/pkg/beam/core/runtime/exec/fn.go](https://codecov.io/gh/apache/beam/pull/16917/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-c2Rrcy9nby9wa2cvYmVhbS9jb3JlL3J1bnRpbWUvZXhlYy9mbi5nbw==) | | |
   | [sdks/go/pkg/beam/core/runtime/coderx/string.go](https://codecov.io/gh/apache/beam/pull/16917/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-c2Rrcy9nby9wa2cvYmVhbS9jb3JlL3J1bnRpbWUvY29kZXJ4L3N0cmluZy5nbw==) | | |
   | ... and [647 more](https://codecov.io/gh/apache/beam/pull/16917/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/16917?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/16917?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 [b2f2128...f73b890](https://codecov.io/gh/apache/beam/pull/16917?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 change in pull request #16917: [BEAM-13972] Add RunInference interface

Posted by GitBox <gi...@apache.org>.
yeandy commented on a change in pull request #16917:
URL: https://github.com/apache/beam/pull/16917#discussion_r817204464



##########
File path: sdks/python/apache_beam/ml/inference/api.py
##########
@@ -0,0 +1,84 @@
+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements.  See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License.  You may obtain a copy of the License at
+#
+#    http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+
+from dataclasses import dataclass
+import apache_beam as beam
+from typing import Tuple, TypeVar, Union
+# TODO: implement RunInferenceImpl
+# from apache_beam.ml.inference.base import RunInferenceImpl
+
+
+@dataclass
+class BaseModelSpec:
+  model_url: str
+
+
+@dataclass
+class PyTorchModelSpec(BaseModelSpec):

Review comment:
       Renamed `PytorchModel` to `PytorchModelSpec` since we renamed the old `ModelSpec` to `ModelLoader`. What do you think @ryanthompson591 ?




-- 
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