Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update RunInference documentation #22250

Merged
merged 52 commits into from
Jul 15, 2022
Merged

Conversation

rszper
Copy link
Contributor

@rszper rszper commented Jul 12, 2022

Adding documentation for the RunInference API.

  • Added a ML page in the Python section, linked to from the Python SDK page.
  • Added a RunInference transform page, linked to from the Python Transforms page.
  • Added snippets and output for the RunInference transforms page examples.

Thank you for your contribution! Follow this checklist to help us incorporate your contribution quickly and easily:

  • Choose reviewer(s) and mention them in a comment (R: @username).
  • Mention the appropriate issue in your description (for example: addresses #123), if applicable. This will automatically add a link to the pull request in the issue. If you would like the issue to automatically close on merging the pull request, comment fixes #<ISSUE NUMBER> instead.
  • Update CHANGES.md with noteworthy changes.
  • If this contribution is large, please file an Apache Individual Contributor License Agreement.

See the Contributor Guide for more tips on how to make review process smoother.

To check the build health, please visit https://github.com/apache/beam/blob/master/.test-infra/BUILD_STATUS.md

GitHub Actions Tests Status (on master branch)

Build python source distribution and wheels
Python tests
Java tests

See CI.md for more information about GitHub Actions CI.

@rszper
Copy link
Contributor Author

rszper commented Jul 12, 2022

R: @yeandy @rezarokni

@yeandy
Copy link
Contributor

yeandy commented Jul 12, 2022

R: @AnandInguva @ryanthompson591

@github-actions
Copy link
Contributor

Stopping reviewer notifications for this pull request: review requested by someone other than the bot, ceding control

@@ -48,6 +48,11 @@ language-specific implementation guidance.

## Using Beam Python SDK in your ML pipelines

To use the Beam Python SDK with your machine learning pipelines, you can either use the RunInference API or TensorFlow.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

change: use the RunInference API or TensorFlow
to: use the RunInference API for PyTorch and Sklearn models. If using Tensorflow model you can make use of the library from tfx_bsl. Further integrations for tensorflow are planned.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated, but I'm not sure if we'll need to update this again based on the email thread?

@AnandInguva
Copy link
Contributor

Added the RunInference examples -> #22254

Copy link
Contributor

@yeandy yeandy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good work so far! Left some feedback on changes.

Let me know when to take another pass, especially with the example snippets updated.

@rszper
Copy link
Contributor Author

rszper commented Jul 13, 2022

@yeandy I believe all updates are made based on your comments, except I haven't updated the examples yet.

Copy link
Contributor

@yeandy yeandy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the updates! A few more :)


To import models, you need to wrap them around a `ModelHandler object`. Add one or more of the following lines of code, depending on the framework and type of data structure that holds the data:

```
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These little chunks of code below here seem out of place.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@yeandy How do you want to handle this?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would it make sense to reword it to something like this, and keep the (refactored) code block?

To import models, you need to wrap them around a ModelHandler object. The ModelHandler you import will depend on the framework and type of data structure that contains the inputs. See the following examples on which ones you may want to import.

from apache_beam.ml.inference.sklearn_inference import SklearnModelHandlerNumpy
from apache_beam.ml.inference.sklearn_inference import SklearnModelHandlerPandas
from apache_beam.ml.inference.pytorch_inference import PytorchModelHandlerTensor
from apache_beam.ml.inference.pytorch_inference import PytorchModelHandlerKeyedTensor

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I made some updates. Take a look and let me know if we need more changes.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks. By the way, the imports I originally wrote had some typos, so I fixed them

from apache_beam.ml.inference.sklearn_inference import SklearnModelHandlerNumpy
from apache_beam.ml.inference.sklearn_inference import SklearnModelHandlerPandas
from apache_beam.ml.inference.pytorch_inference import PytorchModelHandlerTensor
from apache_beam.ml.inference.pytorch_inference import PytorchModelHandlerKeyedTensor

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated to fix the typos

Copy link
Contributor

@yeandy yeandy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@rszper A last minute addition on the batching issue.

@codecov
Copy link

codecov bot commented Jul 15, 2022

Codecov Report

Merging #22250 (4160d1b) into master (9cf8cf5) will increase coverage by 9.28%.
The diff coverage is 0.00%.

❗ Current head 4160d1b differs from pull request most recent head 4e89126. Consider uploading reports for the commit 4e89126 to get more accurate results

@@            Coverage Diff             @@
##           master   #22250      +/-   ##
==========================================
+ Coverage   74.25%   83.54%   +9.28%     
==========================================
  Files         702      474     -228     
  Lines       92999    65934   -27065     
==========================================
- Hits        69058    55085   -13973     
+ Misses      22674    10849   -11825     
+ Partials     1267        0    -1267     
Flag Coverage Δ
go ?
python 83.54% <0.00%> (-0.10%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
...es/snippets/transforms/elementwise/runinference.py 0.00% <0.00%> (ø)
sdks/python/apache_beam/io/source_test_utils.py 88.01% <0.00%> (-1.39%) ⬇️
...hon/apache_beam/runners/direct/test_stream_impl.py 93.28% <0.00%> (-0.75%) ⬇️
...eam/runners/portability/fn_api_runner/execution.py 92.44% <0.00%> (-0.65%) ⬇️
...ks/python/apache_beam/runners/worker/sdk_worker.py 89.09% <0.00%> (-0.32%) ⬇️
sdks/python/apache_beam/utils/annotations.py 100.00% <0.00%> (ø)
...hon/apache_beam/runners/worker/bundle_processor.py 93.54% <0.00%> (ø)
sdks/go/pkg/beam/core/graph/coder/int.go
sdks/go/pkg/beam/x/debug/debug.shims.go
sdks/go/pkg/beam/core/graph/xlang.go
... and 227 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 9cf8cf5...4e89126. Read the comment docs.

@AnandInguva
Copy link
Contributor

AnandInguva commented Jul 15, 2022

R: @tvalentyn @pabloem we did the review on the docs and the snippets. Would you be able to do a final review and merge the PR?

@yeandy
Copy link
Contributor

yeandy commented Jul 15, 2022

@tvalentyn @pabloem This one too please #22069. The tests are queued, but it should be fine since it's only the .md file that was modified.


### Shared helper class

Instead of loading a model for each thread in the process, we use the `Shared` class, which allows us to load one model that is shared across all threads of each worker in a DoFn. For more information, see the
Copy link
Contributor

@tvalentyn tvalentyn Jul 15, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How about:

Using the Shared class within RunInference implementation allows us to load the model only once per process and share it with all DoFn instances created in that process. This reduces the memory consumption and model loading time.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated

```
Where `model_handler` is the model handler setup code.

