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

Add ability to customize examples for custom AI platform models #2268

Merged
merged 1 commit into from
May 23, 2019

Conversation

jameswex
Copy link
Contributor

  • Motivation for features / changes

Cloud AI Platform models can have custom prediction code that takes in examples in any format the modeler wants. But WIT requires examples as lists, dicts, or example protos by design. So add the ability to transform examples to the custom format for cloud users that need this capability.

  • Technical description of changes
  • Add adjust_example function argument to set_ai_platform_model methods, similar to existing adjust_prediction argument.
  • Call adjust_example before sending to the model if that argument is passed.
  • Screenshots of UI changes

No UI changes.

  • Detailed steps to verify changes work correctly (as executed by you)

Ran colab with custom cloud model that needs this capability and verified it works.

@jameswex
Copy link
Contributor Author

@tolga-b please review

Copy link
Contributor

@tolga-b tolga-b left a comment

Choose a reason for hiding this comment

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

It looks good James. One comment for future, now we have many modes (set_ai_platform_model, set_custom_predict_fn, set_estimator etc.), I think it would be good to implement a clear_config function. For example, if someone sets aip then sets estimator it may cause unintended behavior depending on the order of if statements where config is being parsed. This is currently done is some functions (e.g. self.delete('estimator_and_spec') in set_custom_predict_fn).

@jameswex jameswex requested a review from stephanwlee May 22, 2019 23:26
@jameswex
Copy link
Contributor Author

@tolga-b Agree we can approach that in a future PR

@stephanwlee please review :)

@jameswex jameswex merged commit cc8e59f into tensorflow:master May 23, 2019
@jameswex jameswex deleted the caip branch May 23, 2019 01:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants