Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

feat: adds function/method enhancements, demo samples #122

Merged
merged 31 commits into from Dec 17, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
3 changes: 3 additions & 0 deletions .github/CODEOWNERS
Validating CODEOWNERS rules …
Expand Up @@ -9,3 +9,6 @@

# The python-samples-owners team is the default owner for samples
/samples/**/*.py @dizcology @googleapis/python-samples-owners

# The enhanced client library tests are owned by @telpirion
/tests/unit/enhanced_library/*.py @telpirion
3 changes: 3 additions & 0 deletions google/cloud/aiplatform/helpers/__init__.py
@@ -0,0 +1,3 @@
from google.cloud.aiplatform.helpers import value_converter

__all__ = (value_converter,)
70 changes: 70 additions & 0 deletions google/cloud/aiplatform/helpers/_decorators.py
@@ -0,0 +1,70 @@
# Copyright 2020 Google LLC
#
# Licensed under the Apache License, Version 2.0 (the "License");
# you may not use this file except in compliance with the License.
# You may obtain a copy of the License at
#
# http://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing, software
# distributed under the License is distributed on an "AS IS" BASIS,
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
# See the License for the specific language governing permissions and
# limitations under the License.
from __future__ import absolute_import
from google.cloud.aiplatform.helpers import value_converter

from proto.marshal import Marshal
from proto.marshal.rules.struct import ValueRule
from google.protobuf.struct_pb2 import Value


class ConversionValueRule(ValueRule):
def to_python(self, value, *, absent: bool = None):
return super().to_python(value, absent=absent)

def to_proto(self, value):

# Need to check whether value is an instance
# of an enhanced type
if callable(getattr(value, "to_value", None)):
return value.to_value()
else:
return super().to_proto(value)


def _add_methods_to_classes_in_package(pkg):
classes = dict(
[(name, cls) for name, cls in pkg.__dict__.items() if isinstance(cls, type)]
)

for class_name, cls in classes.items():
# Add to_value() method to class with docstring
setattr(cls, "to_value", value_converter.to_value)
cls.to_value.__doc__ = value_converter.to_value.__doc__

# Add from_value() method to class with docstring
setattr(cls, "from_value", _add_from_value_to_class(cls))
cls.from_value.__doc__ = value_converter.from_value.__doc__

# Add from_map() method to class with docstring
setattr(cls, "from_map", _add_from_map_to_class(cls))
cls.from_map.__doc__ = value_converter.from_map.__doc__


def _add_from_value_to_class(cls):
def _from_value(value):
return value_converter.from_value(cls, value)

return _from_value


def _add_from_map_to_class(cls):
def _from_map(map_):
return value_converter.from_map(cls, map_)

return _from_map


marshal = Marshal(name="google.cloud.aiplatform.v1beta1")
marshal.register(Value, ConversionValueRule(marshal=marshal))
60 changes: 60 additions & 0 deletions google/cloud/aiplatform/helpers/value_converter.py
@@ -0,0 +1,60 @@
# Copyright 2020 Google LLC
#
# Licensed 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
#
# https://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing, software
# distributed under the License is distributed on an "AS IS" BASIS,
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
# See the License for the specific language governing permissions and
# limitations under the License.
from __future__ import absolute_import
from google.protobuf.struct_pb2 import Value
from google.protobuf import json_format
from proto.marshal.collections.maps import MapComposite
from proto.marshal import Marshal
from proto import Message
from proto.message import MessageMeta


def to_value(self: Message) -> Value:
"""Converts a message type to a :class:`~google.protobuf.struct_pb2.Value` object.

Args:
message: the message to convert

Returns:
the message as a :class:`~google.protobuf.struct_pb2.Value` object
"""
tmp_dict = json_format.MessageToDict(self._pb)
return json_format.ParseDict(tmp_dict, Value())


def from_value(cls: MessageMeta, value: Value) -> Message:
"""Creates instance of class from a :class:`~google.protobuf.struct_pb2.Value` object.

Args:
value: a :class:`~google.protobuf.struct_pb2.Value` object

Returns:
Instance of class
"""
value_dict = json_format.MessageToDict(value)
return json_format.ParseDict(value_dict, cls()._pb)


def from_map(cls: MessageMeta, map_: MapComposite) -> Message:
"""Creates instance of class from a :class:`~proto.marshal.collections.maps.MapComposite` object.

Args:
map_: a :class:`~proto.marshal.collections.maps.MapComposite` object

Returns:
Instance of class
"""
marshal = Marshal(name="marshal")
pb = marshal.to_proto(Value, map_)
return from_value(cls, pb)
Expand Up @@ -14,6 +14,8 @@
# See the License for the specific language governing permissions and
# limitations under the License.
#
from google.cloud.aiplatform.helpers import _decorators
import google.cloud.aiplatform.v1beta1.schema.predict.instance_v1beta1.types as pkg

from google.cloud.aiplatform.v1beta1.schema.predict.instance_v1beta1.types.image_classification import (
ImageClassificationPredictionInstance,
Expand Down Expand Up @@ -54,3 +56,4 @@
"VideoClassificationPredictionInstance",
"VideoObjectTrackingPredictionInstance",
)
_decorators._add_methods_to_classes_in_package(pkg)
Expand Up @@ -14,6 +14,8 @@
# See the License for the specific language governing permissions and
# limitations under the License.
#
from google.cloud.aiplatform.helpers import _decorators
import google.cloud.aiplatform.v1beta1.schema.predict.params_v1beta1.types as pkg

from google.cloud.aiplatform.v1beta1.schema.predict.params_v1beta1.types.image_classification import (
ImageClassificationPredictionParams,
Expand Down Expand Up @@ -42,3 +44,4 @@
"VideoClassificationPredictionParams",
"VideoObjectTrackingPredictionParams",
)
_decorators._add_methods_to_classes_in_package(pkg)
Expand Up @@ -14,6 +14,8 @@
# See the License for the specific language governing permissions and
# limitations under the License.
#
from google.cloud.aiplatform.helpers import _decorators
import google.cloud.aiplatform.v1beta1.schema.predict.prediction_v1beta1.types as pkg

from google.cloud.aiplatform.v1beta1.schema.predict.prediction_v1beta1.types.classification import (
ClassificationPredictionResult,
Expand Down Expand Up @@ -62,3 +64,4 @@
"VideoClassificationPredictionResult",
"VideoObjectTrackingPredictionResult",
)
_decorators._add_methods_to_classes_in_package(pkg)
Expand Up @@ -17,8 +17,10 @@

import proto # type: ignore


from google.cloud.aiplatform.v1beta1.schema.predict.instance import text_sentiment_pb2 as gcaspi_text_sentiment # type: ignore
# DO NOT OVERWRITE FOLLOWING LINE: it was manually edited.
from google.cloud.aiplatform.v1beta1.schema.predict.instance import (
TextSentimentPredictionInstance,
)


__protobuf__ = proto.module(
Expand Down Expand Up @@ -57,9 +59,7 @@ class Prediction(proto.Message):
sentiment = proto.Field(proto.INT32, number=1)

instance = proto.Field(
proto.MESSAGE,
number=1,
message=gcaspi_text_sentiment.TextSentimentPredictionInstance,
proto.MESSAGE, number=1, message=TextSentimentPredictionInstance,
)

prediction = proto.Field(proto.MESSAGE, number=2, message=Prediction,)
Expand Down
Expand Up @@ -14,6 +14,8 @@
# See the License for the specific language governing permissions and
# limitations under the License.
#
from google.cloud.aiplatform.helpers import _decorators
import google.cloud.aiplatform.v1beta1.schema.trainingjob.definition_v1beta1.types as pkg

from google.cloud.aiplatform.v1beta1.schema.trainingjob.definition_v1beta1.types.automl_forecasting import (
AutoMlForecasting,
Expand Down Expand Up @@ -130,3 +132,4 @@
"AutoMlVideoObjectTrackingInputs",
"ExportEvaluatedDataItemsConfig",
)
_decorators._add_methods_to_classes_in_package(pkg)
Expand Up @@ -78,14 +78,14 @@ class AutoMlForecastingInputs(proto.Message):
function over the validation set.

The supported optimization objectives:
"minimize-rmse" (default) - Minimize root-
"minimize-rmse" (default) - Minimize root-
Copy link
Contributor

Choose a reason for hiding this comment

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

PSA: If the proto comments are formatted correctly but the docstrings are getting generated in a weird state please file bugs on the generator repo. https://github.com/googleapis/gapic-generator-python

mean-squared error (RMSE). "minimize-mae" -
Minimize mean-absolute error (MAE). "minimize-
rmsle" - Minimize root-mean-squared log error
(RMSLE). "minimize-rmspe" - Minimize root-
mean-squared percentage error (RMSPE).
"minimize-wape-mae" - Minimize the combination
of weighted absolute percentage error (WAPE)
of weighted absolute percentage error (WAPE)
and mean-absolute-error (MAE).
train_budget_milli_node_hours (int):
Required. The train budget of creating this
Expand Down Expand Up @@ -418,11 +418,11 @@ class Period(proto.Message):
unit (str):
The time granularity unit of this time
period. The supported unit are:
"hour"
"day"
"week"
"month"
"year".
"hour"
"day"
"week"
"month"
"year".
quantity (int):
The number of units per period, e.g. 3 weeks
or 2 months.
Expand Down
Expand Up @@ -61,7 +61,7 @@ class AutoMlTablesInputs(proto.Message):
produce. "classification" - Predict one out of
multiple target values is
picked for each row.
"regression" - Predict a value based on its
"regression" - Predict a value based on its
relation to other values. This
type is available only to columns that contain
semantically numeric values, i.e. integers or
Expand All @@ -87,22 +87,22 @@ class AutoMlTablesInputs(proto.Message):
the prediction type. If the field is not set, a
default objective function is used.
classification (binary):
"maximize-au-roc" (default) - Maximize the
"maximize-au-roc" (default) - Maximize the
area under the receiver
operating characteristic (ROC) curve.
"minimize-log-loss" - Minimize log loss.
"maximize-au-prc" - Maximize the area under
"maximize-au-prc" - Maximize the area under
the precision-recall curve. "maximize-
precision-at-recall" - Maximize precision for a
specified
recall value. "maximize-recall-at-precision" -
Maximize recall for a specified
precision value.
classification (multi-class):
"minimize-log-loss" (default) - Minimize log
"minimize-log-loss" (default) - Minimize log
loss.
regression:
"minimize-rmse" (default) - Minimize root-
"minimize-rmse" (default) - Minimize root-
mean-squared error (RMSE). "minimize-mae" -
Minimize mean-absolute error (MAE). "minimize-
rmsle" - Minimize root-mean-squared log error
Expand Down
Expand Up @@ -14,8 +14,8 @@

# [START aiplatform_create_training_pipeline_image_classification_sample]
from google.cloud import aiplatform
from google.protobuf import json_format
from google.protobuf.struct_pb2 import Value
from google.cloud.aiplatform.v1beta1.schema.trainingjob import definition
Copy link
Contributor

Choose a reason for hiding this comment

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

is it possible to remove v1beta by exporting schema under cloud.aiplatform? (without moving all the files)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not very easily, no.

ModelType = definition.AutoMlImageClassificationInputs().ModelType
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we don't have to create an instance of AutoMlImageClassificationInputs here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Removed.



def create_training_pipeline_image_classification_sample(
Expand All @@ -30,13 +30,14 @@ def create_training_pipeline_image_classification_sample(
# Initialize client that will be used to create and send requests.
# This client only needs to be created once, and can be reused for multiple requests.
client = aiplatform.gapic.PipelineServiceClient(client_options=client_options)
training_task_inputs_dict = {
"multiLabel": True,
"modelType": "CLOUD",
"budgetMilliNodeHours": 8000,
"disableEarlyStopping": False,
}
training_task_inputs = json_format.ParseDict(training_task_inputs_dict, Value())

icn_training_inputs = definition.AutoMlImageClassificationInputs(
Copy link
Contributor

Choose a reason for hiding this comment

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

Note that here we introduce an inconsistent style of using instance of a particular python class, whereas the rest of the sample is in python dicts. I think technically we could keep using python dict here too (with only the change of camelCase to snake_case for dict keys).

@leahecole let us know if this is fine according to sample style guidelines. Several more samples will be updated and follow the same pattern as the two samples of this PR.

Copy link
Contributor

Choose a reason for hiding this comment

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

Instances are nicer b/c IDEs can offer more assistance with field names and types. Dicts are sometimes easier to pass around though.

I don't think we currently mandate one style or the other in the style guide. There is a mix in the currently published samples.

Copy link
Contributor

Choose a reason for hiding this comment

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

General consensus amongst the owners was preference to have generated classes for API resources - having spell check and autocomplete as well as knowing where to look in reference docs is helpful

If you're using a user-defined object with arbitrary properties, a dict may be simpler.

Copy link
Contributor

Choose a reason for hiding this comment

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

This may be a place where we need to avoid imposing consistency across the board. For example explicitly construct instances can make certain samples (e.g. https://github.com/googleapis/python-aiplatform/blob/master/samples/snippets/create_hyperparameter_tuning_job_python_package_sample.py) much more difficult to read than dicts, and in some cases we are forced to use instances (e.g. https://github.com/googleapis/python-aiplatform/blob/master/samples/snippets/upload_model_explain_image_managed_container_sample.py) because of dependency issues.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Just to clarify: I'm keeping this sample as-is?

multi_label=True,
model_type=ModelType.CLOUD,
budget_milli_node_hours=8000,
disable_early_stopping=False
)
training_task_inputs = icn_training_inputs.to_value()

training_pipeline = {
"display_name": display_name,
Expand Down
29 changes: 17 additions & 12 deletions samples/snippets/predict_image_classification_sample.py
Expand Up @@ -16,8 +16,9 @@
import base64

from google.cloud import aiplatform
from google.protobuf import json_format
from google.protobuf.struct_pb2 import Value
from google.cloud.aiplatform.v1beta1.schema.predict import instance
from google.cloud.aiplatform.v1beta1.schema.predict import params
from google.cloud.aiplatform.v1beta1.schema.predict import prediction


def predict_image_classification_sample(
Expand All @@ -36,25 +37,29 @@ def predict_image_classification_sample(

# The format of each instance should conform to the deployed model's prediction input schema.
encoded_content = base64.b64encode(file_content).decode("utf-8")
instance_dict = {"content": encoded_content}

instance = json_format.ParseDict(instance_dict, Value())
instances = [instance]
# See gs://google-cloud-aiplatform/schema/predict/params/image_classification_1.0.0.yaml for the format of the parameters.
parameters_dict = {"confidence_threshold": 0.5, "max_predictions": 5}
parameters = json_format.ParseDict(parameters_dict, Value())
instance_obj = instance.ImageClassificationPredictionInstance(
content=encoded_content)

instance_val = instance_obj.to_value()
instances = [instance_val]

params_obj = params.ImageClassificationPredictionParams(
confidence_threshold=0.5, max_predictions=5)

endpoint = client.endpoint_path(
project=project, location=location, endpoint=endpoint_id
)
response = client.predict(
endpoint=endpoint, instances=instances, parameters=parameters
endpoint=endpoint, instances=instances, parameters=params_obj
)
print("response")
print(" deployed_model_id:", response.deployed_model_id)
print("\tdeployed_model_id:", response.deployed_model_id)
# See gs://google-cloud-aiplatform/schema/predict/prediction/classification.yaml for the format of the predictions.
predictions = response.predictions
for prediction in predictions:
print(" prediction:", dict(prediction))
for prediction_ in predictions:
prediction_obj = prediction.ClassificationPredictionResult.from_map(prediction_)
print(prediction_obj)


# [END aiplatform_predict_image_classification_sample]
Expand Up @@ -31,4 +31,4 @@ def test_ucaip_generated_predict_image_classification_sample(capsys):
)

out, _ = capsys.readouterr()
assert 'string_value: "daisy"' in out
assert 'deployed_model_id:' in out
Copy link
Contributor

Choose a reason for hiding this comment

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

Why did this test case change? Is there any chance this could lead to a false positive if no model ID is returned? Or will the sample straight up fail before it gets to this print statement?

Copy link
Contributor Author

@telpirion telpirion Dec 16, 2020

Choose a reason for hiding this comment

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

A couple of reasons, but biggest of them: we want to avoid testing for the output of models, since retraining can cause the predictions to change.

No, a model ID must be returned as part of the online prediction--you can't have a prediction without a model! The sample will fail if you attempt to send a prediction request to an endpoint that has no model deployed to it.