tvalentyn commented on code in PR #17462: URL: https://github.com/apache/beam/pull/17462#discussion_r879311719
########## sdks/python/apache_beam/examples/inference/pytorch_image_classification.py: ########## @@ -0,0 +1,122 @@ +# +# 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 argparse +import io +import os +from functools import partial + +import apache_beam as beam +import torch +import torchvision +from apache_beam.io.filesystems import FileSystems +from apache_beam.ml.inference.api import RunInference +from apache_beam.ml.inference.pytorch import PytorchModelLoader +from apache_beam.options.pipeline_options import PipelineOptions +from apache_beam.options.pipeline_options import SetupOptions +from PIL import Image +from torchvision import transforms + + +def read_image(image_file_name: str, path_to_dir: str): + image_file_name = os.path.join(path_to_dir, image_file_name) + with FileSystems().open(image_file_name, 'r') as file: + data = Image.open(io.BytesIO(file.read())).convert('RGB') + return image_file_name, data + + +def preprocess_data(data): + image_size = (224, 224) + normalize = transforms.Normalize( + mean=[0.485, 0.456, 0.406], std=[0.229, 0.224, 0.225]) Review Comment: Can you add a comment on how these numbers were chosen? ########## sdks/python/apache_beam/examples/inference/pytorch_image_classification.py: ########## @@ -0,0 +1,122 @@ +# +# 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 argparse +import io +import os +from functools import partial + +import apache_beam as beam +import torch +import torchvision +from apache_beam.io.filesystems import FileSystems +from apache_beam.ml.inference.api import RunInference +from apache_beam.ml.inference.pytorch import PytorchModelLoader +from apache_beam.options.pipeline_options import PipelineOptions +from apache_beam.options.pipeline_options import SetupOptions +from PIL import Image +from torchvision import transforms + + +def read_image(image_file_name: str, path_to_dir: str): Review Comment: Given this is an example, let's add typehints throughout, including the return types. ########## sdks/python/apache_beam/examples/inference/pytorch_image_classification.py: ########## @@ -0,0 +1,122 @@ +# +# 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 argparse +import io +import os +from functools import partial + +import apache_beam as beam +import torch +import torchvision +from apache_beam.io.filesystems import FileSystems +from apache_beam.ml.inference.api import RunInference +from apache_beam.ml.inference.pytorch import PytorchModelLoader +from apache_beam.options.pipeline_options import PipelineOptions +from apache_beam.options.pipeline_options import SetupOptions +from PIL import Image +from torchvision import transforms + + +def read_image(image_file_name: str, path_to_dir: str): + image_file_name = os.path.join(path_to_dir, image_file_name) + with FileSystems().open(image_file_name, 'r') as file: + data = Image.open(io.BytesIO(file.read())).convert('RGB') + return image_file_name, data + + +def preprocess_data(data): + image_size = (224, 224) + normalize = transforms.Normalize( + mean=[0.485, 0.456, 0.406], std=[0.229, 0.224, 0.225]) + transform = transforms.Compose([ + transforms.Resize(image_size), + transforms.ToTensor(), + normalize, + ]) + return transform(data) + + +class PostProcessor(beam.DoFn): + """Post process PredictionResult to output filename and + prediction using torch.""" + def process(self, element): + filename, prediction_result = element + prediction = torch.argmax(prediction_result.inference, dim=0) + yield filename + ',' + str(int(prediction)) + + +def run_pipeline(options: PipelineOptions, args=None): + """Sets up PyTorch RunInference pipeline""" + model_class = torchvision.models.mobilenet_v2 + model_params = {'pretrained': False} Review Comment: for my education, why are we setting this bit? maybe leave a comment? ########## sdks/python/apache_beam/examples/inference/pytorch_image_classification.py: ########## @@ -0,0 +1,122 @@ +# +# 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 Review Comment: A few examples: https://github.com/apache/beam/blob/1dfab628d03e161cf003dad01f55b9d6674aa8e2/sdks/python/apache_beam/examples/dataframe/taxiride.py#L18 https://github.com/apache/beam/blob/1dfab628d03e161cf003dad01f55b9d6674aa8e2/sdks/python/apache_beam/examples/sql_taxi.py#L18 ########## build.gradle.kts: ########## @@ -312,25 +312,26 @@ tasks.register("python37PostCommit") { dependsOn(":sdks:python:test-suites:dataflow:py37:spannerioIT") dependsOn(":sdks:python:test-suites:direct:py37:spannerioIT") dependsOn(":sdks:python:test-suites:portable:py37:xlangSpannerIOIT") + dependsOn(":sdks:python:test-suites:dataflow:py37:torchTests") + } tasks.register("python38PostCommit") { dependsOn(":sdks:python:test-suites:dataflow:py38:postCommitIT") dependsOn(":sdks:python:test-suites:direct:py38:postCommitIT") dependsOn(":sdks:python:test-suites:direct:py38:hdfsIntegrationTest") dependsOn(":sdks:python:test-suites:portable:py38:postCommitPy38") + dependsOn(":sdks:python:test-suites:dataflow:py38:torchTests") Review Comment: consider running it only for the lowest version (py37) or lowest and highest (py37, py39). testing middle versions don't add as much value for most tests. ########## sdks/python/apache_beam/examples/inference/pytorch_image_classification.py: ########## @@ -0,0 +1,122 @@ +# +# 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 argparse +import io +import os +from functools import partial + +import apache_beam as beam +import torch +import torchvision +from apache_beam.io.filesystems import FileSystems +from apache_beam.ml.inference.api import RunInference +from apache_beam.ml.inference.pytorch import PytorchModelLoader +from apache_beam.options.pipeline_options import PipelineOptions +from apache_beam.options.pipeline_options import SetupOptions +from PIL import Image +from torchvision import transforms + + +def read_image(image_file_name: str, path_to_dir: str): + image_file_name = os.path.join(path_to_dir, image_file_name) + with FileSystems().open(image_file_name, 'r') as file: + data = Image.open(io.BytesIO(file.read())).convert('RGB') + return image_file_name, data + + +def preprocess_data(data): + image_size = (224, 224) + normalize = transforms.Normalize( + mean=[0.485, 0.456, 0.406], std=[0.229, 0.224, 0.225]) + transform = transforms.Compose([ + transforms.Resize(image_size), + transforms.ToTensor(), + normalize, + ]) + return transform(data) + + +class PostProcessor(beam.DoFn): + """Post process PredictionResult to output filename and + prediction using torch.""" Review Comment: Docstrings are typically either one-line, or a oneliner summary + multiline docstring that follows (https://peps.python.org/pep-0257/#one-line-docstrings) ########## sdks/python/apache_beam/ml/inference/pytorch_it_test.py: ########## @@ -0,0 +1,95 @@ +# +# 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. +# + +# pylint: skip-file + +"""End-to-End test for Pytorch Inference""" + +import logging +import unittest +import uuid + +import pytest + +from apache_beam.io.filesystems import FileSystems +from apache_beam.testing.test_pipeline import TestPipeline + +try: + import torch + from apache_beam.examples.inference import pytorch_image_classification +except ImportError as e: + torch = None + +_EXPECTED_OUTPUTS = { + 'gs://apache-beam-ml/temp_storage_end_to_end_testing/inputs/ILSVRC2012_val_00005001.JPEG': '681', + 'gs://apache-beam-ml/temp_storage_end_to_end_testing/inputs/ILSVRC2012_val_00005002.JPEG': '333', + 'gs://apache-beam-ml/temp_storage_end_to_end_testing/inputs/ILSVRC2012_val_00005003.JPEG': '711', + 'gs://apache-beam-ml/temp_storage_end_to_end_testing/inputs/ILSVRC2012_val_00005004.JPEG': '286', + 'gs://apache-beam-ml/temp_storage_end_to_end_testing/inputs/ILSVRC2012_val_00005005.JPEG': '433', + 'gs://apache-beam-ml/temp_storage_end_to_end_testing/inputs/ILSVRC2012_val_00005006.JPEG': '290', + 'gs://apache-beam-ml/temp_storage_end_to_end_testing/inputs/ILSVRC2012_val_00005007.JPEG': '890', + 'gs://apache-beam-ml/temp_storage_end_to_end_testing/inputs/ILSVRC2012_val_00005008.JPEG': '592', + 'gs://apache-beam-ml/temp_storage_end_to_end_testing/inputs/ILSVRC2012_val_00005009.JPEG': '406', + 'gs://apache-beam-ml/temp_storage_end_to_end_testing/inputs/ILSVRC2012_val_00005010.JPEG': '996', + 'gs://apache-beam-ml/temp_storage_end_to_end_testing/inputs/ILSVRC2012_val_00005011.JPEG': '327', + 'gs://apache-beam-ml/temp_storage_end_to_end_testing/inputs/ILSVRC2012_val_00005012.JPEG': '573' +} + + +def process_outputs(filepath): + with FileSystems().open(filepath) as f: + lines = f.readlines() + lines = [l.decode('utf-8').strip('\n') for l in lines] + return lines + + [email protected]( + torch is None, + 'Missing dependencies. ' + 'Test depends on torch, torchvision and pillow') Review Comment: due to this skip stanza, there is a risk that this test could silently stop running and we don't notice, for example if some dependency fails to install, or a new dependency needs to be added for a successful import. Possible solutions: - a dedicated single marker such as `uses_pytorch_it_postcommit`, and no skips here. Possible issues: test still gets executed in unit test suite that collects everything, and fails before reaching https://github.com/apache/beam/blob/1dfab628d03e161cf003dad01f55b9d6674aa8e2/sdks/python/apache_beam/testing/test_pipeline.py#L155. - setting an environment variable: `args '-c', ". ${envdir}/bin/activate && FORCE_PYTORCH_IT=1 ${runScriptsDir}/run_integration_test.sh $cmdArgs"` and then making the condition stronger `@unittest.skipIf(torch is None and os.getenv("FORCE_PYTORCH_IT") is None)` ########## sdks/python/apache_beam/examples/inference/pytorch_image_classification.py: ########## @@ -0,0 +1,122 @@ +# +# 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 Review Comment: Examples serve as one of the learning materials for users. Could you add a short intro about what this example demonstrates? ########## sdks/python/apache_beam/ml/inference/torch_tests_requirements.txt: ########## @@ -0,0 +1,20 @@ +# +# 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. +# + +torch>=1.7.1 +torchvision>=0.8.2 +pillow>=8.0.0 # bump the version to support Python 3.10 later Review Comment: with an open upper bound, we will not need to bump the version as newest versions will be installed. ########## sdks/python/apache_beam/examples/inference/pytorch_image_classification.py: ########## @@ -0,0 +1,122 @@ +# +# 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 argparse +import io +import os +from functools import partial + +import apache_beam as beam +import torch +import torchvision +from apache_beam.io.filesystems import FileSystems +from apache_beam.ml.inference.api import RunInference +from apache_beam.ml.inference.pytorch import PytorchModelLoader +from apache_beam.options.pipeline_options import PipelineOptions +from apache_beam.options.pipeline_options import SetupOptions +from PIL import Image +from torchvision import transforms + + +def read_image(image_file_name: str, path_to_dir: str): + image_file_name = os.path.join(path_to_dir, image_file_name) + with FileSystems().open(image_file_name, 'r') as file: + data = Image.open(io.BytesIO(file.read())).convert('RGB') + return image_file_name, data + + +def preprocess_data(data): Review Comment: how about calling this preprocess_image? ########## sdks/python/apache_beam/ml/inference/pytorch_it_test.py: ########## @@ -0,0 +1,95 @@ +# +# 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. +# + +# pylint: skip-file + +"""End-to-End test for Pytorch Inference""" + +import logging +import unittest +import uuid + +import pytest + +from apache_beam.io.filesystems import FileSystems +from apache_beam.testing.test_pipeline import TestPipeline + +try: + import torch + from apache_beam.examples.inference import pytorch_image_classification +except ImportError as e: + torch = None + +_EXPECTED_OUTPUTS = { + 'gs://apache-beam-ml/temp_storage_end_to_end_testing/inputs/ILSVRC2012_val_00005001.JPEG': '681', + 'gs://apache-beam-ml/temp_storage_end_to_end_testing/inputs/ILSVRC2012_val_00005002.JPEG': '333', + 'gs://apache-beam-ml/temp_storage_end_to_end_testing/inputs/ILSVRC2012_val_00005003.JPEG': '711', + 'gs://apache-beam-ml/temp_storage_end_to_end_testing/inputs/ILSVRC2012_val_00005004.JPEG': '286', + 'gs://apache-beam-ml/temp_storage_end_to_end_testing/inputs/ILSVRC2012_val_00005005.JPEG': '433', + 'gs://apache-beam-ml/temp_storage_end_to_end_testing/inputs/ILSVRC2012_val_00005006.JPEG': '290', + 'gs://apache-beam-ml/temp_storage_end_to_end_testing/inputs/ILSVRC2012_val_00005007.JPEG': '890', + 'gs://apache-beam-ml/temp_storage_end_to_end_testing/inputs/ILSVRC2012_val_00005008.JPEG': '592', + 'gs://apache-beam-ml/temp_storage_end_to_end_testing/inputs/ILSVRC2012_val_00005009.JPEG': '406', + 'gs://apache-beam-ml/temp_storage_end_to_end_testing/inputs/ILSVRC2012_val_00005010.JPEG': '996', + 'gs://apache-beam-ml/temp_storage_end_to_end_testing/inputs/ILSVRC2012_val_00005011.JPEG': '327', + 'gs://apache-beam-ml/temp_storage_end_to_end_testing/inputs/ILSVRC2012_val_00005012.JPEG': '573' +} + + +def process_outputs(filepath): + with FileSystems().open(filepath) as f: + lines = f.readlines() + lines = [l.decode('utf-8').strip('\n') for l in lines] + return lines + + [email protected]( + torch is None, + 'Missing dependencies. ' + 'Test depends on torch, torchvision and pillow') +class PyTorchInference(unittest.TestCase): + @pytest.mark.uses_pytorch + @pytest.mark.it_postcommit + def test_predictions_output_file(self): + test_pipeline = TestPipeline(is_integration_test=True) + output_file_dir = 'gs://apache-beam-ml/temp_storage_end_to_end_testing/outputs' + output_file = '/'.join([output_file_dir, str(uuid.uuid4()), 'result.txt']) + file_of_image_names = 'gs://apache-beam-ml/temp_storage_end_to_end_testing/inputs/imagenet_samples.csv' Review Comment: nit: temp_storage_end_to_end_testing is not the best name in the path with the input, as the location should be permanent (otherwise the test will stop running if someone deletes the content.) ########## sdks/python/apache_beam/examples/inference/pytorch_image_classification.py: ########## @@ -0,0 +1,122 @@ +# +# 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 argparse +import io +import os +from functools import partial + +import apache_beam as beam +import torch +import torchvision +from apache_beam.io.filesystems import FileSystems +from apache_beam.ml.inference.api import RunInference +from apache_beam.ml.inference.pytorch import PytorchModelLoader +from apache_beam.options.pipeline_options import PipelineOptions +from apache_beam.options.pipeline_options import SetupOptions +from PIL import Image +from torchvision import transforms + + +def read_image(image_file_name: str, path_to_dir: str): + image_file_name = os.path.join(path_to_dir, image_file_name) + with FileSystems().open(image_file_name, 'r') as file: + data = Image.open(io.BytesIO(file.read())).convert('RGB') + return image_file_name, data + + +def preprocess_data(data): + image_size = (224, 224) + normalize = transforms.Normalize( + mean=[0.485, 0.456, 0.406], std=[0.229, 0.224, 0.225]) + transform = transforms.Compose([ + transforms.Resize(image_size), + transforms.ToTensor(), + normalize, + ]) + return transform(data) + + +class PostProcessor(beam.DoFn): + """Post process PredictionResult to output filename and + prediction using torch.""" + def process(self, element): + filename, prediction_result = element + prediction = torch.argmax(prediction_result.inference, dim=0) + yield filename + ',' + str(int(prediction)) Review Comment: nit: argmax sounds like it would return integers or integer arrays. is int() necessary 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