To import models, you need to wrap them around a `ModelHandler` object. Which `ModelHandler` you import depends on the framework and type of data structure that contains the inputs. The following examples show some ModelHandlers that you might want to import.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To import models, you need to wrap them around a ModelHandler object

Consider instead:
To import models, you need to configure a ModelHandler object that will wrap the underlying model

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated


Disable batching by overriding the `batch_elements_kwargs` function in your ModelHandler and setting the maximum batch size (`max_batch_size`) to one: `max_batch_size=1`. For more information, see
[BatchElements PTransforms](/documentation/sdks/python-machine-learning/#batchelements-ptransform).

Copy link
Contributor

@tvalentyn tvalentyn Jul 15, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How about we also link apache_beam/examples/inference/pytorch_language_modeling.py as an example that does this?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added

@tvalentyn tvalentyn changed the title Rszper run inference docs Update RunInference documentation Jul 15, 2022
@@ -171,7 +171,7 @@ In some cases, the `PredictionResults` output might not include the correct pred

The RunInference API currently expects outputs to be an `Iterable[Any]`. Example return types are `Iterable[Tensor]` or `Iterable[Dict[str, Tensor]]`. When RunInference zips the inputs with the predictions, the predictions iterate over the dictionary keys instead of the batch elements. The result is that the key name is preserved but the prediction tensors are discarded. For more information, see the [Pytorch RunInference PredictionResult is a Dict](https://github.com/apache/beam/issues/22240) issue in the Apache Beam GitHub project.

To work with the current RunInference implementation, you can create a wrapper class that overrides the `model(input)` call. In PyTorch, for example, your wrapper would override the `forward()` function and return an output with the appropriate format of `List[Dict[str, torch.Tensor]]`. For more information, see our [HuggingFace language modeling example](https://github.com/apache/beam/blob/master/sdks/python/apache_beam/examples/inference/pytorch_language_modeling.py#L49).
To work with the current RunInference implementation, you can create a wrapper class that overrides the `model(input)` call. In PyTorch, for example, your wrapper would override the `forward()` function and return an output with the appropriate format of `List[Dict[str, torch.Tensor]]`. For more information, see our [HuggingFace language modeling example](https://github.com/apache/beam/blob/master/sdks/python/apache_beam/examples/inference/pytorch_language_modeling.py#L49) and our [Bert language modeling example](https://github.com/apache/beam/blob/master/sdks/python/apache_beam/examples/inference/pytorch_language_modeling.py).
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

these are the same links, looks like not the change we intended to make?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

my last comment referred to disable batching section

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oops. This should be fixed now.

@tvalentyn tvalentyn merged commit fa028d3 into apache:master Jul 15, 2022
@rszper rszper deleted the rszper-runInferenceDocs branch July 18, 2022 15:57
lostluck pushed a commit to lostluck/beam that referenced this pull request Aug 26, 2022
Co-authored-by: Anand Inguva <34158215+AnandInguva@users.noreply.github.com>
Co-authored-by: Andy Ye <andyye333@gmail.com>
Co-authored-by: Anand Inguva <anandinguva98@gmail.com>
Co-authored-by: Anand Inguva <anandinguva@google.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants