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 2023/01/10 17:36:40 UTC

[GitHub] [beam] jaxpr opened a new pull request, #24965: XGBoost modelhandler implementation

jaxpr opened a new pull request, #24965:
URL: https://github.com/apache/beam/pull/24965

   This PR contains the implementation of the XGBoost model handler. The model handler supports input data in the form of a Numpy array, Pandas dataframe, Datatable dataframe and a SciPy matrix. The PR also contains tests and an example.
   
   TODOs:
   
   - [ ] If the code is approved, the input and output files for the integration tests should be moved to the proper location on GCS
   
   - [] Write documentation and example for the website once code has been approved
   
   ------------------------
   
   Thank you for your contribution! Follow this checklist to help us incorporate your contribution quickly and easily:
   
    - [ ] 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/get-started-contributing/#make-the-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)
   ------------------------------------------------------------------------------------------------
   [![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)
   [![Go tests](https://github.com/apache/beam/workflows/Go%20tests/badge.svg?branch=master&event=schedule)](https://github.com/apache/beam/actions?query=workflow%3A%22Go+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] damccorm commented on a diff in pull request #24965: XGBoost modelhandler implementation

Posted by GitBox <gi...@apache.org>.
damccorm commented on code in PR #24965:
URL: https://github.com/apache/beam/pull/24965#discussion_r1067143048


##########
sdks/python/apache_beam/ml/inference/xgboost_inference.py:
##########
@@ -0,0 +1,212 @@
+#
+# 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 sys
+from abc import ABC
+from typing import Any
+from typing import Callable
+from typing import Dict
+from typing import Iterable
+from typing import Optional
+from typing import Sequence
+from typing import Union
+
+import datatable
+import numpy
+import pandas
+import scipy
+import xgboost
+
+from apache_beam.ml.inference.base import ExampleT
+from apache_beam.ml.inference.base import ModelHandler
+from apache_beam.ml.inference.base import ModelT
+from apache_beam.ml.inference.base import PredictionResult
+from apache_beam.ml.inference.base import PredictionT
+
+
+class XGBoostModelHandler(ModelHandler[ExampleT, PredictionT, ModelT], ABC):

Review Comment:
   Since users aren't supposed to directly use this class, could we make this `_XGBoostModelHandler`?



##########
sdks/python/apache_beam/ml/inference/xgboost_inference.py:
##########
@@ -0,0 +1,212 @@
+#
+# 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 sys
+from abc import ABC
+from typing import Any
+from typing import Callable
+from typing import Dict
+from typing import Iterable
+from typing import Optional
+from typing import Sequence
+from typing import Union
+
+import datatable
+import numpy
+import pandas
+import scipy
+import xgboost
+
+from apache_beam.ml.inference.base import ExampleT
+from apache_beam.ml.inference.base import ModelHandler
+from apache_beam.ml.inference.base import ModelT
+from apache_beam.ml.inference.base import PredictionResult
+from apache_beam.ml.inference.base import PredictionT
+
+
+class XGBoostModelHandler(ModelHandler[ExampleT, PredictionT, ModelT], ABC):
+  def __init__(
+      self,
+      model_class: Union[Callable[..., xgboost.Booster],
+                         Callable[..., xgboost.XGBModel]],
+      model_state: str):
+    self.model_class = model_class
+    self.model_state = model_state
+
+  def load_model(self) -> Union[xgboost.Booster, xgboost.XGBModel]:
+    model = self.model_class()
+    model.load_model(self.model_state)
+    return model
+
+  def get_metrics_namespace(self) -> str:
+    return 'BeamML_XGBoost'
+
+
+class XGBoostModelHandlerNumpy(XGBoostModelHandler[numpy.ndarray,
+                                                   PredictionResult,
+                                                   Union[xgboost.Booster,
+                                                         xgboost.XGBModel]]):
+  def run_inference(
+      self,
+      batch: Sequence[numpy.ndarray],
+      model: Union[xgboost.Booster, xgboost.XGBModel],
+      inference_args: Optional[Dict[str, Any]] = None) -> Iterable[PredictionT]:
+    """Runs inferences on a batch of 2d numpy arrays.
+
+        Args:
+          batch: A sequence of examples as 2d numpy arrays. Each
+            row in an array is a single example. The dimensions
+            must match the dimensions of the data used to train
+            the model.
+          model: XGBoost booster or XBGModel (sklearn interface). Must
+            implement predict(X). Where the parameter X is a 2d numpy array.
+          inference_args: Any additional arguments for an inference.
+
+        Returns:
+          An Iterable of type PredictionResult.
+        """
+    inference_args = {} if not inference_args else inference_args
+
+    if type(model) == xgboost.Booster:
+      batch = (xgboost.DMatrix(array) for array in batch)
+    predictions = [model.predict(el, **inference_args) for el in batch]

Review Comment:
   Are there cases where we want functions other than `predict` called? I'd imagine yes, if so can we add an option for a custom inference fn following the pattern here - https://github.com/apache/beam/blob/c0e689331c2a6573ecf267b9bef133a85ea8a36c/sdks/python/apache_beam/ml/inference/pytorch_inference.py#L165



##########
sdks/python/apache_beam/ml/inference/xgboost_inference.py:
##########
@@ -0,0 +1,212 @@
+#
+# 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 sys
+from abc import ABC
+from typing import Any
+from typing import Callable
+from typing import Dict
+from typing import Iterable
+from typing import Optional
+from typing import Sequence
+from typing import Union
+
+import datatable
+import numpy
+import pandas
+import scipy
+import xgboost
+
+from apache_beam.ml.inference.base import ExampleT
+from apache_beam.ml.inference.base import ModelHandler
+from apache_beam.ml.inference.base import ModelT
+from apache_beam.ml.inference.base import PredictionResult
+from apache_beam.ml.inference.base import PredictionT
+
+
+class XGBoostModelHandler(ModelHandler[ExampleT, PredictionT, ModelT], ABC):
+  def __init__(
+      self,
+      model_class: Union[Callable[..., xgboost.Booster],
+                         Callable[..., xgboost.XGBModel]],
+      model_state: str):
+    self.model_class = model_class
+    self.model_state = model_state
+
+  def load_model(self) -> Union[xgboost.Booster, xgboost.XGBModel]:
+    model = self.model_class()
+    model.load_model(self.model_state)
+    return model
+
+  def get_metrics_namespace(self) -> str:
+    return 'BeamML_XGBoost'
+
+
+class XGBoostModelHandlerNumpy(XGBoostModelHandler[numpy.ndarray,

Review Comment:
   Could you please add class level comments similar to https://beam.apache.org/releases/pydoc/current/apache_beam.ml.inference.pytorch_inference.html that have expected usage of these classes?



##########
sdks/python/apache_beam/examples/inference/xgboost_iris_classification.py:
##########
@@ -0,0 +1,169 @@
+#

Review Comment:
   This might be covered as part of your TODO, but please add a brief description of this example/usage to https://github.com/apache/beam/blob/0c9999a6faaf7dd64e2abbbe96dac6c68c79d2d5/sdks/python/apache_beam/examples/inference/README.md



##########
sdks/python/apache_beam/ml/inference/xgboost_inference.py:
##########
@@ -0,0 +1,212 @@
+#
+# 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 sys
+from abc import ABC
+from typing import Any
+from typing import Callable
+from typing import Dict
+from typing import Iterable
+from typing import Optional
+from typing import Sequence
+from typing import Union
+
+import datatable
+import numpy
+import pandas
+import scipy
+import xgboost
+
+from apache_beam.ml.inference.base import ExampleT
+from apache_beam.ml.inference.base import ModelHandler
+from apache_beam.ml.inference.base import ModelT
+from apache_beam.ml.inference.base import PredictionResult
+from apache_beam.ml.inference.base import PredictionT
+
+
+class XGBoostModelHandler(ModelHandler[ExampleT, PredictionT, ModelT], ABC):
+  def __init__(
+      self,
+      model_class: Union[Callable[..., xgboost.Booster],
+                         Callable[..., xgboost.XGBModel]],
+      model_state: str):
+    self.model_class = model_class
+    self.model_state = model_state
+
+  def load_model(self) -> Union[xgboost.Booster, xgboost.XGBModel]:
+    model = self.model_class()
+    model.load_model(self.model_state)
+    return model
+
+  def get_metrics_namespace(self) -> str:
+    return 'BeamML_XGBoost'
+
+
+class XGBoostModelHandlerNumpy(XGBoostModelHandler[numpy.ndarray,

Review Comment:
   Generally, this and other comments apply to all the Model Handlers implemented here.



##########
sdks/python/apache_beam/examples/inference/xgboost_iris_classification.py:
##########
@@ -0,0 +1,169 @@
+#
+# 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 logging
+from typing import Callable
+from typing import Iterable
+from typing import List
+from typing import Tuple
+from typing import Union
+
+import datatable
+import numpy
+import pandas
+import scipy
+import xgboost
+from sklearn.datasets import load_iris
+from sklearn.model_selection import train_test_split
+
+import apache_beam as beam
+from apache_beam.ml.inference.base import KeyedModelHandler
+from apache_beam.ml.inference.base import PredictionResult
+from apache_beam.ml.inference.base import RunInference
+from apache_beam.ml.inference.xgboost_inference import XGBoostModelHandlerDatatable
+from apache_beam.ml.inference.xgboost_inference import XGBoostModelHandlerNumpy
+from apache_beam.ml.inference.xgboost_inference import XGBoostModelHandlerPandas
+from apache_beam.ml.inference.xgboost_inference import XGBoostModelHandlerSciPy
+from apache_beam.options.pipeline_options import PipelineOptions
+from apache_beam.options.pipeline_options import SetupOptions
+from apache_beam.runners.runner import PipelineResult
+
+
+def _train_model(model_state_output_path: str = '/tmp/model.json', seed=999):
+  """Function to train an XGBoost Classifier using the sklearn Iris dataset"""
+  dataset = load_iris()
+  x_train, _, y_train, _ = train_test_split(
+      dataset['data'], dataset['target'], test_size=.2, random_state=seed)
+  booster = xgboost.XGBClassifier(
+      n_estimators=2, max_depth=2, learning_rate=1, objective='binary:logistic')
+  booster.fit(x_train, y_train)
+  booster.save_model(model_state_output_path)
+  return booster
+
+
+class PostProcessor(beam.DoFn):
+  """Process the PredictionResult to get the predicted label.
+  Returns a comma separated string with true label and predicted label.
+  """
+  def process(self, element: Tuple[int, PredictionResult]) -> Iterable[str]:
+    label, prediction_result = element
+    prediction = prediction_result.inference
+    yield '{},{}'.format(label, prediction)
+
+
+def parse_known_args(argv):
+  """Parses args for the workflow."""
+  parser = argparse.ArgumentParser()
+  parser.add_argument(
+      '--input-type',
+      dest='input_type',
+      required=True,
+      choices=['numpy', 'pandas', 'scipy', 'datatable'],
+      help=
+      'Datatype of the input data.'
+  )
+  parser.add_argument(
+      '--output',
+      dest='output',
+      required=True,
+      help='Path to save output predictions.')
+  parser.add_argument(
+      '--model-state',
+      dest='model_state',
+      required=True,
+      help='Path to the state of the XGBoost model loaded for Inference.'
+  )
+  group = parser.add_mutually_exclusive_group(required=True)
+  group.add_argument('--split', action='store_true', dest='split')
+  group.add_argument('--no-split', action='store_false', dest='split')
+  return parser.parse_known_args(argv)
+
+
+def load_sklearn_iris_test_data(
+    data_type: Callable,
+    split: bool = True,
+    seed: int = 999) -> List[Union[numpy.array, pandas.DataFrame]]:
+  """
+    Loads test data from the sklearn Iris dataset in a given format,
+    either in a single or multiple batches.
+    Args:
+      data_type: Datatype of the iris test dataset.
+      split: Split the dataset in different batches or return single batch.
+      seed: Random state for splitting the train and test set.
+  """
+  dataset = load_iris()
+  _, x_test, _, _ = train_test_split(
+      dataset['data'], dataset['target'], test_size=.2, random_state=seed)
+
+  if split:
+    return [(index, data_type(sample.reshape(1, -1))) for index,
+            sample in enumerate(x_test)]
+  return [(0, data_type(x_test))]
+
+
+def run(
+    argv=None, save_main_session=True, test_pipeline=None) -> PipelineResult:
+  """
+    Args:
+      argv: Command line arguments defined for this example.
+      save_main_session: Used for internal testing.
+      test_pipeline: Used for internal testing.
+  """
+  known_args, pipeline_args = parse_known_args(argv)
+  pipeline_options = PipelineOptions(pipeline_args)
+  pipeline_options.view_as(SetupOptions).save_main_session = save_main_session
+
+  data_types = {
+      'numpy': (numpy.array, XGBoostModelHandlerNumpy),
+      'pandas': (pandas.DataFrame, XGBoostModelHandlerPandas),
+      'scipy': (scipy.sparse.csr_matrix, XGBoostModelHandlerSciPy),
+      'datatable': (datatable.Frame, XGBoostModelHandlerDatatable),
+  }
+
+  input_data_type, model_handler = data_types[known_args.input_type]
+
+  xgboost_model_handler = KeyedModelHandler(
+      model_handler(
+          model_class=xgboost.XGBClassifier,
+          model_state=known_args.model_state))
+
+  input_data = load_sklearn_iris_test_data(
+      data_type=input_data_type, split=known_args.split)
+
+  pipeline = test_pipeline
+  if not test_pipeline:
+    pipeline = beam.Pipeline(options=pipeline_options)
+
+  predictions = (
+      pipeline
+      | "ReadInputData" >> beam.Create(input_data)
+      | "RunInference" >> RunInference(xgboost_model_handler)
+      | "PostProcessOutputs" >> beam.ParDo(PostProcessor()))
+
+  _ = predictions | "WriteOutput" >> beam.io.WriteToText(
+      known_args.output, shard_name_template='', append_trailing_newlines=True)
+
+  result = pipeline.run()
+  result.wait_until_finish()
+  return result
+
+
+if __name__ == '__main__':
+  logging.getLogger().setLevel(logging.INFO)
+  _train_model()

Review Comment:
   Could we move this to `run()` and parameterize the path that's getting saved to? Then in tests we can train the model in a temporary directory to avoid conflicts



##########
sdks/python/container/py310/base_image_requirements.txt:
##########
@@ -154,4 +155,5 @@ urllib3==1.26.13
 websocket-client==1.4.2
 Werkzeug==2.2.2
 wrapt==1.14.1
+xgboost==1.7.1

Review Comment:
   Instead of manually updating this file (and the other similar ones), please update https://github.com/apache/beam/blob/0c9999a6faaf7dd64e2abbbe96dac6c68c79d2d5/sdks/python/container/base_image_requirements_manual.txt and run `./gradlew :sdks:python:container:generatePythonRequirementsAll` - see https://github.com/apache/beam/blob/0c9999a6faaf7dd64e2abbbe96dac6c68c79d2d5/sdks/python/container/py310/base_image_requirements.txt#LL17C7-L17C69



##########
sdks/python/apache_beam/ml/inference/xgboost_inference.py:
##########
@@ -0,0 +1,212 @@
+#
+# 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 sys
+from abc import ABC
+from typing import Any
+from typing import Callable
+from typing import Dict
+from typing import Iterable
+from typing import Optional
+from typing import Sequence
+from typing import Union
+
+import datatable
+import numpy
+import pandas
+import scipy
+import xgboost
+
+from apache_beam.ml.inference.base import ExampleT
+from apache_beam.ml.inference.base import ModelHandler
+from apache_beam.ml.inference.base import ModelT
+from apache_beam.ml.inference.base import PredictionResult
+from apache_beam.ml.inference.base import PredictionT
+
+
+class XGBoostModelHandler(ModelHandler[ExampleT, PredictionT, ModelT], ABC):
+  def __init__(
+      self,
+      model_class: Union[Callable[..., xgboost.Booster],
+                         Callable[..., xgboost.XGBModel]],
+      model_state: str):
+    self.model_class = model_class
+    self.model_state = model_state
+
+  def load_model(self) -> Union[xgboost.Booster, xgboost.XGBModel]:
+    model = self.model_class()
+    model.load_model(self.model_state)

Review Comment:
   Could we extend this so that it can accept arbitrary Beam Filesystem locations (e.g. gcs) so that it can easily be used on remote runners? You should be able to do that by using the Filesystem IO utilities - https://github.com/apache/beam/blob/c0e689331c2a6573ecf267b9bef133a85ea8a36c/sdks/python/apache_beam/io/filesystem.py#L810 - if there's not a good way to load the model via file stream, its ok to save the model locally before loading



##########
sdks/python/apache_beam/examples/inference/xgboost_iris_classification.py:
##########
@@ -0,0 +1,169 @@
+#

Review Comment:
   Note - I'd ask that to be part of this PR, the remaining documentation would ideally come in a separate PR though, that way we can merge this before the cut and publish the documentation once the release has been published.



##########
sdks/python/apache_beam/ml/inference/xgboost_inference.py:
##########
@@ -0,0 +1,212 @@
+#
+# 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 sys
+from abc import ABC
+from typing import Any
+from typing import Callable
+from typing import Dict
+from typing import Iterable
+from typing import Optional
+from typing import Sequence
+from typing import Union
+
+import datatable
+import numpy
+import pandas
+import scipy
+import xgboost
+
+from apache_beam.ml.inference.base import ExampleT
+from apache_beam.ml.inference.base import ModelHandler
+from apache_beam.ml.inference.base import ModelT
+from apache_beam.ml.inference.base import PredictionResult
+from apache_beam.ml.inference.base import PredictionT
+
+
+class XGBoostModelHandler(ModelHandler[ExampleT, PredictionT, ModelT], ABC):
+  def __init__(
+      self,
+      model_class: Union[Callable[..., xgboost.Booster],
+                         Callable[..., xgboost.XGBModel]],
+      model_state: str):
+    self.model_class = model_class
+    self.model_state = model_state
+
+  def load_model(self) -> Union[xgboost.Booster, xgboost.XGBModel]:
+    model = self.model_class()
+    model.load_model(self.model_state)
+    return model
+
+  def get_metrics_namespace(self) -> str:
+    return 'BeamML_XGBoost'
+
+
+class XGBoostModelHandlerNumpy(XGBoostModelHandler[numpy.ndarray,

Review Comment:
   Discussing versioning in this comment would be helpful as well - IIRC we're requiring a minimum XGBoost version to use this, right?



-- 
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] jaxpr commented on pull request #24965: XGBoost modelhandler implementation

Posted by "jaxpr (via GitHub)" <gi...@apache.org>.
jaxpr commented on PR #24965:
URL: https://github.com/apache/beam/pull/24965#issuecomment-1457870281

   @damccorm all checks passed except for codecov, I don't know if how to fix that one.


-- 
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] damccorm merged pull request #24965: XGBoost modelhandler implementation

Posted by "damccorm (via GitHub)" <gi...@apache.org>.
damccorm merged PR #24965:
URL: https://github.com/apache/beam/pull/24965


-- 
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] jaxpr commented on pull request #24965: XGBoost modelhandler implementation

Posted by "jaxpr (via GitHub)" <gi...@apache.org>.
jaxpr commented on PR #24965:
URL: https://github.com/apache/beam/pull/24965#issuecomment-1425545240

   @damccorm I think the pipelines succeeded


-- 
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] damccorm commented on pull request #24965: XGBoost modelhandler implementation

Posted by "damccorm (via GitHub)" <gi...@apache.org>.
damccorm commented on PR #24965:
URL: https://github.com/apache/beam/pull/24965#issuecomment-1448677086

   Run Python_Coverage 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] damccorm commented on pull request #24965: XGBoost modelhandler implementation

Posted by "damccorm (via GitHub)" <gi...@apache.org>.
damccorm commented on PR #24965:
URL: https://github.com/apache/beam/pull/24965#issuecomment-1458248070

   > @damccorm all checks passed except for codecov, I don't know if how to fix that one.
   
   It is fine to ignore that, it doesn't count most of our inference tests since they're postcommits


-- 
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] damccorm commented on pull request #24965: XGBoost modelhandler implementation

Posted by "damccorm (via GitHub)" <gi...@apache.org>.
damccorm commented on PR #24965:
URL: https://github.com/apache/beam/pull/24965#issuecomment-1424693480

   I think the last pieces that are missing for precommits are:
   1) Docs precommit - Add xg_boost (and maybe datatable?) here - https://github.com/apache/beam/blob/64e40d2c018f8e906f4bec32ef67f02734a95721/sdks/python/tox.ini#L153
   2) From the root of the repo, run `./gradlew :sdks:python:test-suites:tox:py37:mypyPy37` and fix any breakages
   3) From the root of the `sdks/python` folder, run `tox -e py3-yapf`
   
   Those should fix the remaining precommit failures


