-
Notifications
You must be signed in to change notification settings - Fork 350
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: Support uploading local models #779
feat: Support uploading local models #779
Conversation
354f23e
to
d346c1d
Compare
It can properly upload both files and directories.
Switched the stage_local_data_in_gcs function to use upload_to_gcs
…arn and Tensorflow
bfec598
to
8f61efb
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please add unit tests.
Test examples with mock Model service: https://github.com/googleapis/python-aiplatform/blob/main/tests/unit/aiplatform/test_models.py#L500
Testing with mock GCS: https://github.com/googleapis/python-aiplatform/blob/main/tests/unit/aiplatform/test_training_jobs.py#L311
Would also prefer to have one integration test as these methods use both GCS and Vertex. Here's an example: https://github.com/googleapis/python-aiplatform/blob/main/tests/system/aiplatform/test_e2e_tabular.py#L50
Let me know if you have any questions.
Thanks for this PR, Alexey!
…stead of error message
TemporaryDirectory returns object with `.name` when called directly, but returns a path string when used as a context.
The buckets that we create are regional. This prevents errors when some service required regional bucket. E.g. "FailedPrecondition: 400 The Cloud Storage bucket of `gs://...` is in location `us`. It must be in the same regional location as the service location `us-central1`." We are making the bucket name region-specific since the bucket is regional.
We cannot clean up the directory immediately after calling Model.upload since that call may be asynchronous and return before the model file has been read. The temporary data will be automatically cleaned up by the system later.
Thank you for the great pointers for the tests. I've added unit tests for the new Please take a look. |
52fb340
to
73a4868
Compare
b466cb2
to
9138036
Compare
The `aiplatform.helpers.get_prebuilt_prediction_container_uri` does not support future framework versions yet. See googleapis#779 (comment)
…age_local_data_in_gcs
…Simplified the component after my Vertex SDK fixes were merged Some of my Vertex SDK fixes: googleapis/python-aiplatform#779 googleapis/python-aiplatform#882 googleapis/python-aiplatform#943 googleapis/python-aiplatform#997
… - Simplified the component after my Vertex SDK fixes were merged Some of my Vertex SDK fixes: googleapis/python-aiplatform#779 googleapis/python-aiplatform#882 googleapis/python-aiplatform#943 googleapis/python-aiplatform#997
…- Simplified the component after my Vertex SDK fixes were merged Some of my Vertex SDK fixes: googleapis/python-aiplatform#779 googleapis/python-aiplatform#882 googleapis/python-aiplatform#943 googleapis/python-aiplatform#997
…Simplified the component after my Vertex SDK fixes were merged Some of my Vertex SDK fixes: googleapis/python-aiplatform#779 googleapis/python-aiplatform#882 googleapis/python-aiplatform#943 googleapis/python-aiplatform#997
… - Simplified the component after my Vertex SDK fixes were merged Some of my Vertex SDK fixes: googleapis/python-aiplatform#779 googleapis/python-aiplatform#882 googleapis/python-aiplatform#943 googleapis/python-aiplatform#997
…- Simplified the component after my Vertex SDK fixes were merged Some of my Vertex SDK fixes: googleapis/python-aiplatform#779 googleapis/python-aiplatform#882 googleapis/python-aiplatform#943 googleapis/python-aiplatform#997
Added framework-specific model uploaders for XGBoost, Scikit-learn and Tensorflow
This PR adds framework-specific upload functions to the
Model
class:upload_scikit_learn_model_file
upload_xgboost_model_file
upload_tensorflow_saved_model
Here is what these functions do for the user:
This PR also augments the
Model.upload
function such that whenartifact_uri
points to a local model file or directory, the model is automatically staged in Google Cloud Storage as required by theModel.upload
API. TheModel.upload
function now also checks the model files for correctness.This change does not add any new dependencies.
Example usage:
Prediction example:
Output:
Thank you for opening a Pull Request! Before submitting your PR, there are a few things you can do to make sure it goes smoothly:
Fixes #<issue_number_goes_here> 🦕