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/07/05 23:08:43 UTC
[GitHub] [beam] AnandInguva opened a new pull request, #22164: Modify RunInference to return PipelineResult for the benchmark tests
AnandInguva opened a new pull request, #22164:
URL: https://github.com/apache/beam/pull/22164
RunInference benchmarks tests uses the [example](https://github.com/apache/beam/tree/master/sdks/python/apache_beam/examples/inference) to run the pipeline.
These tests utilizes the `LoadTest` class to publish metrics to BigQuery and InfluxDB.
This PR modifies the pipeline to use a `test_pipeline` param for the benchmark testing `Pipeline` object
------------------------
Thank you for your contribution! Follow this checklist to help us incorporate your contribution quickly and easily:
- [ ] [**Choose reviewer(s)**](https://beam.apache.org/contribute/#make-your-change) and mention them in a comment (`R: @username`).
- [ ] Mention the appropriate issue in your description (for example: `addresses #123`), if applicable. This will automatically add a link to the pull request in the issue. If you would like the issue to automatically close on merging the pull request, comment `fixes #<ISSUE NUMBER>` instead.
- [ ] Update `CHANGES.md` with noteworthy changes.
- [ ] If this contribution is large, please file an Apache [Individual Contributor License Agreement](https://www.apache.org/licenses/icla.pdf).
See the [Contributor Guide](https://beam.apache.org/contribute) for more tips on [how to make review process smoother](https://beam.apache.org/contribute/#make-reviewers-job-easier).
To check the build health, please visit [https://github.com/apache/beam/blob/master/.test-infra/BUILD_STATUS.md](https://github.com/apache/beam/blob/master/.test-infra/BUILD_STATUS.md)
GitHub Actions Tests Status (on master branch)
------------------------------------------------------------------------------------------------
[](https://github.com/apache/beam/actions?query=workflow%3A%22Build+python+source+distribution+and+wheels%22+branch%3Amaster+event%3Aschedule)
[](https://github.com/apache/beam/actions?query=workflow%3A%22Python+Tests%22+branch%3Amaster+event%3Aschedule)
[](https://github.com/apache/beam/actions?query=workflow%3A%22Java+Tests%22+branch%3Amaster+event%3Aschedule)
See [CI.md](https://github.com/apache/beam/blob/master/CI.md) for more information about GitHub Actions CI.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] yeandy commented on a diff in pull request #22164: Modify RunInference to return PipelineResult for the benchmark tests
Posted by GitBox <gi...@apache.org>.
yeandy commented on code in PR #22164:
URL: https://github.com/apache/beam/pull/22164#discussion_r916082915
##########
sdks/python/apache_beam/examples/inference/pytorch_language_modeling.py:
##########
@@ -131,6 +133,7 @@ def parse_known_args(argv):
"""Parses args for the workflow."""
parser = argparse.ArgumentParser()
parser.add_argument(
+ '--input_file',
'--input',
Review Comment:
It's just proposing to changing the order of the two `input` and then `input_file`.
And sounds good
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] AnandInguva commented on pull request #22164: Modify RunInference to return PipelineResult for the benchmark tests
Posted by GitBox <gi...@apache.org>.
AnandInguva commented on PR #22164:
URL: https://github.com/apache/beam/pull/22164#issuecomment-1179941594
> Where/how will the `PipelineResult` be used? do you have an example?
>
> These tests are skipped for now in the PostCommits. Did you test locally?
Yes, i tested them and they run locally
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] AnandInguva commented on a diff in pull request #22164: Modify RunInference to return PipelineResult for the benchmark tests
Posted by GitBox <gi...@apache.org>.
AnandInguva commented on code in PR #22164:
URL: https://github.com/apache/beam/pull/22164#discussion_r931768362
##########
sdks/python/apache_beam/examples/inference/sklearn_mnist_classification.py:
##########
@@ -60,7 +62,7 @@ def parse_known_args(argv):
"""Parses args for the workflow."""
parser = argparse.ArgumentParser()
parser.add_argument(
- '--input_file',
+ '--input',
Review Comment:
+1 with @rszper.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] tvalentyn commented on a diff in pull request #22164: Modify RunInference to return PipelineResult for the benchmark tests
Posted by GitBox <gi...@apache.org>.
tvalentyn commented on code in PR #22164:
URL: https://github.com/apache/beam/pull/22164#discussion_r931621587
##########
sdks/python/apache_beam/examples/inference/sklearn_mnist_classification.py:
##########
@@ -60,7 +62,7 @@ def parse_known_args(argv):
"""Parses args for the workflow."""
parser = argparse.ArgumentParser()
parser.add_argument(
- '--input_file',
+ '--input',
Review Comment:
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] AnandInguva commented on pull request #22164: Modify RunInference to return PipelineResult for the benchmark tests
Posted by GitBox <gi...@apache.org>.
AnandInguva commented on PR #22164:
URL: https://github.com/apache/beam/pull/22164#issuecomment-1197645009
Changed md file to reflect the change. Thanks. PTAL @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] AnandInguva commented on pull request #22164: Modify RunInference to return PipelineResult for the benchmark tests
Posted by GitBox <gi...@apache.org>.
AnandInguva commented on PR #22164:
URL: https://github.com/apache/beam/pull/22164#issuecomment-1197645138
Run Python 3.7 PostCommit
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] AnandInguva commented on pull request #22164: Modify RunInference to return PipelineResult for the benchmark tests
Posted by GitBox <gi...@apache.org>.
AnandInguva commented on PR #22164:
URL: https://github.com/apache/beam/pull/22164#issuecomment-1180070729
https://ci-beam.apache.org/job/beam_PostCommit_Python37_PR/361/console
I un-skipped the tests and ran it on the Jenkins as well. This failed due to a different test but the InferenceTests pass 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] github-actions[bot] commented on pull request #22164: Modify RunInference to return PipelineResult for the benchmark tests
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on PR #22164:
URL: https://github.com/apache/beam/pull/22164#issuecomment-1187249173
Reminder, please take a look at this pr: @yeandy
--
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 #22164: Modify RunInference to return PipelineResult for the benchmark tests
Posted by GitBox <gi...@apache.org>.
tvalentyn commented on PR #22164:
URL: https://github.com/apache/beam/pull/22164#issuecomment-1195749813
Run Python Precommit
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] AnandInguva commented on pull request #22164: Modify RunInference to return PipelineResult for the benchmark tests
Posted by GitBox <gi...@apache.org>.
AnandInguva commented on PR #22164:
URL: https://github.com/apache/beam/pull/22164#issuecomment-1189884047
R: @tvalentyn . This PR is needed for the benchmark tests as I need `PipelineResult` object to publish metrics to the BQ using `LoadTest` class
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] tvalentyn commented on a diff in pull request #22164: Modify RunInference to return PipelineResult for the benchmark tests
Posted by GitBox <gi...@apache.org>.
tvalentyn commented on code in PR #22164:
URL: https://github.com/apache/beam/pull/22164#discussion_r927319071
##########
sdks/python/apache_beam/examples/inference/pytorch_image_classification.py:
##########
@@ -95,21 +98,28 @@ def parse_known_args(argv):
return parser.parse_known_args(argv)
-def run(argv=None, model_class=None, model_params=None, save_main_session=True):
+def run(
+ argv=None,
+ model_class=None,
+ model_params=None,
+ save_main_session=True,
+ test_pipeline=None) -> PipelineResult:
"""
Args:
argv: Command line arguments defined for this example.
model_class: Reference to the class definition of the model.
model_params: Parameters passed to the constructor of the model_class.
These will be used to instantiate the model object in the
RunInference API.
+ test_pipeline: used for internal testing. No backwards-compatibility,
Review Comment:
Remove `No backwards-compatibility,`
This is an example, not an API. Backwards compatibility is not relevant.
##########
sdks/python/apache_beam/examples/inference/pytorch_image_classification.py:
##########
@@ -95,21 +98,28 @@ def parse_known_args(argv):
return parser.parse_known_args(argv)
-def run(argv=None, model_class=None, model_params=None, save_main_session=True):
+def run(
+ argv=None,
+ model_class=None,
+ model_params=None,
+ save_main_session=True,
+ test_pipeline=None) -> PipelineResult:
"""
Args:
argv: Command line arguments defined for this example.
model_class: Reference to the class definition of the model.
model_params: Parameters passed to the constructor of the model_class.
These will be used to instantiate the model object in the
RunInference API.
+ test_pipeline: used for internal testing. No backwards-compatibility,
Review Comment:
Add that save_main_session is used for testing only as well.
##########
sdks/python/apache_beam/examples/inference/pytorch_image_classification.py:
##########
@@ -72,6 +74,7 @@ def parse_known_args(argv):
"""Parses args for the workflow."""
parser = argparse.ArgumentParser()
parser.add_argument(
+ '--input_file',
Review Comment:
why do we need two flags? Should we just use `--input` throughout all the examples & docs?
##########
sdks/python/apache_beam/examples/inference/pytorch_image_classification.py:
##########
@@ -120,27 +130,35 @@ def run(argv=None, model_class=None, model_params=None, save_main_session=True):
model_class=model_class,
model_params=model_params))
- with beam.Pipeline(options=pipeline_options) as p:
- filename_value_pair = (
- p
- | 'ReadImageNames' >> beam.io.ReadFromText(
- known_args.input, skip_header_lines=1)
- | 'ReadImageData' >> beam.Map(
- lambda image_name: read_image(
- image_file_name=image_name, path_to_dir=known_args.images_dir))
- | 'PreprocessImages' >> beam.MapTuple(
- lambda file_name, data: (file_name, preprocess_image(data))))
- predictions = (
- filename_value_pair
- | 'PyTorchRunInference' >> RunInference(model_handler)
- | 'ProcessOutput' >> beam.ParDo(PostProcessor()))
-
- if known_args.output:
- predictions | "WriteOutputToGCS" >> beam.io.WriteToText( # pylint: disable=expression-not-assigned
- known_args.output,
- shard_name_template='',
- append_trailing_newlines=True)
+ if not test_pipeline:
Review Comment:
you could s/`test_pipeline`/`pipeline` in args, and delete 135-136.
##########
sdks/python/apache_beam/examples/inference/pytorch_image_classification.py:
##########
@@ -95,21 +98,28 @@ def parse_known_args(argv):
return parser.parse_known_args(argv)
-def run(argv=None, model_class=None, model_params=None, save_main_session=True):
+def run(
+ argv=None,
+ model_class=None,
+ model_params=None,
+ save_main_session=True,
+ test_pipeline=None) -> PipelineResult:
"""
Args:
argv: Command line arguments defined for this example.
model_class: Reference to the class definition of the model.
model_params: Parameters passed to the constructor of the model_class.
These will be used to instantiate the model object in the
RunInference API.
+ test_pipeline: used for internal testing. No backwards-compatibility,
Review Comment:
Use consistent capitalization in docstring, e.g. `Used` instead of `used`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] AnandInguva commented on a diff in pull request #22164: Modify RunInference to return PipelineResult for the benchmark tests
Posted by GitBox <gi...@apache.org>.
AnandInguva commented on code in PR #22164:
URL: https://github.com/apache/beam/pull/22164#discussion_r929612584
##########
sdks/python/apache_beam/examples/inference/pytorch_image_classification.py:
##########
@@ -72,6 +74,7 @@ def parse_known_args(argv):
"""Parses args for the workflow."""
parser = argparse.ArgumentParser()
parser.add_argument(
+ '--input_file',
Review Comment:
Removed input_file.
##########
sdks/python/apache_beam/examples/inference/pytorch_image_classification.py:
##########
@@ -95,21 +98,28 @@ def parse_known_args(argv):
return parser.parse_known_args(argv)
-def run(argv=None, model_class=None, model_params=None, save_main_session=True):
+def run(
+ argv=None,
+ model_class=None,
+ model_params=None,
+ save_main_session=True,
+ test_pipeline=None) -> PipelineResult:
"""
Args:
argv: Command line arguments defined for this example.
model_class: Reference to the class definition of the model.
model_params: Parameters passed to the constructor of the model_class.
These will be used to instantiate the model object in the
RunInference API.
+ test_pipeline: used for internal testing. No backwards-compatibility,
Review Comment:
Done
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] AnandInguva commented on pull request #22164: Modify RunInference to return PipelineResult for the benchmark tests
Posted by GitBox <gi...@apache.org>.
AnandInguva commented on PR #22164:
URL: https://github.com/apache/beam/pull/22164#issuecomment-1220668236
PTAL : @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] AnandInguva commented on pull request #22164: Modify RunInference to return PipelineResult for the benchmark tests
Posted by GitBox <gi...@apache.org>.
AnandInguva commented on PR #22164:
URL: https://github.com/apache/beam/pull/22164#issuecomment-1197135189
Python 3.7 PostCommit errors are not related to this change.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] AnandInguva commented on a diff in pull request #22164: Modify RunInference to return PipelineResult for the benchmark tests
Posted by GitBox <gi...@apache.org>.
AnandInguva commented on code in PR #22164:
URL: https://github.com/apache/beam/pull/22164#discussion_r929616263
##########
sdks/python/apache_beam/examples/inference/pytorch_image_classification.py:
##########
@@ -120,27 +130,35 @@ def run(argv=None, model_class=None, model_params=None, save_main_session=True):
model_class=model_class,
model_params=model_params))
- with beam.Pipeline(options=pipeline_options) as p:
- filename_value_pair = (
- p
- | 'ReadImageNames' >> beam.io.ReadFromText(
- known_args.input, skip_header_lines=1)
- | 'ReadImageData' >> beam.Map(
- lambda image_name: read_image(
- image_file_name=image_name, path_to_dir=known_args.images_dir))
- | 'PreprocessImages' >> beam.MapTuple(
- lambda file_name, data: (file_name, preprocess_image(data))))
- predictions = (
- filename_value_pair
- | 'PyTorchRunInference' >> RunInference(model_handler)
- | 'ProcessOutput' >> beam.ParDo(PostProcessor()))
-
- if known_args.output:
- predictions | "WriteOutputToGCS" >> beam.io.WriteToText( # pylint: disable=expression-not-assigned
- known_args.output,
- shard_name_template='',
- append_trailing_newlines=True)
+ if not test_pipeline:
Review Comment:
This test_pipeline comes from the `load_test` and after the pipeline is ran, the test_pipeline object is used in the `load_test` to publish metrics. So I need its state to be preserved through out the E2E test. That's why I passed it as an argument to the example.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] AnandInguva commented on pull request #22164: Modify RunInference to return PipelineResult for the benchmark tests
Posted by GitBox <gi...@apache.org>.
AnandInguva commented on PR #22164:
URL: https://github.com/apache/beam/pull/22164#issuecomment-1195139888
Run Python 3.9 PostCommit
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] AnandInguva commented on a diff in pull request #22164: Modify RunInference to return PipelineResult for the benchmark tests
Posted by GitBox <gi...@apache.org>.
AnandInguva commented on code in PR #22164:
URL: https://github.com/apache/beam/pull/22164#discussion_r916058751
##########
sdks/python/apache_beam/examples/inference/pytorch_language_modeling.py:
##########
@@ -131,6 +133,7 @@ def parse_known_args(argv):
"""Parses args for the workflow."""
parser = argparse.ArgumentParser()
parser.add_argument(
+ '--input_file',
'--input',
Review Comment:
These are used in this pattern https://github.com/apache/beam/blob/ca653ed0ef7b504536f4bb7968d3d8dd5d4ccf0d/sdks/python/apache_beam/testing/benchmarks/inference/pytorch_benchmarks.py#L55
I forgot that these are getting skipped. I will test them locally. 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] commented on pull request #22164: Modify RunInference to return PipelineResult for the benchmark tests
Posted by GitBox <gi...@apache.org>.
codecov[bot] commented on PR #22164:
URL: https://github.com/apache/beam/pull/22164#issuecomment-1175598552
# [Codecov](https://codecov.io/gh/apache/beam/pull/22164?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 [#22164](https://codecov.io/gh/apache/beam/pull/22164?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (c0d0d4a) into [master](https://codecov.io/gh/apache/beam/commit/b53b16f1fb41913b0e8bbe9d64d71b8e3ebfbbf6?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (b53b16f) will **decrease** coverage by `0.04%`.
> The diff coverage is `5.26%`.
```diff
@@ Coverage Diff @@
## master #22164 +/- ##
==========================================
- Coverage 74.22% 74.18% -0.05%
==========================================
Files 702 702
Lines 92829 92860 +31
==========================================
- Hits 68903 68887 -16
- Misses 22659 22706 +47
Partials 1267 1267
```
| Flag | Coverage Δ | |
|---|---|---|
| python | `83.55% <5.26%> (-0.07%)` | :arrow_down: |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/beam/pull/22164?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [...examples/inference/pytorch\_image\_classification.py](https://codecov.io/gh/apache/beam/pull/22164/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-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vZXhhbXBsZXMvaW5mZXJlbmNlL3B5dG9yY2hfaW1hZ2VfY2xhc3NpZmljYXRpb24ucHk=) | `0.00% <0.00%> (ø)` | |
| [...m/examples/inference/pytorch\_image\_segmentation.py](https://codecov.io/gh/apache/beam/pull/22164/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-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vZXhhbXBsZXMvaW5mZXJlbmNlL3B5dG9yY2hfaW1hZ2Vfc2VnbWVudGF0aW9uLnB5) | `0.00% <0.00%> (ø)` | |
| [...am/examples/inference/pytorch\_language\_modeling.py](https://codecov.io/gh/apache/beam/pull/22164/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-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vZXhhbXBsZXMvaW5mZXJlbmNlL3B5dG9yY2hfbGFuZ3VhZ2VfbW9kZWxpbmcucHk=) | `0.00% <0.00%> (ø)` | |
| [...examples/inference/sklearn\_mnist\_classification.py](https://codecov.io/gh/apache/beam/pull/22164/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-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vZXhhbXBsZXMvaW5mZXJlbmNlL3NrbGVhcm5fbW5pc3RfY2xhc3NpZmljYXRpb24ucHk=) | `43.75% <23.07%> (-3.75%)` | :arrow_down: |
| [sdks/python/apache\_beam/utils/interactive\_utils.py](https://codecov.io/gh/apache/beam/pull/22164/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-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vdXRpbHMvaW50ZXJhY3RpdmVfdXRpbHMucHk=) | `95.12% <0.00%> (-2.44%)` | :arrow_down: |
| [...python/apache\_beam/runners/worker/worker\_status.py](https://codecov.io/gh/apache/beam/pull/22164/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-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy93b3JrZXIvd29ya2VyX3N0YXR1cy5weQ==) | `77.53% <0.00%> (-2.18%)` | :arrow_down: |
| [sdks/python/apache\_beam/io/source\_test\_utils.py](https://codecov.io/gh/apache/beam/pull/22164/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vaW8vc291cmNlX3Rlc3RfdXRpbHMucHk=) | `88.01% <0.00%> (-1.39%)` | :arrow_down: |
| [sdks/python/apache\_beam/io/localfilesystem.py](https://codecov.io/gh/apache/beam/pull/22164/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-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vaW8vbG9jYWxmaWxlc3lzdGVtLnB5) | `90.97% <0.00%> (-0.76%)` | :arrow_down: |
| [...hon/apache\_beam/runners/direct/test\_stream\_impl.py](https://codecov.io/gh/apache/beam/pull/22164/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-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy9kaXJlY3QvdGVzdF9zdHJlYW1faW1wbC5weQ==) | `93.28% <0.00%> (-0.75%)` | :arrow_down: |
| [...eam/runners/portability/fn\_api\_runner/execution.py](https://codecov.io/gh/apache/beam/pull/22164/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy9wb3J0YWJpbGl0eS9mbl9hcGlfcnVubmVyL2V4ZWN1dGlvbi5weQ==) | `92.44% <0.00%> (-0.65%)` | :arrow_down: |
| ... and [3 more](https://codecov.io/gh/apache/beam/pull/22164/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/22164?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/22164?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 [b53b16f...c0d0d4a](https://codecov.io/gh/apache/beam/pull/22164?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 pull request #22164: Modify RunInference to return PipelineResult for the benchmark tests
Posted by GitBox <gi...@apache.org>.
AnandInguva commented on PR #22164:
URL: https://github.com/apache/beam/pull/22164#issuecomment-1175590548
Run Python 3.7 PostCommit
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] AnandInguva commented on pull request #22164: Modify RunInference to return PipelineResult for the benchmark tests
Posted by GitBox <gi...@apache.org>.
AnandInguva commented on PR #22164:
URL: https://github.com/apache/beam/pull/22164#issuecomment-1195139752
Run Python 3.7 PostCommit
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] tvalentyn commented on a diff in pull request #22164: Modify RunInference to return PipelineResult for the benchmark tests
Posted by GitBox <gi...@apache.org>.
tvalentyn commented on code in PR #22164:
URL: https://github.com/apache/beam/pull/22164#discussion_r931613165
##########
sdks/python/apache_beam/examples/inference/sklearn_mnist_classification.py:
##########
@@ -60,7 +62,7 @@ def parse_known_args(argv):
"""Parses args for the workflow."""
parser = argparse.ArgumentParser()
parser.add_argument(
- '--input_file',
+ '--input',
Review Comment:
let's update the markdown at: https://github.com/apache/beam/tree/master/sdks/python/apache_beam/examples/inference
to reflect this change. can be a separate pr.
@rszper , @rezarokni are there any documentations/notebooks where we provide complete command line to run examples and might need to fix this?
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] tvalentyn commented on a diff in pull request #22164: Modify RunInference to return PipelineResult for the benchmark tests
Posted by GitBox <gi...@apache.org>.
tvalentyn commented on code in PR #22164:
URL: https://github.com/apache/beam/pull/22164#discussion_r930217928
##########
sdks/python/apache_beam/examples/inference/pytorch_image_classification.py:
##########
@@ -120,27 +130,35 @@ def run(argv=None, model_class=None, model_params=None, save_main_session=True):
model_class=model_class,
model_params=model_params))
- with beam.Pipeline(options=pipeline_options) as p:
- filename_value_pair = (
- p
- | 'ReadImageNames' >> beam.io.ReadFromText(
- known_args.input, skip_header_lines=1)
- | 'ReadImageData' >> beam.Map(
- lambda image_name: read_image(
- image_file_name=image_name, path_to_dir=known_args.images_dir))
- | 'PreprocessImages' >> beam.MapTuple(
- lambda file_name, data: (file_name, preprocess_image(data))))
- predictions = (
- filename_value_pair
- | 'PyTorchRunInference' >> RunInference(model_handler)
- | 'ProcessOutput' >> beam.ParDo(PostProcessor()))
-
- if known_args.output:
- predictions | "WriteOutputToGCS" >> beam.io.WriteToText( # pylint: disable=expression-not-assigned
- known_args.output,
- shard_name_template='',
- append_trailing_newlines=True)
+ if not test_pipeline:
Review Comment:
current version is fine too though.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] tvalentyn commented on a diff in pull request #22164: Modify RunInference to return PipelineResult for the benchmark tests
Posted by GitBox <gi...@apache.org>.
tvalentyn commented on code in PR #22164:
URL: https://github.com/apache/beam/pull/22164#discussion_r930146853
##########
sdks/python/apache_beam/examples/inference/pytorch_image_classification.py:
##########
@@ -120,27 +130,35 @@ def run(argv=None, model_class=None, model_params=None, save_main_session=True):
model_class=model_class,
model_params=model_params))
- with beam.Pipeline(options=pipeline_options) as p:
- filename_value_pair = (
- p
- | 'ReadImageNames' >> beam.io.ReadFromText(
- known_args.input, skip_header_lines=1)
- | 'ReadImageData' >> beam.Map(
- lambda image_name: read_image(
- image_file_name=image_name, path_to_dir=known_args.images_dir))
- | 'PreprocessImages' >> beam.MapTuple(
- lambda file_name, data: (file_name, preprocess_image(data))))
- predictions = (
- filename_value_pair
- | 'PyTorchRunInference' >> RunInference(model_handler)
- | 'ProcessOutput' >> beam.ParDo(PostProcessor()))
-
- if known_args.output:
- predictions | "WriteOutputToGCS" >> beam.io.WriteToText( # pylint: disable=expression-not-assigned
- known_args.output,
- shard_name_template='',
- append_trailing_newlines=True)
+ if not test_pipeline:
Review Comment:
I understand that, i meant that we could call the `test_pipeline` args just `pipeline`, and then have the code like:
```
...
pipeline: used for internal testing. No backwards-compatibility,
...
if not pipeline:
pipeline = ...
```
--
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] github-actions[bot] commented on pull request #22164: Modify RunInference to return PipelineResult for the benchmark tests
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on PR #22164:
URL: https://github.com/apache/beam/pull/22164#issuecomment-1189888156
Stopping reviewer notifications for this pull request: review requested by someone other than the bot, ceding control
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] tvalentyn commented on a diff in pull request #22164: Modify RunInference to return PipelineResult for the benchmark tests
Posted by GitBox <gi...@apache.org>.
tvalentyn commented on code in PR #22164:
URL: https://github.com/apache/beam/pull/22164#discussion_r930146853
##########
sdks/python/apache_beam/examples/inference/pytorch_image_classification.py:
##########
@@ -120,27 +130,35 @@ def run(argv=None, model_class=None, model_params=None, save_main_session=True):
model_class=model_class,
model_params=model_params))
- with beam.Pipeline(options=pipeline_options) as p:
- filename_value_pair = (
- p
- | 'ReadImageNames' >> beam.io.ReadFromText(
- known_args.input, skip_header_lines=1)
- | 'ReadImageData' >> beam.Map(
- lambda image_name: read_image(
- image_file_name=image_name, path_to_dir=known_args.images_dir))
- | 'PreprocessImages' >> beam.MapTuple(
- lambda file_name, data: (file_name, preprocess_image(data))))
- predictions = (
- filename_value_pair
- | 'PyTorchRunInference' >> RunInference(model_handler)
- | 'ProcessOutput' >> beam.ParDo(PostProcessor()))
-
- if known_args.output:
- predictions | "WriteOutputToGCS" >> beam.io.WriteToText( # pylint: disable=expression-not-assigned
- known_args.output,
- shard_name_template='',
- append_trailing_newlines=True)
+ if not test_pipeline:
Review Comment:
I understand that, i meant that we could call the `test_pipeline` args just `pipeline`, and then have the code like:
```
...
pipeline: Used for internal testing.
...
if not pipeline:
pipeline = ...
```
--
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] github-actions[bot] commented on pull request #22164: Modify RunInference to return PipelineResult for the benchmark tests
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on PR #22164:
URL: https://github.com/apache/beam/pull/22164#issuecomment-1175642366
Assigning reviewers. If you would like to opt out of this review, comment `assign to next reviewer`:
R: @yeandy for label python.
Available commands:
- `stop reviewer notifications` - opt out of the automated review tooling
- `remind me after tests pass` - tag the comment author after tests pass
- `waiting on author` - shift the attention set back to the author (any comment or push by the author will return the attention set to the reviewers)
The PR bot will only process comments in the main thread (not review comments).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] AnandInguva commented on pull request #22164: Modify RunInference to return PipelineResult for the benchmark tests
Posted by GitBox <gi...@apache.org>.
AnandInguva commented on PR #22164:
URL: https://github.com/apache/beam/pull/22164#issuecomment-1220232908
Run Python 3.7 PostCommit
--
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] rszper commented on a diff in pull request #22164: Modify RunInference to return PipelineResult for the benchmark tests
Posted by GitBox <gi...@apache.org>.
rszper commented on code in PR #22164:
URL: https://github.com/apache/beam/pull/22164#discussion_r931620492
##########
sdks/python/apache_beam/examples/inference/sklearn_mnist_classification.py:
##########
@@ -60,7 +62,7 @@ def parse_known_args(argv):
"""Parses args for the workflow."""
parser = argparse.ArgumentParser()
parser.add_argument(
- '--input_file',
+ '--input',
Review Comment:
Not in the documentation that I created. @AnandInguva has been working on notebooks, but I'm not sure if they would be related to this change.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] tvalentyn commented on a diff in pull request #22164: Modify RunInference to return PipelineResult for the benchmark tests
Posted by GitBox <gi...@apache.org>.
tvalentyn commented on code in PR #22164:
URL: https://github.com/apache/beam/pull/22164#discussion_r930146853
##########
sdks/python/apache_beam/examples/inference/pytorch_image_classification.py:
##########
@@ -120,27 +130,35 @@ def run(argv=None, model_class=None, model_params=None, save_main_session=True):
model_class=model_class,
model_params=model_params))
- with beam.Pipeline(options=pipeline_options) as p:
- filename_value_pair = (
- p
- | 'ReadImageNames' >> beam.io.ReadFromText(
- known_args.input, skip_header_lines=1)
- | 'ReadImageData' >> beam.Map(
- lambda image_name: read_image(
- image_file_name=image_name, path_to_dir=known_args.images_dir))
- | 'PreprocessImages' >> beam.MapTuple(
- lambda file_name, data: (file_name, preprocess_image(data))))
- predictions = (
- filename_value_pair
- | 'PyTorchRunInference' >> RunInference(model_handler)
- | 'ProcessOutput' >> beam.ParDo(PostProcessor()))
-
- if known_args.output:
- predictions | "WriteOutputToGCS" >> beam.io.WriteToText( # pylint: disable=expression-not-assigned
- known_args.output,
- shard_name_template='',
- append_trailing_newlines=True)
+ if not test_pipeline:
Review Comment:
I understand that, i meant that we could call the `test_pipeline` args just `pipeline`, and then have the code like:
```
...
pipeline: used for internal testing.
...
if not pipeline:
pipeline = ...
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] yeandy commented on a diff in pull request #22164: Modify RunInference to return PipelineResult for the benchmark tests
Posted by GitBox <gi...@apache.org>.
yeandy commented on code in PR #22164:
URL: https://github.com/apache/beam/pull/22164#discussion_r915244374
##########
sdks/python/apache_beam/examples/inference/pytorch_language_modeling.py:
##########
@@ -131,6 +133,7 @@ def parse_known_args(argv):
"""Parses args for the workflow."""
parser = argparse.ArgumentParser()
parser.add_argument(
+ '--input_file',
'--input',
Review Comment:
```suggestion
'--input',
'--input_file',
```
--
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] github-actions[bot] commented on pull request #22164: Modify RunInference to return PipelineResult for the benchmark tests
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on PR #22164:
URL: https://github.com/apache/beam/pull/22164#issuecomment-1188309312
R: @y1chi for final approval
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] AnandInguva commented on a diff in pull request #22164: Modify RunInference to return PipelineResult for the benchmark tests
Posted by GitBox <gi...@apache.org>.
AnandInguva commented on code in PR #22164:
URL: https://github.com/apache/beam/pull/22164#discussion_r931768362
##########
sdks/python/apache_beam/examples/inference/sklearn_mnist_classification.py:
##########
@@ -60,7 +62,7 @@ def parse_known_args(argv):
"""Parses args for the workflow."""
parser = argparse.ArgumentParser()
parser.add_argument(
- '--input_file',
+ '--input',
Review Comment:
+1 with @rszper. The markdown seems fine with the current implementation as well.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] AnandInguva commented on pull request #22164: Modify RunInference to return PipelineResult for the benchmark tests
Posted by GitBox <gi...@apache.org>.
AnandInguva commented on PR #22164:
URL: https://github.com/apache/beam/pull/22164#issuecomment-1197645235
Run Python 3.9 PostCommit
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] AnandInguva commented on pull request #22164: Modify RunInference to return PipelineResult for the benchmark tests
Posted by GitBox <gi...@apache.org>.
AnandInguva commented on PR #22164:
URL: https://github.com/apache/beam/pull/22164#issuecomment-1220232987
Run Python 3.9 PostCommit
--
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 #22164: Modify RunInference to return PipelineResult for the benchmark tests
Posted by GitBox <gi...@apache.org>.
tvalentyn merged PR #22164:
URL: https://github.com/apache/beam/pull/22164
--
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