globocom / gcloud-utils

Global package for Cloud Management in Python
https://gcloud-utils.readthedocs.io/en/latest/
Apache License 2.0
16 stars 23 forks source link

adding funciont to export models and predictions with json data #25

Closed GMVargass closed 4 years ago

GMVargass commented 4 years ago

Was that what you expected?

Close #13

alexandreyy commented 4 years ago

@GMVargass The export function does not seem right. It should export a trained model and register it in the ml engine platform for online prediction. Could you check if your code is working to export a TensorFlow model and execute the online prediction?

GMVargass commented 4 years ago

@alexandreyy The function exports any template, in the previous version it copied it to the folder where the job saves output, now only exports.

alexandreyy commented 4 years ago

@GMVargass I checked the documentation, we do support XGBoost and scikit-learn for online prediction. However, TensorFlow uses a different export that does not use pickle. Could you check that?

GMVargass commented 4 years ago

♻️ @alexandreyy

dmvieira commented 4 years ago

I don't know if a good strategy is use some kind of conversor like this https://github.com/microsoft/MMdnn/blob/master/README.md with this pattern: https://onnx.ai/getting-started

GMVargass commented 4 years ago

@dmvieira @alexandreyy So you guys have to decide what you want to do because each one is asking for something and are going into contradiction.

dmvieira commented 4 years ago

What do you think about support pickle models and open an issue to discuss about tensorflow integration @alexandreyy ?... Because with this pattern we need to be coupled with tensorflow

alexandreyy commented 4 years ago

@dmvieira @GMVargass The online prediction only supports XGBoost, Scikit-learn and Tensorflow. We are limited by the API restrictions to add the PyTorch support.

I think we can omit the tensorflow in the requirements and move the tensorflow import to the export function. If the user is exporting a tensorflow model, it means that he has already trained his model using tensorflow and it is already configured in his environment. @dmvieira What do you think about this solution?

Anyway, we can do this in another issue. @GMVargass, you can remove the export_tf_model from this PR. the export function for tensorflow is not correct yet. I have already used this API and I know it is very boring to make it works. The signatures should be a parameter for the function. I will continue the review of this PR and you can proceed with your changes.

dmvieira commented 4 years ago

We discuss later! Agree with new issue

GMVargass commented 4 years ago

♻️ @alexandreyy Pickle is part of the standard library in Python for a very long time now so there is no need to install it via pip. But, i put it into requirements.txt anyway.

alexandreyy commented 4 years ago

♻️ @alexandreyy Pickle is part of the standard library in Python for a very long time now so there is no need to install it via pip. But, i put it into requirements.txt anyway.

Ok. We can omit it.

GMVargass commented 4 years ago

♻️ @alexandreyy

GMVargass commented 4 years ago

♻️ @alexandreyy

alexandreyy commented 4 years ago

Shipped.