-- 
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 #24965: XGBoost modelhandler implementation

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

   # [Codecov](https://codecov.io/gh/apache/beam/pull/24965?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 [#24965](https://codecov.io/gh/apache/beam/pull/24965?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (0c9999a) into [master](https://codecov.io/gh/apache/beam/commit/3838528fde37262ecadf642791e3e0f3a57d7b25?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (3838528) will **decrease** coverage by `16.22%`.
   > The diff coverage is `0.00%`.
   
   ```diff
   @@             Coverage Diff             @@
   ##           master   #24965       +/-   ##
   ===========================================
   - Coverage   73.13%   56.90%   -16.23%     
   ===========================================
     Files         735      737        +2     
     Lines       98146    98283      +137     
   ===========================================
   - Hits        71777    55928    -15849     
   - Misses      25006    40992    +15986     
     Partials     1363     1363               
   ```
   
   | Flag | Coverage Δ | |
   |---|---|---|
   | python | `59.17% <0.00%> (-23.52%)` | :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/24965?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/xgboost\_iris\_classification.py](https://codecov.io/gh/apache/beam/pull/24965?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vZXhhbXBsZXMvaW5mZXJlbmNlL3hnYm9vc3RfaXJpc19jbGFzc2lmaWNhdGlvbi5weQ==) | `0.00% <0.00%> (ø)` | |
   | [...thon/apache\_beam/ml/inference/xgboost\_inference.py](https://codecov.io/gh/apache/beam/pull/24965?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vbWwvaW5mZXJlbmNlL3hnYm9vc3RfaW5mZXJlbmNlLnB5) | `0.00% <0.00%> (ø)` | |
   | [sdks/python/apache\_beam/io/utils.py](https://codecov.io/gh/apache/beam/pull/24965?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vaW8vdXRpbHMucHk=) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
   | [sdks/python/apache\_beam/ml/\_\_init\_\_.py](https://codecov.io/gh/apache/beam/pull/24965?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vbWwvX19pbml0X18ucHk=) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
   | [sdks/python/apache\_beam/utils/shared.py](https://codecov.io/gh/apache/beam/pull/24965?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vdXRpbHMvc2hhcmVkLnB5) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
   | [sdks/python/apache\_beam/ml/inference/\_\_init\_\_.py](https://codecov.io/gh/apache/beam/pull/24965?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vbWwvaW5mZXJlbmNlL19faW5pdF9fLnB5) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
   | [sdks/python/apache\_beam/examples/snippets/util.py](https://codecov.io/gh/apache/beam/pull/24965?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vZXhhbXBsZXMvc25pcHBldHMvdXRpbC5weQ==) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
   | [...s/python/apache\_beam/typehints/testing/\_\_init\_\_.py](https://codecov.io/gh/apache/beam/pull/24965?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vdHlwZWhpbnRzL3Rlc3RpbmcvX19pbml0X18ucHk=) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
   | [.../python/apache\_beam/io/gcp/datastore/v1new/util.py](https://codecov.io/gh/apache/beam/pull/24965?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vaW8vZ2NwL2RhdGFzdG9yZS92MW5ldy91dGlsLnB5) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
   | [.../python/apache\_beam/testing/analyzers/constants.py](https://codecov.io/gh/apache/beam/pull/24965?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vdGVzdGluZy9hbmFseXplcnMvY29uc3RhbnRzLnB5) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
   | ... and [250 more](https://codecov.io/gh/apache/beam/pull/24965?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
   
   :mega: We’re building smart automated test selection to slash your CI/CD build times. [Learn more](https://about.codecov.io/iterative-testing/?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] jaxpr commented on a diff in pull request #24965: XGBoost modelhandler implementation

Posted by "jaxpr (via GitHub)" <gi...@apache.org>.
jaxpr commented on code in PR #24965:
URL: https://github.com/apache/beam/pull/24965#discussion_r1085502764


##########
sdks/python/apache_beam/ml/inference/xgboost_inference.py:
##########
@@ -0,0 +1,212 @@
+#
+# 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 sys
+from abc import ABC
+from typing import Any
+from typing import Callable
+from typing import Dict
+from typing import Iterable
+from typing import Optional
+from typing import Sequence
+from typing import Union
+
+import datatable
+import numpy
+import pandas
+import scipy
+import xgboost
+
+from apache_beam.ml.inference.base import ExampleT
+from apache_beam.ml.inference.base import ModelHandler
+from apache_beam.ml.inference.base import ModelT
+from apache_beam.ml.inference.base import PredictionResult
+from apache_beam.ml.inference.base import PredictionT
+
+
+class XGBoostModelHandler(ModelHandler[ExampleT, PredictionT, ModelT], ABC):

Review Comment:
   I have added the comment saying the class should not be instantiated directly as @AnandInguva suggested.



##########
sdks/python/apache_beam/ml/inference/xgboost_inference.py:
##########
@@ -0,0 +1,212 @@
+#
+# 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 sys
+from abc import ABC
+from typing import Any
+from typing import Callable
+from typing import Dict
+from typing import Iterable
+from typing import Optional
+from typing import Sequence
+from typing import Union
+
+import datatable
+import numpy
+import pandas
+import scipy
+import xgboost
+
+from apache_beam.ml.inference.base import ExampleT
+from apache_beam.ml.inference.base import ModelHandler
+from apache_beam.ml.inference.base import ModelT
+from apache_beam.ml.inference.base import PredictionResult
+from apache_beam.ml.inference.base import PredictionT
+
+
+class XGBoostModelHandler(ModelHandler[ExampleT, PredictionT, ModelT], ABC):
+  def __init__(
+      self,
+      model_class: Union[Callable[..., xgboost.Booster],
+                         Callable[..., xgboost.XGBModel]],
+      model_state: str):
+    self.model_class = model_class
+    self.model_state = model_state
+
+  def load_model(self) -> Union[xgboost.Booster, xgboost.XGBModel]:
+    model = self.model_class()
+    model.load_model(self.model_state)
+    return model
+
+  def get_metrics_namespace(self) -> str:
+    return 'BeamML_XGBoost'
+
+
+class XGBoostModelHandlerNumpy(XGBoostModelHandler[numpy.ndarray,
+                                                   PredictionResult,
+                                                   Union[xgboost.Booster,
+                                                         xgboost.XGBModel]]):
+  def run_inference(
+      self,
+      batch: Sequence[numpy.ndarray],
+      model: Union[xgboost.Booster, xgboost.XGBModel],
+      inference_args: Optional[Dict[str, Any]] = None) -> Iterable[PredictionT]:
+    """Runs inferences on a batch of 2d numpy arrays.
+
+        Args:
+          batch: A sequence of examples as 2d numpy arrays. Each
+            row in an array is a single example. The dimensions
+            must match the dimensions of the data used to train
+            the model.
+          model: XGBoost booster or XBGModel (sklearn interface). Must
+            implement predict(X). Where the parameter X is a 2d numpy array.
+          inference_args: Any additional arguments for an inference.
+
+        Returns:
+          An Iterable of type PredictionResult.
+        """
+    inference_args = {} if not inference_args else inference_args
+
+    if type(model) == xgboost.Booster:
+      batch = (xgboost.DMatrix(array) for array in batch)
+    predictions = [model.predict(el, **inference_args) for el in batch]

Review Comment:
   Good suggestion indeed as this seems possible using the XGBoost scipy api. I added a custom inference fn.



##########
sdks/python/container/py310/base_image_requirements.txt:
##########
@@ -154,4 +155,5 @@ urllib3==1.26.13
 websocket-client==1.4.2
 Werkzeug==2.2.2
 wrapt==1.14.1
+xgboost==1.7.1

Review Comment:
   I followed the suggestion by @AnandInguva as well over 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] AnandInguva commented on a diff in pull request #24965: XGBoost modelhandler implementation

Posted by "AnandInguva (via GitHub)" <gi...@apache.org>.
AnandInguva commented on code in PR #24965:
URL: https://github.com/apache/beam/pull/24965#discussion_r1096930767


##########
sdks/python/apache_beam/ml/inference/xgboost_inference.py:
##########
@@ -0,0 +1,370 @@
+#
+# 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 sys
+from abc import ABC
+from typing import Any
+from typing import Callable
+from typing import Dict
+from typing import Iterable
+from typing import Optional
+from typing import Sequence
+from typing import Union
+
+import datatable
+import numpy
+import pandas
+import scipy
+import xgboost
+
+from apache_beam.io.filesystems import FileSystems
+from apache_beam.ml.inference.base import ExampleT
+from apache_beam.ml.inference.base import ModelHandler
+from apache_beam.ml.inference.base import ModelT
+from apache_beam.ml.inference.base import PredictionResult
+from apache_beam.ml.inference.base import PredictionT
+
+__all__ = [
+    'XGBoostModelHandler',
+    'XGBoostModelHandlerNumpy',
+    'XGBoostModelHandlerPandas',
+    'XGBoostModelHandlerSciPy',
+    'XGBoostModelHandlerDatatable'
+]
+
+XGBoostInferenceFn = Callable[[
+    Sequence[object],
+    Union[xgboost.Booster, xgboost.XGBModel],
+    Optional[Dict[str, Any]]
+],
+                              Iterable[PredictionResult]]
+
+
+def default_xgboost_inference_fn(
+    batch: Sequence[object],
+    model: Union[xgboost.Booster, xgboost.XGBModel],
+    inference_args: Optional[Dict[str, Any]] = None):
+  inference_args = {} if not inference_args else inference_args
+
+  if type(model) == xgboost.Booster:
+    batch = (xgboost.DMatrix(array) for array in batch)
+  predictions = [model.predict(el, **inference_args) for el in batch]
+
+  return [PredictionResult(x, y) for x, y in zip(batch, predictions)]
+
+
+class XGBoostModelHandler(ModelHandler[ExampleT, PredictionT, ModelT], ABC):
+
+  def __init__(
+      self,
+      model_class: Union[Callable[..., xgboost.Booster],
+                         Callable[..., xgboost.XGBModel]],
+      model_state: str,
+      inference_fn: XGBoostInferenceFn = default_xgboost_inference_fn):
+    """Implementation of the ModelHandler interface for XGBoost.
+
+    Example Usage::
+
+        pcoll | RunInference(
+                    XGBoostModelHandler(
+                        model_class="XGBoost Model Class",
+                        model_state="my_model_state.json")))
+
+    See https://xgboost.readthedocs.io/en/stable/tutorials/saving_model.html
+    for details
+
+    Args:
+    model_class: class of the XGBoost model that defines the model
+      structure.
+    model_state: path to a json file that contains the model's
+      configuration.
+    inference_fn: the inference function to use during RunInference.
+      default=default_xgboost_inference_fn
+
+    **Supported Versions:** RunInference APIs in Apache Beam have been tested
+    with PyTorch 1.6.0 and 1.7.0

Review Comment:
   ```suggestion
       with XGBoost 1.6.0 and 1.7.0
   ```
   



-- 
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] damccorm commented on pull request #24965: XGBoost modelhandler implementation

Posted by "damccorm (via GitHub)" <gi...@apache.org>.
damccorm commented on PR #24965:
URL: https://github.com/apache/beam/pull/24965#issuecomment-1448677779

   Run Python_PVR_Flink 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] damccorm commented on pull request #24965: XGBoost modelhandler implementation

Posted by "damccorm (via GitHub)" <gi...@apache.org>.
damccorm commented on PR #24965:
URL: https://github.com/apache/beam/pull/24965#issuecomment-1458248473

   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] damccorm commented on pull request #24965: XGBoost modelhandler implementation

Posted by "damccorm (via GitHub)" <gi...@apache.org>.
damccorm commented on PR #24965:
URL: https://github.com/apache/beam/pull/24965#issuecomment-1464178879

   Looks like a Jenkins flake caused the postcommit failure, but tests are passing. Running again to make sure, but I think this should be ready to go in.


-- 
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] damccorm commented on a diff in pull request #24965: XGBoost modelhandler implementation

Posted by "damccorm (via GitHub)" <gi...@apache.org>.
damccorm commented on code in PR #24965:
URL: https://github.com/apache/beam/pull/24965#discussion_r1091071810


##########
sdks/python/apache_beam/examples/inference/README.md:
##########
@@ -374,3 +374,59 @@ True Price 31000000.0, Predicted Price 25654277.256461
 ...
 ```
 
+## Iris Classification
+
+[`xgboost_iris_classification.py.py`](./xgboost_iris_classification.py.py) contains an implementation for a RunInference pipeline that performs classification on tabular data from the [Iris Dataset](https://scikit-learn.org/stable/auto_examples/datasets/plot_iris_dataset.html).
+
+The pipeline reads rows that contain the features of a given iris. The features are Sepal Length, Sepal Width, Petal Length and Petal Width. The pipeline passes those features to the XGBoost implementation of RunInference which writes the iris type predictions to a text file.
+
+### Dataset and model for language modeling
+
+To use this transform, you need to have sklearn installed. The dataset is loaded from using sklearn. The `_train_model` function can be used to train a simple classifier. The function outputs it's configuration in a file that can be loaded by the `XGBoostModelHandler`.
+
+### Training a simple classifier
+
+The following function you to train a simple classifier using the sklearn Iris dataset. The trained model will be saved in the location passed as a parameter and can then later be loaded in an pipeline using the `XGBoostModelHandler`.
+
+```
+def _train_model(model_state_output_path: str = '/tmp/model.json', seed=999):
+  """Function to train an XGBoost Classifier using the sklearn Iris dataset"""
+  dataset = load_iris()
+  x_train, _, y_train, _ = train_test_split(
+      dataset['data'], dataset['target'], test_size=.2, random_state=seed)
+  booster = xgboost.XGBClassifier(
+      n_estimators=2, max_depth=2, learning_rate=1, objective='binary:logistic')
+  booster.fit(x_train, y_train)
+  booster.save_model(model_state_output_path)
+  return booster
+```
+
+#### Running the Pipeline
+To run locally, use the following command:
+
+```
+python -m apache_beam.examples.inference.xgboost_iris_classification.py \
+  --input_type INPUT_TYPE \
+  --output OUTPUT_FILE \
+  -- model_state MODEL_STATE_JSON \
+  [--no_split|--split]
+```
+
+For example:
+
+```
+python -m apache_beam.examples.inference.xgboost_iris_classification.py \
+  --input_type numpy \
+  --output predictions.txt \
+  --model_state model_state.json \
+  --split
+```
+
+This writes the output to the `predictions.txt` with contents like:
+```
+0,[1]
+1,[2]
+2,[1]
+3,[0]
+...
+```

Review Comment:
   Could you please add a quick description of what these predictions map to? (I think they are row number + iris class?)



##########
sdks/python/apache_beam/examples/inference/README.md:
##########
@@ -374,3 +374,59 @@ True Price 31000000.0, Predicted Price 25654277.256461
 ...
 ```
 
+## Iris Classification
+
+[`xgboost_iris_classification.py.py`](./xgboost_iris_classification.py.py) contains an implementation for a RunInference pipeline that performs classification on tabular data from the [Iris Dataset](https://scikit-learn.org/stable/auto_examples/datasets/plot_iris_dataset.html).
+
+The pipeline reads rows that contain the features of a given iris. The features are Sepal Length, Sepal Width, Petal Length and Petal Width. The pipeline passes those features to the XGBoost implementation of RunInference which writes the iris type predictions to a text file.
+
+### Dataset and model for language modeling
+
+To use this transform, you need to have sklearn installed. The dataset is loaded from using sklearn. The `_train_model` function can be used to train a simple classifier. The function outputs it's configuration in a file that can be loaded by the `XGBoostModelHandler`.
+
+### Training a simple classifier
+
+The following function you to train a simple classifier using the sklearn Iris dataset. The trained model will be saved in the location passed as a parameter and can then later be loaded in an pipeline using the `XGBoostModelHandler`.

Review Comment:
   ```suggestion
   The following function allows you to train a simple classifier using the sklearn Iris dataset. The trained model will be saved in the location passed as a parameter and can then later be loaded in an pipeline using the `XGBoostModelHandler`.
   ```



##########
sdks/python/tox.ini:
##########
@@ -326,3 +326,16 @@ commands =
   # Run all PyTorch unit tests
   # Allow exit code 5 (no tests run) so that we can run this command safely on arbitrary subdirectories.
   /bin/sh -c 'pytest -o junit_suite_name={envname} --junitxml=pytest_{envname}.xml -n 6 -m uses_pytorch {posargs}; ret=$?; [ $ret = 5 ] && exit 0 || exit $ret'
+
+[testenv:py{37,38,39,310}-xgboost-{160,170}]
+deps =
+  -r build-requirements.txt
+  160: torch>=1.6.0,<1.7.0
+  170: torch>=1.7.0

Review Comment:
   Instead of torch, this should be installing xgboost, right? I think we also need to be installing datatable here, right?



-- 
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] damccorm commented on a diff in pull request #24965: XGBoost modelhandler implementation

Posted by "damccorm (via GitHub)" <gi...@apache.org>.
damccorm commented on code in PR #24965:
URL: https://github.com/apache/beam/pull/24965#discussion_r1097498121


##########
sdks/python/tox.ini:
##########
@@ -326,3 +326,16 @@ commands =
   # Run all PyTorch unit tests
   # Allow exit code 5 (no tests run) so that we can run this command safely on arbitrary subdirectories.
   /bin/sh -c 'pytest -o junit_suite_name={envname} --junitxml=pytest_{envname}.xml -n 6 -m uses_pytorch {posargs}; ret=$?; [ $ret = 5 ] && exit 0 || exit $ret'
+
+[testenv:py{37,38,39,310}-xgboost-{160,170}]
+deps =
+  -r build-requirements.txt
+  160: torch>=1.6.0,<1.7.0
+  170: torch>=1.7.0

Review Comment:
   Ah, we also need to handle import exceptions like this - https://github.com/apache/beam/blob/aee2c844ad8f583dc161219f4ea0988d5a8860e9/sdks/python/apache_beam/ml/inference/pytorch_inference_it_test.py#L36 - and skip if datatable/xgboost aren't installed like this - https://github.com/apache/beam/blob/aee2c844ad8f583dc161219f4ea0988d5a8860e9/sdks/python/apache_beam/ml/inference/pytorch_inference_it_test.py#L63
   
   Also, please add uses_xgboost to https://github.com/apache/beam/blob/aee2c844ad8f583dc161219f4ea0988d5a8860e9/sdks/python/pytest.ini#L29



-- 
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] damccorm commented on a diff in pull request #24965: XGBoost modelhandler implementation

Posted by "damccorm (via GitHub)" <gi...@apache.org>.
damccorm commented on code in PR #24965:
URL: https://github.com/apache/beam/pull/24965#discussion_r1129514487


##########
sdks/python/apache_beam/ml/inference/xgboost_inference.py:
##########
@@ -0,0 +1,362 @@
+#
+# 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 sys
+from abc import ABC
+from typing import Any
+from typing import Callable
+from typing import Dict
+from typing import Iterable
+from typing import Optional
+from typing import Sequence
+from typing import Union
+
+import numpy
+import pandas
+import scipy
+
+import datatable
+import xgboost
+from apache_beam.io.filesystems import FileSystems
+from apache_beam.ml.inference.base import ExampleT
+from apache_beam.ml.inference.base import ModelHandler
+from apache_beam.ml.inference.base import ModelT
+from apache_beam.ml.inference.base import PredictionResult
+from apache_beam.ml.inference.base import PredictionT
+
+__all__ = [
+    'XGBoostModelHandler',
+    'XGBoostModelHandlerNumpy',
+    'XGBoostModelHandlerPandas',
+    'XGBoostModelHandlerSciPy',
+    'XGBoostModelHandlerDatatable'
+]
+
+XGBoostInferenceFn = Callable[[
+    Sequence[object],
+    Union[xgboost.Booster, xgboost.XGBModel],
+    Optional[Dict[str, Any]]
+],
+                              Iterable[PredictionResult]]
+
+
+def default_xgboost_inference_fn(
+    batch: Sequence[object],
+    model: Union[xgboost.Booster, xgboost.XGBModel],
+    inference_args: Optional[Dict[str,
+                                  Any]] = None) -> Iterable[PredictionResult]:
+  inference_args = {} if not inference_args else inference_args
+
+  if type(model) == xgboost.Booster:
+    batch = [xgboost.DMatrix(array) for array in batch]
+  predictions = [model.predict(el, **inference_args) for el in batch]
+
+  return [PredictionResult(x, y) for x, y in zip(batch, predictions)]
+
+
+class XGBoostModelHandler(ModelHandler[ExampleT, PredictionT, ModelT], ABC):
+  def __init__(
+      self,
+      model_class: Union[Callable[..., xgboost.Booster],
+                         Callable[..., xgboost.XGBModel]],
+      model_state: str,
+      inference_fn: XGBoostInferenceFn = default_xgboost_inference_fn):
+    """Implementation of the ModelHandler interface for XGBoost.
+
+    Example Usage::
+
+        pcoll | RunInference(
+                    XGBoostModelHandler(
+                        model_class="XGBoost Model Class",
+                        model_state="my_model_state.json")))
+
+    See https://xgboost.readthedocs.io/en/stable/tutorials/saving_model.html
+    for details
+
+    Args:
+      model_class: class of the XGBoost model that defines the model
+        structure.
+      model_state: path to a json file that contains the model's
+        configuration.
+      inference_fn: the inference function to use during RunInference.
+        default=default_xgboost_inference_fn
+
+    **Supported Versions:** RunInference APIs in Apache Beam have been tested
+    with XGBoost 1.6.0 and 1.7.0
+
+    XGBoost 1.0.0 introduced support for using JSON to save and load
+    XGBoost models. XGBoost 1.6.0, additional support for Universal Binary JSON.
+    It is recommended to use a model trained in XGBoost 1.6.0 or higher.
+    While you should be able to load models created in older versions, there
+    are no guarantees this will work as expected.
+
+    This class is the superclass of all the various XGBoostModelhandlers
+    and should not be instantiated directly. (See instead
+    XGBoostModelHandlerNumpy, XGBoostModelHandlerPandas, etc.)
+    """
+    self._model_class = model_class
+    self._model_state = model_state
+    self._inference_fn = inference_fn
+
+  def load_model(self) -> Union[xgboost.Booster, xgboost.XGBModel]:
+    model = self._model_class()
+    model_state_file_handler = FileSystems.open(self._model_state, 'rb')
+    model_state_bytes = model_state_file_handler.read()
+    # Convert into a bytearray so that the
+    # model state can be loaded in XGBoost
+    model_state_bytearray = bytearray(model_state_bytes)
+    model.load_model(model_state_bytearray)
+    return model
+
+  def get_metrics_namespace(self) -> str:
+    return 'BeamML_XGBoost'
+
+
+class XGBoostModelHandlerNumpy(XGBoostModelHandler[numpy.ndarray,
+                                                   PredictionResult,
+                                                   Union[xgboost.Booster,
+                                                         xgboost.XGBModel]]):
+  def __init__(
+      self,
+      model_class: Union[Callable[..., xgboost.Booster],
+                         Callable[..., xgboost.XGBModel]],
+      model_state: str,
+      inference_fn: XGBoostInferenceFn = default_xgboost_inference_fn):
+    """ Implementation of the ModelHandler interface for XGBoost
+    using numpy arrays as input.
+
+    Example Usage::
+
+        pcoll | RunInference(
+                    XGBoostModelHandlerNumpy(
+                        model_class="XGBoost Model Class",
+                        model_state="my_model_state.json")))
+
+    Args:
+      model_class: class of the XGBoost model that defines the model
+        structure.
+      model_state: path to a json file that contains the model's
+        configuration.
+      inference_fn: the inference function to use during RunInference.
+        default=default_xgboost_inference_fn
+    """

Review Comment:
   I think removing the super.__init__() call broke the tests (and the functionality). Tests are now failing with errors like: `AttributeError: 'XGBoostModelHandlerSciPy' object has no attribute '_model_class'` (https://ci-beam.apache.org/job/beam_PostCommit_Python37_PR/520/consoleFull#gradle-task-435).
   
   Instead of having the comment for the pydoc on the init function, could we just have it on the class itself (like https://github.com/apache/beam/blob/5d1bcfbae18272e54574fe718428d2f7a6cfff50/sdks/python/apache_beam/transforms/core.py#L531)? Then we should be able to omit the __init__ function entirely.



-- 
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 #24965: XGBoost modelhandler implementation

Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on PR #24965:
URL: https://github.com/apache/beam/pull/24965#issuecomment-1396973794

   Stopping reviewer notifications for this pull request: requested by reviewer


-- 
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] jaxpr commented on a diff in pull request #24965: XGBoost modelhandler implementation

Posted by "jaxpr (via GitHub)" <gi...@apache.org>.
jaxpr commented on code in PR #24965:
URL: https://github.com/apache/beam/pull/24965#discussion_r1085503200


##########
sdks/python/apache_beam/ml/inference/xgboost_inference.py:
##########
@@ -0,0 +1,212 @@
+#
+# 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 sys
+from abc import ABC
+from typing import Any
+from typing import Callable
+from typing import Dict
+from typing import Iterable
+from typing import Optional
+from typing import Sequence
+from typing import Union
+
+import datatable
+import numpy
+import pandas
+import scipy
+import xgboost
+
+from apache_beam.ml.inference.base import ExampleT
+from apache_beam.ml.inference.base import ModelHandler
+from apache_beam.ml.inference.base import ModelT
+from apache_beam.ml.inference.base import PredictionResult
+from apache_beam.ml.inference.base import PredictionT
+
+
+class XGBoostModelHandler(ModelHandler[ExampleT, PredictionT, ModelT], ABC):
+  def __init__(
+      self,
+      model_class: Union[Callable[..., xgboost.Booster],
+                         Callable[..., xgboost.XGBModel]],
+      model_state: str):
+    self.model_class = model_class
+    self.model_state = model_state
+
+  def load_model(self) -> Union[xgboost.Booster, xgboost.XGBModel]:
+    model = self.model_class()
+    model.load_model(self.model_state)

Review Comment:
   I used the Apache Beam `open` method from the FileSystem modude. After reading the file's data from the filehandler, I converted it into a Python bytestream that can be loaded by XGBoost.



##########
sdks/python/apache_beam/examples/inference/xgboost_iris_classification.py:
##########
@@ -0,0 +1,169 @@
+#
+# 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 logging
+from typing import Callable
+from typing import Iterable
+from typing import List
+from typing import Tuple
+from typing import Union
+
+import datatable
+import numpy
+import pandas
+import scipy
+import xgboost
+from sklearn.datasets import load_iris
+from sklearn.model_selection import train_test_split
+
+import apache_beam as beam
+from apache_beam.ml.inference.base import KeyedModelHandler
+from apache_beam.ml.inference.base import PredictionResult
+from apache_beam.ml.inference.base import RunInference
+from apache_beam.ml.inference.xgboost_inference import XGBoostModelHandlerDatatable
+from apache_beam.ml.inference.xgboost_inference import XGBoostModelHandlerNumpy
+from apache_beam.ml.inference.xgboost_inference import XGBoostModelHandlerPandas
+from apache_beam.ml.inference.xgboost_inference import XGBoostModelHandlerSciPy
+from apache_beam.options.pipeline_options import PipelineOptions
+from apache_beam.options.pipeline_options import SetupOptions
+from apache_beam.runners.runner import PipelineResult
+
+
+def _train_model(model_state_output_path: str = '/tmp/model.json', seed=999):
+  """Function to train an XGBoost Classifier using the sklearn Iris dataset"""
+  dataset = load_iris()
+  x_train, _, y_train, _ = train_test_split(
+      dataset['data'], dataset['target'], test_size=.2, random_state=seed)
+  booster = xgboost.XGBClassifier(
+      n_estimators=2, max_depth=2, learning_rate=1, objective='binary:logistic')
+  booster.fit(x_train, y_train)
+  booster.save_model(model_state_output_path)
+  return booster
+
+
+class PostProcessor(beam.DoFn):
+  """Process the PredictionResult to get the predicted label.
+  Returns a comma separated string with true label and predicted label.
+  """
+  def process(self, element: Tuple[int, PredictionResult]) -> Iterable[str]:
+    label, prediction_result = element
+    prediction = prediction_result.inference
+    yield '{},{}'.format(label, prediction)
+
+
+def parse_known_args(argv):
+  """Parses args for the workflow."""
+  parser = argparse.ArgumentParser()
+  parser.add_argument(
+      '--input-type',
+      dest='input_type',
+      required=True,
+      choices=['numpy', 'pandas', 'scipy', 'datatable'],
+      help=
+      'Datatype of the input data.'
+  )
+  parser.add_argument(
+      '--output',
+      dest='output',
+      required=True,
+      help='Path to save output predictions.')
+  parser.add_argument(
+      '--model-state',
+      dest='model_state',
+      required=True,
+      help='Path to the state of the XGBoost model loaded for Inference.'
+  )
+  group = parser.add_mutually_exclusive_group(required=True)
+  group.add_argument('--split', action='store_true', dest='split')
+  group.add_argument('--no-split', action='store_false', dest='split')
+  return parser.parse_known_args(argv)
+
+
+def load_sklearn_iris_test_data(
+    data_type: Callable,
+    split: bool = True,
+    seed: int = 999) -> List[Union[numpy.array, pandas.DataFrame]]:
+  """
+    Loads test data from the sklearn Iris dataset in a given format,
+    either in a single or multiple batches.
+    Args:
+      data_type: Datatype of the iris test dataset.
+      split: Split the dataset in different batches or return single batch.
+      seed: Random state for splitting the train and test set.
+  """
+  dataset = load_iris()
+  _, x_test, _, _ = train_test_split(
+      dataset['data'], dataset['target'], test_size=.2, random_state=seed)
+
+  if split:
+    return [(index, data_type(sample.reshape(1, -1))) for index,
+            sample in enumerate(x_test)]
+  return [(0, data_type(x_test))]
+
+
+def run(
+    argv=None, save_main_session=True, test_pipeline=None) -> PipelineResult:
+  """
+    Args:
+      argv: Command line arguments defined for this example.
+      save_main_session: Used for internal testing.
+      test_pipeline: Used for internal testing.
+  """
+  known_args, pipeline_args = parse_known_args(argv)
+  pipeline_options = PipelineOptions(pipeline_args)
+  pipeline_options.view_as(SetupOptions).save_main_session = save_main_session
+
+  data_types = {
+      'numpy': (numpy.array, XGBoostModelHandlerNumpy),
+      'pandas': (pandas.DataFrame, XGBoostModelHandlerPandas),
+      'scipy': (scipy.sparse.csr_matrix, XGBoostModelHandlerSciPy),
+      'datatable': (datatable.Frame, XGBoostModelHandlerDatatable),
+  }
+
+  input_data_type, model_handler = data_types[known_args.input_type]
+
+  xgboost_model_handler = KeyedModelHandler(
+      model_handler(
+          model_class=xgboost.XGBClassifier,
+          model_state=known_args.model_state))
+
+  input_data = load_sklearn_iris_test_data(
+      data_type=input_data_type, split=known_args.split)
+
+  pipeline = test_pipeline
+  if not test_pipeline:
+    pipeline = beam.Pipeline(options=pipeline_options)
+
+  predictions = (
+      pipeline
+      | "ReadInputData" >> beam.Create(input_data)
+      | "RunInference" >> RunInference(xgboost_model_handler)
+      | "PostProcessOutputs" >> beam.ParDo(PostProcessor()))
+
+  _ = predictions | "WriteOutput" >> beam.io.WriteToText(
+      known_args.output, shard_name_template='', append_trailing_newlines=True)
+
+  result = pipeline.run()
+  result.wait_until_finish()
+  return result
+
+
+if __name__ == '__main__':
+  logging.getLogger().setLevel(logging.INFO)
+  _train_model()

Review Comment:
   @damccorm Do you think it is better to keep the training function or load the model directly from GCS? It looks like the pipelines for pytorch and sklearn go for the second approach.



-- 
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] damccorm commented on pull request #24965: XGBoost modelhandler implementation

Posted by "damccorm (via GitHub)" <gi...@apache.org>.
damccorm commented on PR #24965:
URL: https://github.com/apache/beam/pull/24965#issuecomment-1463808579

   Run PythonDocker 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] damccorm commented on pull request #24965: XGBoost modelhandler implementation

Posted by "damccorm (via GitHub)" <gi...@apache.org>.
damccorm commented on PR #24965:
URL: https://github.com/apache/beam/pull/24965#issuecomment-1463808858

   Run Python_Transforms 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] damccorm commented on a diff in pull request #24965: XGBoost modelhandler implementation

Posted by "damccorm (via GitHub)" <gi...@apache.org>.
damccorm commented on code in PR #24965:
URL: https://github.com/apache/beam/pull/24965#discussion_r1127964549


##########
sdks/python/test-suites/direct/common.gradle:
##########
@@ -308,12 +308,41 @@ task tensorflowInferenceTest {
     }
 }
 
+// XGBoost RunInference IT tests
+task xgboostInferenceTest {
+  dependsOn 'installGcpTest'
+  dependsOn ':sdks:python:sdist'
+  def requirementsFile = "${rootDir}/sdks/python/apache_beam/ml/inference/xgboost_tests_requirements.txt"
+  doFirst {
+      exec {
+        executable 'sh'
+        args '-c', ". ${envdir}/bin/activate && pip install -r $requirementsFile"
+      }
+    }
+  doLast {
+      def testOpts = basicTestOpts
+      def argMap = [
+          "test_opts": testOpts,
+          "suite": "postCommitIT-direct-py${pythonVersionSuffix}",
+          "collect": "uses_xgboost and it_postcommit",
+          "runner": "TestDirectRunner"
+      ]
+      def cmdArgs = mapToArgString(argMap)
+      exec {
+        executable 'sh'
+        args '-c', ". ${envdir}/bin/activate && export FORCE_XGBOOST_IT=1 && ${runScriptsDir}/run_integration_test.sh $cmdArgs"
+      }
+    }
+
+}
+
 // Add all the RunInference framework IT tests to this gradle task that runs on Direct Runner Post commit suite.
 project.tasks.register("inferencePostCommitIT") {
   dependsOn = [
   'torchInferenceTest',
   'sklearnInferenceTest',
   'tfxInferenceTest',
   'tensorflowInferenceTest'
+  'xgboostInferenceTest'

Review Comment:
   ```suggestion
     'tensorflowInferenceTest',
     'xgboostInferenceTest'
   ```
   
   Ah darn, we need a comma here too



-- 
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] jaxpr commented on pull request #24965: XGBoost modelhandler implementation

Posted by "jaxpr (via GitHub)" <gi...@apache.org>.
jaxpr commented on PR #24965:
URL: https://github.com/apache/beam/pull/24965#issuecomment-1413809419

   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] damccorm commented on pull request #24965: XGBoost modelhandler implementation

Posted by "damccorm (via GitHub)" <gi...@apache.org>.
damccorm commented on PR #24965:
URL: https://github.com/apache/beam/pull/24965#issuecomment-1463809115

   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] damccorm commented on pull request #24965: XGBoost modelhandler implementation

Posted by "damccorm (via GitHub)" <gi...@apache.org>.
damccorm commented on PR #24965:
URL: https://github.com/apache/beam/pull/24965#issuecomment-1463933668

   Run Python_Dataframes 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] damccorm commented on pull request #24965: XGBoost modelhandler implementation

Posted by "damccorm (via GitHub)" <gi...@apache.org>.
damccorm commented on PR #24965:
URL: https://github.com/apache/beam/pull/24965#issuecomment-1464178977

   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] damccorm commented on pull request #24965: XGBoost modelhandler implementation

Posted by GitBox <gi...@apache.org>.
damccorm commented on PR #24965:
URL: https://github.com/apache/beam/pull/24965#issuecomment-1379047321

   > @damccorm @AnandInguva Could you take a look at the code ?
   
   I was in the process of reviewing :) looks great, thank you!


-- 
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] damccorm commented on pull request #24965: XGBoost modelhandler implementation

Posted by GitBox <gi...@apache.org>.
damccorm commented on PR #24965:
URL: https://github.com/apache/beam/pull/24965#issuecomment-1396972201

   stop reviewer notifications


-- 
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 #24965: XGBoost modelhandler implementation

Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on PR #24965:
URL: https://github.com/apache/beam/pull/24965#issuecomment-1396885804

   Reminder, please take a look at this pr: @pabloem 


-- 
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] riteshghorse commented on a diff in pull request #24965: XGBoost modelhandler implementation

Posted by "riteshghorse (via GitHub)" <gi...@apache.org>.
riteshghorse commented on code in PR #24965:
URL: https://github.com/apache/beam/pull/24965#discussion_r1100277153


##########
sdks/python/tox.ini:
##########
@@ -326,3 +326,20 @@ commands =
   # Run all PyTorch unit tests
   # Allow exit code 5 (no tests run) so that we can run this command safely on arbitrary subdirectories.
   /bin/sh -c 'pytest -o junit_suite_name={envname} --junitxml=pytest_{envname}.xml -n 6 -m uses_pytorch {posargs}; ret=$?; [ $ret = 5 ] && exit 0 || exit $ret'
+
+[testenv:py{37,38,39,310}-xgboost-{160,170}]
+deps =
+  -r build-requirements.txt
+  160:
+    xgboost>=1.6.0,<1.7.0
+    datatable==1.0.0
+  170:
+    xgboost>=1.7.0
+    datatable==1.0.0
+extras = test,gcp
+commands =
+  # Log XGBoost version for debugging
+  /bin/sh -c "pip freeze | grep -E torch"
+  # Run all PyTorch unit tests

Review Comment:
   ```suggestion
     # Run all XGBoost unit tests
   ```



-- 
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 #24965: XGBoost modelhandler implementation

Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on PR #24965:
URL: https://github.com/apache/beam/pull/24965#issuecomment-1377651802

   Assigning reviewers. If you would like to opt out of this review, comment `assign to next reviewer`:
   
   R: @pabloem 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] damccorm commented on pull request #24965: XGBoost modelhandler implementation

Posted by "damccorm (via GitHub)" <gi...@apache.org>.
damccorm commented on PR #24965:
URL: https://github.com/apache/beam/pull/24965#issuecomment-1463808383

   Test failures so far seem to be from jenkins issues


-- 
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] damccorm commented on pull request #24965: XGBoost modelhandler implementation

Posted by "damccorm (via GitHub)" <gi...@apache.org>.
damccorm commented on PR #24965:
URL: https://github.com/apache/beam/pull/24965#issuecomment-1463808942

   Run Python_Transforms 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] damccorm commented on pull request #24965: XGBoost modelhandler implementation

Posted by "damccorm (via GitHub)" <gi...@apache.org>.
damccorm commented on PR #24965:
URL: https://github.com/apache/beam/pull/24965#issuecomment-1419222899

   Also, looks like the formatter/lint/RAT precommit checks are failing.
   
   Formatter/lint exceptions should be visible in the "details" link, along with a command you can run locally to fix.
   
   RAT exceptions are due to missing a license header (looks like you're missing a license on `xgboost_tests_requirements.txt`)


-- 
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] damccorm commented on a diff in pull request #24965: XGBoost modelhandler implementation

Posted by "damccorm (via GitHub)" <gi...@apache.org>.
damccorm commented on code in PR #24965:
URL: https://github.com/apache/beam/pull/24965#discussion_r1085732379


##########
sdks/python/apache_beam/examples/inference/xgboost_iris_classification.py:
##########
@@ -0,0 +1,169 @@
+#
+# 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 logging
+from typing import Callable
+from typing import Iterable
+from typing import List
+from typing import Tuple
+from typing import Union
+
+import datatable
+import numpy
+import pandas
+import scipy
+import xgboost
+from sklearn.datasets import load_iris
+from sklearn.model_selection import train_test_split
+
+import apache_beam as beam
+from apache_beam.ml.inference.base import KeyedModelHandler
+from apache_beam.ml.inference.base import PredictionResult
+from apache_beam.ml.inference.base import RunInference
+from apache_beam.ml.inference.xgboost_inference import XGBoostModelHandlerDatatable
+from apache_beam.ml.inference.xgboost_inference import XGBoostModelHandlerNumpy
+from apache_beam.ml.inference.xgboost_inference import XGBoostModelHandlerPandas
+from apache_beam.ml.inference.xgboost_inference import XGBoostModelHandlerSciPy
+from apache_beam.options.pipeline_options import PipelineOptions
+from apache_beam.options.pipeline_options import SetupOptions
+from apache_beam.runners.runner import PipelineResult
+
+
+def _train_model(model_state_output_path: str = '/tmp/model.json', seed=999):
+  """Function to train an XGBoost Classifier using the sklearn Iris dataset"""
+  dataset = load_iris()
+  x_train, _, y_train, _ = train_test_split(
+      dataset['data'], dataset['target'], test_size=.2, random_state=seed)
+  booster = xgboost.XGBClassifier(
+      n_estimators=2, max_depth=2, learning_rate=1, objective='binary:logistic')
+  booster.fit(x_train, y_train)
+  booster.save_model(model_state_output_path)
+  return booster
+
+
+class PostProcessor(beam.DoFn):
+  """Process the PredictionResult to get the predicted label.
+  Returns a comma separated string with true label and predicted label.
+  """
+  def process(self, element: Tuple[int, PredictionResult]) -> Iterable[str]:
+    label, prediction_result = element
+    prediction = prediction_result.inference
+    yield '{},{}'.format(label, prediction)
+
+
+def parse_known_args(argv):
+  """Parses args for the workflow."""
+  parser = argparse.ArgumentParser()
+  parser.add_argument(
+      '--input-type',
+      dest='input_type',
+      required=True,
+      choices=['numpy', 'pandas', 'scipy', 'datatable'],
+      help=
+      'Datatype of the input data.'
+  )
+  parser.add_argument(
+      '--output',
+      dest='output',
+      required=True,
+      help='Path to save output predictions.')
+  parser.add_argument(
+      '--model-state',
+      dest='model_state',
+      required=True,
+      help='Path to the state of the XGBoost model loaded for Inference.'
+  )
+  group = parser.add_mutually_exclusive_group(required=True)
+  group.add_argument('--split', action='store_true', dest='split')
+  group.add_argument('--no-split', action='store_false', dest='split')
+  return parser.parse_known_args(argv)
+
+
+def load_sklearn_iris_test_data(
+    data_type: Callable,
+    split: bool = True,
+    seed: int = 999) -> List[Union[numpy.array, pandas.DataFrame]]:
+  """
+    Loads test data from the sklearn Iris dataset in a given format,
+    either in a single or multiple batches.
+    Args:
+      data_type: Datatype of the iris test dataset.
+      split: Split the dataset in different batches or return single batch.
+      seed: Random state for splitting the train and test set.
+  """
+  dataset = load_iris()
+  _, x_test, _, _ = train_test_split(
+      dataset['data'], dataset['target'], test_size=.2, random_state=seed)
+
+  if split:
+    return [(index, data_type(sample.reshape(1, -1))) for index,
+            sample in enumerate(x_test)]
+  return [(0, data_type(x_test))]
+
+
+def run(
+    argv=None, save_main_session=True, test_pipeline=None) -> PipelineResult:
+  """
+    Args:
+      argv: Command line arguments defined for this example.
+      save_main_session: Used for internal testing.
+      test_pipeline: Used for internal testing.
+  """
+  known_args, pipeline_args = parse_known_args(argv)
+  pipeline_options = PipelineOptions(pipeline_args)
+  pipeline_options.view_as(SetupOptions).save_main_session = save_main_session
+
+  data_types = {
+      'numpy': (numpy.array, XGBoostModelHandlerNumpy),
+      'pandas': (pandas.DataFrame, XGBoostModelHandlerPandas),
+      'scipy': (scipy.sparse.csr_matrix, XGBoostModelHandlerSciPy),
+      'datatable': (datatable.Frame, XGBoostModelHandlerDatatable),
+  }
+
+  input_data_type, model_handler = data_types[known_args.input_type]
+
+  xgboost_model_handler = KeyedModelHandler(
+      model_handler(
+          model_class=xgboost.XGBClassifier,
+          model_state=known_args.model_state))
+
+  input_data = load_sklearn_iris_test_data(
+      data_type=input_data_type, split=known_args.split)
+
+  pipeline = test_pipeline
+  if not test_pipeline:
+    pipeline = beam.Pipeline(options=pipeline_options)
+
+  predictions = (
+      pipeline
+      | "ReadInputData" >> beam.Create(input_data)
+      | "RunInference" >> RunInference(xgboost_model_handler)
+      | "PostProcessOutputs" >> beam.ParDo(PostProcessor()))
+
+  _ = predictions | "WriteOutput" >> beam.io.WriteToText(
+      known_args.output, shard_name_template='', append_trailing_newlines=True)
+
+  result = pipeline.run()
+  result.wait_until_finish()
+  return result
+
+
+if __name__ == '__main__':
+  logging.getLogger().setLevel(logging.INFO)
+  _train_model()

Review Comment:
   Loading it from GCS is probably better, please note in the Readme how the model was trained 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] damccorm commented on pull request #24965: XGBoost modelhandler implementation

Posted by "damccorm (via GitHub)" <gi...@apache.org>.
damccorm commented on PR #24965:
URL: https://github.com/apache/beam/pull/24965#issuecomment-1448675603

   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] damccorm commented on a diff in pull request #24965: XGBoost modelhandler implementation

Posted by "damccorm (via GitHub)" <gi...@apache.org>.
damccorm commented on code in PR #24965:
URL: https://github.com/apache/beam/pull/24965#discussion_r1127961694


##########
sdks/python/test-suites/direct/common.gradle:
##########
@@ -308,12 +308,42 @@ task tensorflowInferenceTest {
     }
 }
 
+// XGBoost RunInference IT tests
+task xgboostInferenceTest {
+  dependsOn 'installGcpTest'
+  dependsOn ':sdks:python:sdist'
+  def requirementsFile = "${rootDir}/sdks/python/apache_beam/ml/inference/xgboost_tests_requirements.txt"
+  doFirst {
+      exec {
+        executable 'sh'
+        args '-c', ". ${envdir}/bin/activate && pip install -r $requirementsFile"
+      }
+    }
+  doLast {
+      def testOpts = basicTestOpts
+      def argMap = [
+          "test_opts": testOpts,
+          "suite": "postCommitIT-direct-py${pythonVersionSuffix}",
+          "collect": "uses_xgboost and it_postcommit",
+          "runner": "TestDirectRunner"
+      ]
+      def cmdArgs = mapToArgString(argMap)
+      exec {
+        executable 'sh'
+        args '-c', ". ${envdir}/bin/activate && export FORCE_XGBOOST_IT=1 && ${runScriptsDir}/run_integration_test.sh $cmdArgs"
+      }
+    }
+
+}
+
 // Add all the RunInference framework IT tests to this gradle task that runs on Direct Runner Post commit suite.
 project.tasks.register("inferencePostCommitIT") {
   dependsOn = [
   'torchInferenceTest',
   'sklearnInferenceTest',
   'tfxInferenceTest',
   'tensorflowInferenceTest'
+  'tfxInferenceTest',

Review Comment:
   ```suggestion
   ```
   
   This got duplicated



-- 
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] damccorm commented on a diff in pull request #24965: XGBoost modelhandler implementation

Posted by "damccorm (via GitHub)" <gi...@apache.org>.
damccorm commented on code in PR #24965:
URL: https://github.com/apache/beam/pull/24965#discussion_r1127963626


##########
sdks/python/test-suites/direct/common.gradle:
##########
@@ -308,12 +308,42 @@ task tensorflowInferenceTest {
     }
 }
 
+// XGBoost RunInference IT tests
+task xgboostInferenceTest {
+  dependsOn 'installGcpTest'
+  dependsOn ':sdks:python:sdist'
+  def requirementsFile = "${rootDir}/sdks/python/apache_beam/ml/inference/xgboost_tests_requirements.txt"
+  doFirst {
+      exec {
+        executable 'sh'
+        args '-c', ". ${envdir}/bin/activate && pip install -r $requirementsFile"
+      }
+    }
+  doLast {
+      def testOpts = basicTestOpts
+      def argMap = [
+          "test_opts": testOpts,
+          "suite": "postCommitIT-direct-py${pythonVersionSuffix}",
+          "collect": "uses_xgboost and it_postcommit",
+          "runner": "TestDirectRunner"
+      ]
+      def cmdArgs = mapToArgString(argMap)
+      exec {
+        executable 'sh'
+        args '-c', ". ${envdir}/bin/activate && export FORCE_XGBOOST_IT=1 && ${runScriptsDir}/run_integration_test.sh $cmdArgs"
+      }
+    }
+
+}
+
 // Add all the RunInference framework IT tests to this gradle task that runs on Direct Runner Post commit suite.
 project.tasks.register("inferencePostCommitIT") {
   dependsOn = [
   'torchInferenceTest',
   'sklearnInferenceTest',
   'tfxInferenceTest',
   'tensorflowInferenceTest'
+  'tfxInferenceTest',

Review Comment:
   I'm going to just commit it so that we can get more signal from other tests that this is blocking



-- 
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] damccorm commented on pull request #24965: XGBoost modelhandler implementation

Posted by "damccorm (via GitHub)" <gi...@apache.org>.
damccorm commented on PR #24965:
URL: https://github.com/apache/beam/pull/24965#issuecomment-1458287306

   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] jaxpr commented on pull request #24965: XGBoost modelhandler implementation

Posted by GitBox <gi...@apache.org>.
jaxpr commented on PR #24965:
URL: https://github.com/apache/beam/pull/24965#issuecomment-1379021802

   @damccorm @AnandInguva Could you take a look at the code ?


-- 
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 #24965: XGBoost modelhandler implementation

Posted by GitBox <gi...@apache.org>.
AnandInguva commented on code in PR #24965:
URL: https://github.com/apache/beam/pull/24965#discussion_r1067299492


##########
sdks/python/apache_beam/ml/inference/xgboost_inference_it_test.py:
##########
@@ -0,0 +1,314 @@
+#
+# 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 logging
+
+import pytest
+import unittest
+import uuid
+
+from apache_beam.examples.inference import xgboost_iris_classification
+from apache_beam.io.filesystems import FileSystems
+from apache_beam.testing.test_pipeline import TestPipeline
+
+
+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
+
+
+@pytest.mark.uses_xgboost
+@pytest.mark.it_postcommit
+class XGBoostInference(unittest.TestCase):
+  def test_iris_classification_numpy_single_batch(self):
+    test_pipeline = TestPipeline(is_integration_test=True)
+    input_type = 'numpy'
+    output_file_dir = '/tmp'

Review Comment:
   Can we have output file directory to gcs since it gets cleaned up periodically? https://github.com/apache/beam/blob/c0e689331c2a6573ecf267b9bef133a85ea8a36c/sdks/python/apache_beam/ml/inference/pytorch_inference_it_test.py#L74



##########
sdks/python/apache_beam/ml/inference/xgboost_inference_test.py:
##########
@@ -0,0 +1,443 @@
+#
+# 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 os
+import shutil
+import sys
+import tempfile
+import unittest
+import zipfile
+from typing import Any
+from typing import Tuple
+
+import datatable
+import numpy
+import pandas
+import pytest
+import scipy
+
+import apache_beam as beam
+from apache_beam.ml.inference import RunInference
+from apache_beam.ml.inference.base import KeyedModelHandler
+from apache_beam.ml.inference.base import PredictionResult
+from apache_beam.ml.inference.xgboost_inference import XGBoostModelHandlerDatatable
+from apache_beam.ml.inference.xgboost_inference import XGBoostModelHandlerNumpy
+from apache_beam.ml.inference.xgboost_inference import XGBoostModelHandlerPandas
+from apache_beam.ml.inference.xgboost_inference import XGBoostModelHandlerSciPy
+from apache_beam.testing.test_pipeline import TestPipeline
+from apache_beam.testing.util import assert_that
+from apache_beam.testing.util import equal_to
+
+try:
+  import xgboost
+except ImportError:
+  raise unittest.SkipTest('XGBoost dependencies are not installed')
+
+
+def _compare_prediction_result(a: PredictionResult, b: PredictionResult):
+  if isinstance(a.example, scipy.sparse.csr_matrix) and isinstance(

Review Comment:
   For unittests to run, let us add these to the tox.ini file with the dependencies needed for xgboost tests similar to torch
   
   https://github.com/apache/beam/blob/3a98ecbdbfad59ae8cb96d7eaef544d25d9c437f/sdks/python/tox.ini#L317



##########
sdks/python/apache_beam/examples/inference/xgboost_iris_classification.py:
##########
@@ -0,0 +1,169 @@
+#
+# 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 logging
+from typing import Callable
+from typing import Iterable
+from typing import List
+from typing import Tuple
+from typing import Union
+
+import datatable
+import numpy
+import pandas
+import scipy
+import xgboost
+from sklearn.datasets import load_iris
+from sklearn.model_selection import train_test_split
+
+import apache_beam as beam
+from apache_beam.ml.inference.base import KeyedModelHandler
+from apache_beam.ml.inference.base import PredictionResult
+from apache_beam.ml.inference.base import RunInference
+from apache_beam.ml.inference.xgboost_inference import XGBoostModelHandlerDatatable
+from apache_beam.ml.inference.xgboost_inference import XGBoostModelHandlerNumpy
+from apache_beam.ml.inference.xgboost_inference import XGBoostModelHandlerPandas
+from apache_beam.ml.inference.xgboost_inference import XGBoostModelHandlerSciPy
+from apache_beam.options.pipeline_options import PipelineOptions
+from apache_beam.options.pipeline_options import SetupOptions
+from apache_beam.runners.runner import PipelineResult
+
+
+def _train_model(model_state_output_path: str = '/tmp/model.json', seed=999):
+  """Function to train an XGBoost Classifier using the sklearn Iris dataset"""
+  dataset = load_iris()
+  x_train, _, y_train, _ = train_test_split(
+      dataset['data'], dataset['target'], test_size=.2, random_state=seed)
+  booster = xgboost.XGBClassifier(
+      n_estimators=2, max_depth=2, learning_rate=1, objective='binary:logistic')
+  booster.fit(x_train, y_train)
+  booster.save_model(model_state_output_path)
+  return booster
+
+
+class PostProcessor(beam.DoFn):
+  """Process the PredictionResult to get the predicted label.
+  Returns a comma separated string with true label and predicted label.
+  """
+  def process(self, element: Tuple[int, PredictionResult]) -> Iterable[str]:
+    label, prediction_result = element
+    prediction = prediction_result.inference
+    yield '{},{}'.format(label, prediction)
+
+
+def parse_known_args(argv):
+  """Parses args for the workflow."""
+  parser = argparse.ArgumentParser()
+  parser.add_argument(
+      '--input-type',
+      dest='input_type',
+      required=True,
+      choices=['numpy', 'pandas', 'scipy', 'datatable'],
+      help=
+      'Datatype of the input data.'
+  )
+  parser.add_argument(
+      '--output',
+      dest='output',
+      required=True,
+      help='Path to save output predictions.')
+  parser.add_argument(
+      '--model-state',

Review Comment:
   ```suggestion
         '--model-state',
         '--model_state'
   ```
   
   I would replace `-` with `_`



##########
sdks/python/container/py310/base_image_requirements.txt:
##########
@@ -154,4 +155,5 @@ urllib3==1.26.13
 websocket-client==1.4.2
 Werkzeug==2.2.2
 wrapt==1.14.1
+xgboost==1.7.1

Review Comment:
   I would like to push back on adding these dependencies to container because they would increase the size of the container image since these are not dependencies of Apache Beam



##########
sdks/python/apache_beam/examples/inference/xgboost_iris_classification.py:
##########
@@ -0,0 +1,169 @@
+#
+# 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 logging
+from typing import Callable
+from typing import Iterable
+from typing import List
+from typing import Tuple
+from typing import Union
+
+import datatable
+import numpy
+import pandas
+import scipy
+import xgboost
+from sklearn.datasets import load_iris
+from sklearn.model_selection import train_test_split
+
+import apache_beam as beam
+from apache_beam.ml.inference.base import KeyedModelHandler
+from apache_beam.ml.inference.base import PredictionResult
+from apache_beam.ml.inference.base import RunInference
+from apache_beam.ml.inference.xgboost_inference import XGBoostModelHandlerDatatable
+from apache_beam.ml.inference.xgboost_inference import XGBoostModelHandlerNumpy
+from apache_beam.ml.inference.xgboost_inference import XGBoostModelHandlerPandas
+from apache_beam.ml.inference.xgboost_inference import XGBoostModelHandlerSciPy
+from apache_beam.options.pipeline_options import PipelineOptions
+from apache_beam.options.pipeline_options import SetupOptions
+from apache_beam.runners.runner import PipelineResult
+
+
+def _train_model(model_state_output_path: str = '/tmp/model.json', seed=999):
+  """Function to train an XGBoost Classifier using the sklearn Iris dataset"""
+  dataset = load_iris()
+  x_train, _, y_train, _ = train_test_split(
+      dataset['data'], dataset['target'], test_size=.2, random_state=seed)
+  booster = xgboost.XGBClassifier(
+      n_estimators=2, max_depth=2, learning_rate=1, objective='binary:logistic')
+  booster.fit(x_train, y_train)
+  booster.save_model(model_state_output_path)
+  return booster
+
+
+class PostProcessor(beam.DoFn):
+  """Process the PredictionResult to get the predicted label.
+  Returns a comma separated string with true label and predicted label.
+  """
+  def process(self, element: Tuple[int, PredictionResult]) -> Iterable[str]:
+    label, prediction_result = element
+    prediction = prediction_result.inference
+    yield '{},{}'.format(label, prediction)
+
+
+def parse_known_args(argv):
+  """Parses args for the workflow."""
+  parser = argparse.ArgumentParser()
+  parser.add_argument(
+      '--input-type',
+      dest='input_type',
+      required=True,
+      choices=['numpy', 'pandas', 'scipy', 'datatable'],
+      help=
+      'Datatype of the input data.'
+  )
+  parser.add_argument(
+      '--output',
+      dest='output',
+      required=True,
+      help='Path to save output predictions.')
+  parser.add_argument(
+      '--model-state',

Review Comment:
   In the other arguments as well



##########
sdks/python/apache_beam/examples/inference/xgboost_iris_classification.py:
##########
@@ -0,0 +1,169 @@
+#
+# 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 logging
+from typing import Callable
+from typing import Iterable
+from typing import List
+from typing import Tuple
+from typing import Union
+
+import datatable
+import numpy
+import pandas
+import scipy
+import xgboost
+from sklearn.datasets import load_iris
+from sklearn.model_selection import train_test_split
+
+import apache_beam as beam
+from apache_beam.ml.inference.base import KeyedModelHandler
+from apache_beam.ml.inference.base import PredictionResult
+from apache_beam.ml.inference.base import RunInference
+from apache_beam.ml.inference.xgboost_inference import XGBoostModelHandlerDatatable
+from apache_beam.ml.inference.xgboost_inference import XGBoostModelHandlerNumpy
+from apache_beam.ml.inference.xgboost_inference import XGBoostModelHandlerPandas
+from apache_beam.ml.inference.xgboost_inference import XGBoostModelHandlerSciPy
+from apache_beam.options.pipeline_options import PipelineOptions
+from apache_beam.options.pipeline_options import SetupOptions
+from apache_beam.runners.runner import PipelineResult
+
+
+def _train_model(model_state_output_path: str = '/tmp/model.json', seed=999):
+  """Function to train an XGBoost Classifier using the sklearn Iris dataset"""
+  dataset = load_iris()
+  x_train, _, y_train, _ = train_test_split(
+      dataset['data'], dataset['target'], test_size=.2, random_state=seed)
+  booster = xgboost.XGBClassifier(
+      n_estimators=2, max_depth=2, learning_rate=1, objective='binary:logistic')
+  booster.fit(x_train, y_train)
+  booster.save_model(model_state_output_path)
+  return booster
+
+
+class PostProcessor(beam.DoFn):
+  """Process the PredictionResult to get the predicted label.
+  Returns a comma separated string with true label and predicted label.
+  """
+  def process(self, element: Tuple[int, PredictionResult]) -> Iterable[str]:
+    label, prediction_result = element
+    prediction = prediction_result.inference
+    yield '{},{}'.format(label, prediction)
+
+
+def parse_known_args(argv):
+  """Parses args for the workflow."""
+  parser = argparse.ArgumentParser()
+  parser.add_argument(
+      '--input-type',
+      dest='input_type',
+      required=True,
+      choices=['numpy', 'pandas', 'scipy', 'datatable'],
+      help=
+      'Datatype of the input data.'
+  )
+  parser.add_argument(
+      '--output',
+      dest='output',
+      required=True,
+      help='Path to save output predictions.')
+  parser.add_argument(
+      '--model-state',
+      dest='model_state',
+      required=True,
+      help='Path to the state of the XGBoost model loaded for Inference.'
+  )
+  group = parser.add_mutually_exclusive_group(required=True)
+  group.add_argument('--split', action='store_true', dest='split')
+  group.add_argument('--no-split', action='store_false', dest='split')
+  return parser.parse_known_args(argv)
+
+
+def load_sklearn_iris_test_data(
+    data_type: Callable,
+    split: bool = True,
+    seed: int = 999) -> List[Union[numpy.array, pandas.DataFrame]]:
+  """
+    Loads test data from the sklearn Iris dataset in a given format,
+    either in a single or multiple batches.
+    Args:
+      data_type: Datatype of the iris test dataset.
+      split: Split the dataset in different batches or return single batch.
+      seed: Random state for splitting the train and test set.
+  """
+  dataset = load_iris()
+  _, x_test, _, _ = train_test_split(
+      dataset['data'], dataset['target'], test_size=.2, random_state=seed)
+
+  if split:
+    return [(index, data_type(sample.reshape(1, -1))) for index,
+            sample in enumerate(x_test)]
+  return [(0, data_type(x_test))]
+
+
+def run(
+    argv=None, save_main_session=True, test_pipeline=None) -> PipelineResult:
+  """
+    Args:
+      argv: Command line arguments defined for this example.
+      save_main_session: Used for internal testing.
+      test_pipeline: Used for internal testing.
+  """
+  known_args, pipeline_args = parse_known_args(argv)
+  pipeline_options = PipelineOptions(pipeline_args)
+  pipeline_options.view_as(SetupOptions).save_main_session = save_main_session
+
+  data_types = {
+      'numpy': (numpy.array, XGBoostModelHandlerNumpy),
+      'pandas': (pandas.DataFrame, XGBoostModelHandlerPandas),
+      'scipy': (scipy.sparse.csr_matrix, XGBoostModelHandlerSciPy),
+      'datatable': (datatable.Frame, XGBoostModelHandlerDatatable),
+  }
+
+  input_data_type, model_handler = data_types[known_args.input_type]
+
+  xgboost_model_handler = KeyedModelHandler(
+      model_handler(
+          model_class=xgboost.XGBClassifier,
+          model_state=known_args.model_state))
+
+  input_data = load_sklearn_iris_test_data(
+      data_type=input_data_type, split=known_args.split)
+
+  pipeline = test_pipeline
+  if not test_pipeline:
+    pipeline = beam.Pipeline(options=pipeline_options)
+
+  predictions = (
+      pipeline
+      | "ReadInputData" >> beam.Create(input_data)
+      | "RunInference" >> RunInference(xgboost_model_handler)
+      | "PostProcessOutputs" >> beam.ParDo(PostProcessor()))
+
+  _ = predictions | "WriteOutput" >> beam.io.WriteToText(
+      known_args.output, shard_name_template='', append_trailing_newlines=True)
+
+  result = pipeline.run()
+  result.wait_until_finish()
+  return result
+
+
+if __name__ == '__main__':
+  logging.getLogger().setLevel(logging.INFO)
+  _train_model()

Review Comment:
   How long does it take to train the model? I hope this would take less time since it will run on user's machine



##########
sdks/python/apache_beam/ml/inference/xgboost_inference.py:
##########
@@ -0,0 +1,212 @@
+#
+# 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 sys
+from abc import ABC
+from typing import Any
+from typing import Callable
+from typing import Dict
+from typing import Iterable
+from typing import Optional
+from typing import Sequence
+from typing import Union
+
+import datatable
+import numpy
+import pandas
+import scipy
+import xgboost
+
+from apache_beam.ml.inference.base import ExampleT
+from apache_beam.ml.inference.base import ModelHandler
+from apache_beam.ml.inference.base import ModelT
+from apache_beam.ml.inference.base import PredictionResult
+from apache_beam.ml.inference.base import PredictionT
+
+
+class XGBoostModelHandler(ModelHandler[ExampleT, PredictionT, ModelT], ABC):

Review Comment:
   Users could(may be) use it if they want to use their own run_inference method. 
   
   We can also document in docstring in similar way 
   https://github.com/apache/beam/blob/c0e689331c2a6573ecf267b9bef133a85ea8a36c/sdks/python/apache_beam/pvalue.py#L351



##########
sdks/python/container/py310/base_image_requirements.txt:
##########
@@ -154,4 +155,5 @@ urllib3==1.26.13
 websocket-client==1.4.2
 Werkzeug==2.2.2
 wrapt==1.14.1
+xgboost==1.7.1

Review Comment:
   I wouldn't put `xgboost` here since it is not a dependency of beam. 
   
   We can have a requirements file and gradle task. We can add this gradle task to the PostCommit suites similarly on torch tests run.
   
   For direct runner, https://github.com/apache/beam/blob/c0e689331c2a6573ecf267b9bef133a85ea8a36c/sdks/python/test-suites/direct/common.gradle#L284
   
   For Dataflow runner,  https://github.com/apache/beam/blob/c0e689331c2a6573ecf267b9bef133a85ea8a36c/sdks/python/test-suites/dataflow/common.gradle#L403
   
   If IT tests are light weight and easy to run, we can add them to DirectRunner.
   
   



##########
sdks/python/apache_beam/ml/inference/xgboost_inference_it_test.py:
##########
@@ -0,0 +1,314 @@
+#
+# 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 logging
+
+import pytest
+import unittest
+import uuid
+
+from apache_beam.examples.inference import xgboost_iris_classification
+from apache_beam.io.filesystems import FileSystems
+from apache_beam.testing.test_pipeline import TestPipeline
+
+
+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
+
+
+@pytest.mark.uses_xgboost
+@pytest.mark.it_postcommit
+class XGBoostInference(unittest.TestCase):
+  def test_iris_classification_numpy_single_batch(self):
+    test_pipeline = TestPipeline(is_integration_test=True)
+    input_type = 'numpy'
+    output_file_dir = '/tmp'

Review Comment:
   Can we have this change for all the tests?



-- 
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 #24965: XGBoost modelhandler implementation

Posted by GitBox <gi...@apache.org>.
AnandInguva commented on code in PR #24965:
URL: https://github.com/apache/beam/pull/24965#discussion_r1067312752


##########
sdks/python/apache_beam/ml/inference/xgboost_inference_it_test.py:
##########
@@ -0,0 +1,314 @@
+#
+# 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 logging
+
+import pytest
+import unittest
+import uuid
+
+from apache_beam.examples.inference import xgboost_iris_classification
+from apache_beam.io.filesystems import FileSystems
+from apache_beam.testing.test_pipeline import TestPipeline
+
+
+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
+
+
+@pytest.mark.uses_xgboost
+@pytest.mark.it_postcommit
+class XGBoostInference(unittest.TestCase):
+  def test_iris_classification_numpy_single_batch(self):
+    test_pipeline = TestPipeline(is_integration_test=True)
+    input_type = 'numpy'
+    output_file_dir = '/tmp'

Review Comment:
   Ah I see it was already mentioned in the description about the GCS location. NVM about this commnet



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