You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@submarine.apache.org by GitBox <gi...@apache.org> on 2021/08/31 06:59:23 UTC
[GitHub] [submarine] jeff-901 opened a new pull request #733: SUBMARINE-1000. Add submarine save model method
jeff-901 opened a new pull request #733:
URL: https://github.com/apache/submarine/pull/733
### What is this PR for?
Create submarine save model method.
### What type of PR is it?
Feature
### Todos
* [x] - Create repository
* [x] - Create save method for pytorch and tensorflow
* [ ] - Create registered model when saving.
### What is the Jira issue?
https://issues.apache.org/jira/browse/SUBMARINE-1000
### How should this be tested?
Call the save_model_submarine function and check the artifact by minio ui.
### Screenshots (if appropriate)
### Questions:
* Do the license files need updating? No
* Are there breaking changes for older versions? No
* Does this need new documentation? Yes
--
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: commits-unsubscribe@submarine.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [submarine] pingsutw commented on pull request #733: SUBMARINE-1000. Add submarine save model method
Posted by GitBox <gi...@apache.org>.
pingsutw commented on pull request #733:
URL: https://github.com/apache/submarine/pull/733#issuecomment-913984700
@jeff-901 Could you fix the conflict? thanks.
--
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: commits-unsubscribe@submarine.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [submarine] KUAN-HSUN-LI commented on a change in pull request #733: SUBMARINE-1000. Add submarine save model method
Posted by GitBox <gi...@apache.org>.
KUAN-HSUN-LI commented on a change in pull request #733:
URL: https://github.com/apache/submarine/pull/733#discussion_r699178761
##########
File path: dev-support/docker-images/submarine/create_bucket.sh
##########
@@ -0,0 +1,33 @@
+#!/usr/bin/env bash
+#
+# 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.
+
+set -euo pipefail
+
+S3_ENDPOINT_URL="http://submarine-minio-service:9000"
+AWS_ACCESS_KEY_ID="submarine_minio"
+AWS_SECRET_ACCESS_KEY="submarine_minio"
+
+/bin/bash -c "sleep 60; ./mc config host add minio ${S3_ENDPOINT_URL} ${AWS_ACCESS_KEY_ID} ${AWS_SECRET_ACCESS_KEY}"
Review comment:
why sleep 60?
##########
File path: submarine-sdk/pysubmarine/submarine/models/client.py
##########
@@ -101,6 +107,39 @@ def save_model(self,
else:
raise MlflowException("No valid type of model has been matched")
+ def save_model_submarine(self,
+ model_type,
+ model,
+ artifact_path,
+ registered_model_name=None):
+ pattern = r"[0-9A-Za-z][0-9A-Za-z-_]*[0-9A-Za-z]|[0-9A-Za-z]"
+ if not re.fullmatch(pattern, artifact_path):
+ raise Exception(
+ "Artifact_path must only contains numbers, characters, hyphen and underscore. \
+ Artifact_path must starts and ends with numbers or characters.")
+ local_dir = os.path.join(self.root, artifact_path)
+ if os.path.exists(local_dir):
+ version = len(os.listdir(local_dir)) + 1
+ else:
+ version = 1
+ artifact_path = os.path.join(artifact_path, str(version))
+ local_dir = os.path.join(local_dir, str(version))
+ os.makedirs(local_dir)
+ if model_type == "pytorch":
+ import submarine.models.pytorch
+
+ submarine.models.pytorch.save_model(model, local_dir)
+ elif model_type == "tensorflow":
+ import submarine.models.tensorflow
Review comment:
useless import?
##########
File path: submarine-sdk/pysubmarine/submarine/models/client.py
##########
@@ -101,6 +107,39 @@ def save_model(self,
else:
raise MlflowException("No valid type of model has been matched")
+ def save_model_submarine(self,
+ model_type,
+ model,
+ artifact_path,
+ registered_model_name=None):
+ pattern = r"[0-9A-Za-z][0-9A-Za-z-_]*[0-9A-Za-z]|[0-9A-Za-z]"
+ if not re.fullmatch(pattern, artifact_path):
+ raise Exception(
+ "Artifact_path must only contains numbers, characters, hyphen and underscore. \
+ Artifact_path must starts and ends with numbers or characters.")
+ local_dir = os.path.join(self.root, artifact_path)
+ if os.path.exists(local_dir):
+ version = len(os.listdir(local_dir)) + 1
+ else:
+ version = 1
+ artifact_path = os.path.join(artifact_path, str(version))
+ local_dir = os.path.join(local_dir, str(version))
+ os.makedirs(local_dir)
+ if model_type == "pytorch":
+ import submarine.models.pytorch
Review comment:
useless import?
##########
File path: submarine-sdk/pysubmarine/submarine/models/client.py
##########
@@ -101,6 +107,39 @@ def save_model(self,
else:
raise MlflowException("No valid type of model has been matched")
+ def save_model_submarine(self,
+ model_type,
+ model,
+ artifact_path,
+ registered_model_name=None):
+ pattern = r"[0-9A-Za-z][0-9A-Za-z-_]*[0-9A-Za-z]|[0-9A-Za-z]"
+ if not re.fullmatch(pattern, artifact_path):
+ raise Exception(
+ "Artifact_path must only contains numbers, characters, hyphen and underscore. \
+ Artifact_path must starts and ends with numbers or characters.")
+ local_dir = os.path.join(self.root, artifact_path)
+ if os.path.exists(local_dir):
+ version = len(os.listdir(local_dir)) + 1
+ else:
+ version = 1
Review comment:
version check must use remote folder info
##########
File path: submarine-sdk/pysubmarine/submarine/models/client.py
##########
@@ -101,6 +107,39 @@ def save_model(self,
else:
raise MlflowException("No valid type of model has been matched")
+ def save_model_submarine(self,
+ model_type,
+ model,
+ artifact_path,
+ registered_model_name=None):
+ pattern = r"[0-9A-Za-z][0-9A-Za-z-_]*[0-9A-Za-z]|[0-9A-Za-z]"
+ if not re.fullmatch(pattern, artifact_path):
+ raise Exception(
+ "Artifact_path must only contains numbers, characters, hyphen and underscore. \
+ Artifact_path must starts and ends with numbers or characters.")
+ local_dir = os.path.join(self.root, artifact_path)
+ if os.path.exists(local_dir):
+ version = len(os.listdir(local_dir)) + 1
+ else:
+ version = 1
+ artifact_path = os.path.join(artifact_path, str(version))
+ local_dir = os.path.join(local_dir, str(version))
+ os.makedirs(local_dir)
+ if model_type == "pytorch":
+ import submarine.models.pytorch
+
+ submarine.models.pytorch.save_model(model, local_dir)
+ elif model_type == "tensorflow":
+ import submarine.models.tensorflow
+
+ submarine.models.tensorflow.save_model(model, local_dir)
+ else:
+ raise Exception(
+ "No valid type of model has been matched to {}".format(
+ model_type))
+ self.artifact_repo.log_artifacts(local_dir, artifact_path)
Review comment:
require to delete the local model
--
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: commits-unsubscribe@submarine.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [submarine] jeff-901 commented on pull request #733: SUBMARINE-1000. Add submarine save model method
Posted by GitBox <gi...@apache.org>.
jeff-901 commented on pull request #733:
URL: https://github.com/apache/submarine/pull/733#issuecomment-908955932
@KUAN-HSUN-LI helps me review the code please.
--
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: commits-unsubscribe@submarine.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [submarine] jeff-901 commented on a change in pull request #733: SUBMARINE-1000. Add submarine save model method
Posted by GitBox <gi...@apache.org>.
jeff-901 commented on a change in pull request #733:
URL: https://github.com/apache/submarine/pull/733#discussion_r701854631
##########
File path: submarine-sdk/pysubmarine/submarine/artifacts/Repository.py
##########
@@ -0,0 +1,87 @@
+# Licensed to the Apache Software Foundation (ASF) under one or more
Review comment:
ok.
--
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: commits-unsubscribe@submarine.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [submarine] KUAN-HSUN-LI commented on a change in pull request #733: SUBMARINE-1000. Add submarine save model method
Posted by GitBox <gi...@apache.org>.
KUAN-HSUN-LI commented on a change in pull request #733:
URL: https://github.com/apache/submarine/pull/733#discussion_r699178761
##########
File path: dev-support/docker-images/submarine/create_bucket.sh
##########
@@ -0,0 +1,33 @@
+#!/usr/bin/env bash
+#
+# 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.
+
+set -euo pipefail
+
+S3_ENDPOINT_URL="http://submarine-minio-service:9000"
+AWS_ACCESS_KEY_ID="submarine_minio"
+AWS_SECRET_ACCESS_KEY="submarine_minio"
+
+/bin/bash -c "sleep 60; ./mc config host add minio ${S3_ENDPOINT_URL} ${AWS_ACCESS_KEY_ID} ${AWS_SECRET_ACCESS_KEY}"
Review comment:
why sleep 60?
##########
File path: submarine-sdk/pysubmarine/submarine/models/client.py
##########
@@ -101,6 +107,39 @@ def save_model(self,
else:
raise MlflowException("No valid type of model has been matched")
+ def save_model_submarine(self,
+ model_type,
+ model,
+ artifact_path,
+ registered_model_name=None):
+ pattern = r"[0-9A-Za-z][0-9A-Za-z-_]*[0-9A-Za-z]|[0-9A-Za-z]"
+ if not re.fullmatch(pattern, artifact_path):
+ raise Exception(
+ "Artifact_path must only contains numbers, characters, hyphen and underscore. \
+ Artifact_path must starts and ends with numbers or characters.")
+ local_dir = os.path.join(self.root, artifact_path)
+ if os.path.exists(local_dir):
+ version = len(os.listdir(local_dir)) + 1
+ else:
+ version = 1
+ artifact_path = os.path.join(artifact_path, str(version))
+ local_dir = os.path.join(local_dir, str(version))
+ os.makedirs(local_dir)
+ if model_type == "pytorch":
+ import submarine.models.pytorch
+
+ submarine.models.pytorch.save_model(model, local_dir)
+ elif model_type == "tensorflow":
+ import submarine.models.tensorflow
Review comment:
useless import?
##########
File path: submarine-sdk/pysubmarine/submarine/models/client.py
##########
@@ -101,6 +107,39 @@ def save_model(self,
else:
raise MlflowException("No valid type of model has been matched")
+ def save_model_submarine(self,
+ model_type,
+ model,
+ artifact_path,
+ registered_model_name=None):
+ pattern = r"[0-9A-Za-z][0-9A-Za-z-_]*[0-9A-Za-z]|[0-9A-Za-z]"
+ if not re.fullmatch(pattern, artifact_path):
+ raise Exception(
+ "Artifact_path must only contains numbers, characters, hyphen and underscore. \
+ Artifact_path must starts and ends with numbers or characters.")
+ local_dir = os.path.join(self.root, artifact_path)
+ if os.path.exists(local_dir):
+ version = len(os.listdir(local_dir)) + 1
+ else:
+ version = 1
+ artifact_path = os.path.join(artifact_path, str(version))
+ local_dir = os.path.join(local_dir, str(version))
+ os.makedirs(local_dir)
+ if model_type == "pytorch":
+ import submarine.models.pytorch
Review comment:
useless import?
##########
File path: submarine-sdk/pysubmarine/submarine/models/client.py
##########
@@ -101,6 +107,39 @@ def save_model(self,
else:
raise MlflowException("No valid type of model has been matched")
+ def save_model_submarine(self,
+ model_type,
+ model,
+ artifact_path,
+ registered_model_name=None):
+ pattern = r"[0-9A-Za-z][0-9A-Za-z-_]*[0-9A-Za-z]|[0-9A-Za-z]"
+ if not re.fullmatch(pattern, artifact_path):
+ raise Exception(
+ "Artifact_path must only contains numbers, characters, hyphen and underscore. \
+ Artifact_path must starts and ends with numbers or characters.")
+ local_dir = os.path.join(self.root, artifact_path)
+ if os.path.exists(local_dir):
+ version = len(os.listdir(local_dir)) + 1
+ else:
+ version = 1
Review comment:
version check must use remote folder info
##########
File path: submarine-sdk/pysubmarine/submarine/models/client.py
##########
@@ -101,6 +107,39 @@ def save_model(self,
else:
raise MlflowException("No valid type of model has been matched")
+ def save_model_submarine(self,
+ model_type,
+ model,
+ artifact_path,
+ registered_model_name=None):
+ pattern = r"[0-9A-Za-z][0-9A-Za-z-_]*[0-9A-Za-z]|[0-9A-Za-z]"
+ if not re.fullmatch(pattern, artifact_path):
+ raise Exception(
+ "Artifact_path must only contains numbers, characters, hyphen and underscore. \
+ Artifact_path must starts and ends with numbers or characters.")
+ local_dir = os.path.join(self.root, artifact_path)
+ if os.path.exists(local_dir):
+ version = len(os.listdir(local_dir)) + 1
+ else:
+ version = 1
+ artifact_path = os.path.join(artifact_path, str(version))
+ local_dir = os.path.join(local_dir, str(version))
+ os.makedirs(local_dir)
+ if model_type == "pytorch":
+ import submarine.models.pytorch
+
+ submarine.models.pytorch.save_model(model, local_dir)
+ elif model_type == "tensorflow":
+ import submarine.models.tensorflow
+
+ submarine.models.tensorflow.save_model(model, local_dir)
+ else:
+ raise Exception(
+ "No valid type of model has been matched to {}".format(
+ model_type))
+ self.artifact_repo.log_artifacts(local_dir, artifact_path)
Review comment:
require to delete the local model
--
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: commits-unsubscribe@submarine.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [submarine] jeff-901 commented on a change in pull request #733: SUBMARINE-1000. Add submarine save model method
Posted by GitBox <gi...@apache.org>.
jeff-901 commented on a change in pull request #733:
URL: https://github.com/apache/submarine/pull/733#discussion_r699250116
##########
File path: submarine-sdk/pysubmarine/submarine/models/client.py
##########
@@ -101,6 +107,39 @@ def save_model(self,
else:
raise MlflowException("No valid type of model has been matched")
+ def save_model_submarine(self,
+ model_type,
+ model,
+ artifact_path,
+ registered_model_name=None):
+ pattern = r"[0-9A-Za-z][0-9A-Za-z-_]*[0-9A-Za-z]|[0-9A-Za-z]"
+ if not re.fullmatch(pattern, artifact_path):
+ raise Exception(
+ "Artifact_path must only contains numbers, characters, hyphen and underscore. \
+ Artifact_path must starts and ends with numbers or characters.")
+ local_dir = os.path.join(self.root, artifact_path)
+ if os.path.exists(local_dir):
+ version = len(os.listdir(local_dir)) + 1
+ else:
+ version = 1
+ artifact_path = os.path.join(artifact_path, str(version))
+ local_dir = os.path.join(local_dir, str(version))
+ os.makedirs(local_dir)
+ if model_type == "pytorch":
+ import submarine.models.pytorch
+
+ submarine.models.pytorch.save_model(model, local_dir)
+ elif model_type == "tensorflow":
+ import submarine.models.tensorflow
+
+ submarine.models.tensorflow.save_model(model, local_dir)
+ else:
+ raise Exception(
+ "No valid type of model has been matched to {}".format(
+ model_type))
+ self.artifact_repo.log_artifacts(local_dir, artifact_path)
Review comment:
That sounds correct. I will delete all the local model.
--
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: commits-unsubscribe@submarine.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [submarine] jeff-901 commented on a change in pull request #733: SUBMARINE-1000. Add submarine save model method
Posted by GitBox <gi...@apache.org>.
jeff-901 commented on a change in pull request #733:
URL: https://github.com/apache/submarine/pull/733#discussion_r699249324
##########
File path: submarine-sdk/pysubmarine/submarine/models/client.py
##########
@@ -101,6 +107,39 @@ def save_model(self,
else:
raise MlflowException("No valid type of model has been matched")
+ def save_model_submarine(self,
+ model_type,
+ model,
+ artifact_path,
+ registered_model_name=None):
+ pattern = r"[0-9A-Za-z][0-9A-Za-z-_]*[0-9A-Za-z]|[0-9A-Za-z]"
+ if not re.fullmatch(pattern, artifact_path):
+ raise Exception(
+ "Artifact_path must only contains numbers, characters, hyphen and underscore. \
+ Artifact_path must starts and ends with numbers or characters.")
+ local_dir = os.path.join(self.root, artifact_path)
+ if os.path.exists(local_dir):
+ version = len(os.listdir(local_dir)) + 1
+ else:
+ version = 1
+ artifact_path = os.path.join(artifact_path, str(version))
+ local_dir = os.path.join(local_dir, str(version))
+ os.makedirs(local_dir)
+ if model_type == "pytorch":
+ import submarine.models.pytorch
Review comment:
No. I use the function on 131 line.
##########
File path: submarine-sdk/pysubmarine/submarine/models/client.py
##########
@@ -101,6 +107,39 @@ def save_model(self,
else:
raise MlflowException("No valid type of model has been matched")
+ def save_model_submarine(self,
+ model_type,
+ model,
+ artifact_path,
+ registered_model_name=None):
+ pattern = r"[0-9A-Za-z][0-9A-Za-z-_]*[0-9A-Za-z]|[0-9A-Za-z]"
+ if not re.fullmatch(pattern, artifact_path):
+ raise Exception(
+ "Artifact_path must only contains numbers, characters, hyphen and underscore. \
+ Artifact_path must starts and ends with numbers or characters.")
+ local_dir = os.path.join(self.root, artifact_path)
+ if os.path.exists(local_dir):
+ version = len(os.listdir(local_dir)) + 1
+ else:
+ version = 1
+ artifact_path = os.path.join(artifact_path, str(version))
+ local_dir = os.path.join(local_dir, str(version))
+ os.makedirs(local_dir)
+ if model_type == "pytorch":
+ import submarine.models.pytorch
+
+ submarine.models.pytorch.save_model(model, local_dir)
+ elif model_type == "tensorflow":
+ import submarine.models.tensorflow
Review comment:
No. I use the function on 135 line.
##########
File path: submarine-sdk/pysubmarine/submarine/models/client.py
##########
@@ -101,6 +107,39 @@ def save_model(self,
else:
raise MlflowException("No valid type of model has been matched")
+ def save_model_submarine(self,
+ model_type,
+ model,
+ artifact_path,
+ registered_model_name=None):
+ pattern = r"[0-9A-Za-z][0-9A-Za-z-_]*[0-9A-Za-z]|[0-9A-Za-z]"
+ if not re.fullmatch(pattern, artifact_path):
+ raise Exception(
+ "Artifact_path must only contains numbers, characters, hyphen and underscore. \
+ Artifact_path must starts and ends with numbers or characters.")
+ local_dir = os.path.join(self.root, artifact_path)
+ if os.path.exists(local_dir):
+ version = len(os.listdir(local_dir)) + 1
+ else:
+ version = 1
Review comment:
That sounds good. I will edit it.
##########
File path: submarine-sdk/pysubmarine/submarine/models/client.py
##########
@@ -101,6 +107,39 @@ def save_model(self,
else:
raise MlflowException("No valid type of model has been matched")
+ def save_model_submarine(self,
+ model_type,
+ model,
+ artifact_path,
+ registered_model_name=None):
+ pattern = r"[0-9A-Za-z][0-9A-Za-z-_]*[0-9A-Za-z]|[0-9A-Za-z]"
+ if not re.fullmatch(pattern, artifact_path):
+ raise Exception(
+ "Artifact_path must only contains numbers, characters, hyphen and underscore. \
+ Artifact_path must starts and ends with numbers or characters.")
+ local_dir = os.path.join(self.root, artifact_path)
+ if os.path.exists(local_dir):
+ version = len(os.listdir(local_dir)) + 1
+ else:
+ version = 1
+ artifact_path = os.path.join(artifact_path, str(version))
+ local_dir = os.path.join(local_dir, str(version))
+ os.makedirs(local_dir)
+ if model_type == "pytorch":
+ import submarine.models.pytorch
+
+ submarine.models.pytorch.save_model(model, local_dir)
+ elif model_type == "tensorflow":
+ import submarine.models.tensorflow
+
+ submarine.models.tensorflow.save_model(model, local_dir)
+ else:
+ raise Exception(
+ "No valid type of model has been matched to {}".format(
+ model_type))
+ self.artifact_repo.log_artifacts(local_dir, artifact_path)
Review comment:
That sounds correct. I will delete all the local model.
--
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: commits-unsubscribe@submarine.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [submarine] jeff-901 commented on a change in pull request #733: SUBMARINE-1000. Add submarine save model method
Posted by GitBox <gi...@apache.org>.
jeff-901 commented on a change in pull request #733:
URL: https://github.com/apache/submarine/pull/733#discussion_r701861629
##########
File path: dev-support/docker-images/submarine/create_bucket.sh
##########
@@ -0,0 +1,33 @@
+#!/usr/bin/env bash
+#
+# 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.
+
+set -euo pipefail
+
+S3_ENDPOINT_URL="http://submarine-minio-service:9000"
+AWS_ACCESS_KEY_ID="submarine_minio"
+AWS_SECRET_ACCESS_KEY="submarine_minio"
+
+/bin/bash -c "sleep 60; mc config host add minio ${S3_ENDPOINT_URL} ${AWS_ACCESS_KEY_ID} ${AWS_SECRET_ACCESS_KEY}"
Review comment:
This is for waiting the minio pod to setup. I could add a comment.
--
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: commits-unsubscribe@submarine.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [submarine] jeff-901 commented on a change in pull request #733: SUBMARINE-1000. Add submarine save model method
Posted by GitBox <gi...@apache.org>.
jeff-901 commented on a change in pull request #733:
URL: https://github.com/apache/submarine/pull/733#discussion_r699249673
##########
File path: submarine-sdk/pysubmarine/submarine/models/client.py
##########
@@ -101,6 +107,39 @@ def save_model(self,
else:
raise MlflowException("No valid type of model has been matched")
+ def save_model_submarine(self,
+ model_type,
+ model,
+ artifact_path,
+ registered_model_name=None):
+ pattern = r"[0-9A-Za-z][0-9A-Za-z-_]*[0-9A-Za-z]|[0-9A-Za-z]"
+ if not re.fullmatch(pattern, artifact_path):
+ raise Exception(
+ "Artifact_path must only contains numbers, characters, hyphen and underscore. \
+ Artifact_path must starts and ends with numbers or characters.")
+ local_dir = os.path.join(self.root, artifact_path)
+ if os.path.exists(local_dir):
+ version = len(os.listdir(local_dir)) + 1
+ else:
+ version = 1
Review comment:
That sounds good. I will edit it.
--
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: commits-unsubscribe@submarine.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [submarine] jeff-901 commented on pull request #733: SUBMARINE-1000. Add submarine save model method
Posted by GitBox <gi...@apache.org>.
jeff-901 commented on pull request #733:
URL: https://github.com/apache/submarine/pull/733#issuecomment-908955932
@KUAN-HSUN-LI helps me review the code please.
--
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: commits-unsubscribe@submarine.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [submarine] jeff-901 commented on a change in pull request #733: SUBMARINE-1000. Add submarine save model method
Posted by GitBox <gi...@apache.org>.
jeff-901 commented on a change in pull request #733:
URL: https://github.com/apache/submarine/pull/733#discussion_r699249324
##########
File path: submarine-sdk/pysubmarine/submarine/models/client.py
##########
@@ -101,6 +107,39 @@ def save_model(self,
else:
raise MlflowException("No valid type of model has been matched")
+ def save_model_submarine(self,
+ model_type,
+ model,
+ artifact_path,
+ registered_model_name=None):
+ pattern = r"[0-9A-Za-z][0-9A-Za-z-_]*[0-9A-Za-z]|[0-9A-Za-z]"
+ if not re.fullmatch(pattern, artifact_path):
+ raise Exception(
+ "Artifact_path must only contains numbers, characters, hyphen and underscore. \
+ Artifact_path must starts and ends with numbers or characters.")
+ local_dir = os.path.join(self.root, artifact_path)
+ if os.path.exists(local_dir):
+ version = len(os.listdir(local_dir)) + 1
+ else:
+ version = 1
+ artifact_path = os.path.join(artifact_path, str(version))
+ local_dir = os.path.join(local_dir, str(version))
+ os.makedirs(local_dir)
+ if model_type == "pytorch":
+ import submarine.models.pytorch
Review comment:
No. I use the function on 131 line.
##########
File path: submarine-sdk/pysubmarine/submarine/models/client.py
##########
@@ -101,6 +107,39 @@ def save_model(self,
else:
raise MlflowException("No valid type of model has been matched")
+ def save_model_submarine(self,
+ model_type,
+ model,
+ artifact_path,
+ registered_model_name=None):
+ pattern = r"[0-9A-Za-z][0-9A-Za-z-_]*[0-9A-Za-z]|[0-9A-Za-z]"
+ if not re.fullmatch(pattern, artifact_path):
+ raise Exception(
+ "Artifact_path must only contains numbers, characters, hyphen and underscore. \
+ Artifact_path must starts and ends with numbers or characters.")
+ local_dir = os.path.join(self.root, artifact_path)
+ if os.path.exists(local_dir):
+ version = len(os.listdir(local_dir)) + 1
+ else:
+ version = 1
+ artifact_path = os.path.join(artifact_path, str(version))
+ local_dir = os.path.join(local_dir, str(version))
+ os.makedirs(local_dir)
+ if model_type == "pytorch":
+ import submarine.models.pytorch
+
+ submarine.models.pytorch.save_model(model, local_dir)
+ elif model_type == "tensorflow":
+ import submarine.models.tensorflow
Review comment:
No. I use the function on 135 line.
--
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: commits-unsubscribe@submarine.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [submarine] jeff-901 commented on pull request #733: SUBMARINE-1000. Add submarine save model method
Posted by GitBox <gi...@apache.org>.
jeff-901 commented on pull request #733:
URL: https://github.com/apache/submarine/pull/733#issuecomment-914254556
> @jeff-901 Could you fix the conflict? thanks.
Ok. I have fixed the conflict.
--
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: commits-unsubscribe@submarine.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [submarine] KUAN-HSUN-LI commented on a change in pull request #733: SUBMARINE-1000. Add submarine save model method
Posted by GitBox <gi...@apache.org>.
KUAN-HSUN-LI commented on a change in pull request #733:
URL: https://github.com/apache/submarine/pull/733#discussion_r699178761
##########
File path: dev-support/docker-images/submarine/create_bucket.sh
##########
@@ -0,0 +1,33 @@
+#!/usr/bin/env bash
+#
+# 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.
+
+set -euo pipefail
+
+S3_ENDPOINT_URL="http://submarine-minio-service:9000"
+AWS_ACCESS_KEY_ID="submarine_minio"
+AWS_SECRET_ACCESS_KEY="submarine_minio"
+
+/bin/bash -c "sleep 60; ./mc config host add minio ${S3_ENDPOINT_URL} ${AWS_ACCESS_KEY_ID} ${AWS_SECRET_ACCESS_KEY}"
Review comment:
why sleep 60?
##########
File path: submarine-sdk/pysubmarine/submarine/models/client.py
##########
@@ -101,6 +107,39 @@ def save_model(self,
else:
raise MlflowException("No valid type of model has been matched")
+ def save_model_submarine(self,
+ model_type,
+ model,
+ artifact_path,
+ registered_model_name=None):
+ pattern = r"[0-9A-Za-z][0-9A-Za-z-_]*[0-9A-Za-z]|[0-9A-Za-z]"
+ if not re.fullmatch(pattern, artifact_path):
+ raise Exception(
+ "Artifact_path must only contains numbers, characters, hyphen and underscore. \
+ Artifact_path must starts and ends with numbers or characters.")
+ local_dir = os.path.join(self.root, artifact_path)
+ if os.path.exists(local_dir):
+ version = len(os.listdir(local_dir)) + 1
+ else:
+ version = 1
+ artifact_path = os.path.join(artifact_path, str(version))
+ local_dir = os.path.join(local_dir, str(version))
+ os.makedirs(local_dir)
+ if model_type == "pytorch":
+ import submarine.models.pytorch
+
+ submarine.models.pytorch.save_model(model, local_dir)
+ elif model_type == "tensorflow":
+ import submarine.models.tensorflow
Review comment:
useless import?
##########
File path: submarine-sdk/pysubmarine/submarine/models/client.py
##########
@@ -101,6 +107,39 @@ def save_model(self,
else:
raise MlflowException("No valid type of model has been matched")
+ def save_model_submarine(self,
+ model_type,
+ model,
+ artifact_path,
+ registered_model_name=None):
+ pattern = r"[0-9A-Za-z][0-9A-Za-z-_]*[0-9A-Za-z]|[0-9A-Za-z]"
+ if not re.fullmatch(pattern, artifact_path):
+ raise Exception(
+ "Artifact_path must only contains numbers, characters, hyphen and underscore. \
+ Artifact_path must starts and ends with numbers or characters.")
+ local_dir = os.path.join(self.root, artifact_path)
+ if os.path.exists(local_dir):
+ version = len(os.listdir(local_dir)) + 1
+ else:
+ version = 1
+ artifact_path = os.path.join(artifact_path, str(version))
+ local_dir = os.path.join(local_dir, str(version))
+ os.makedirs(local_dir)
+ if model_type == "pytorch":
+ import submarine.models.pytorch
Review comment:
useless import?
##########
File path: submarine-sdk/pysubmarine/submarine/models/client.py
##########
@@ -101,6 +107,39 @@ def save_model(self,
else:
raise MlflowException("No valid type of model has been matched")
+ def save_model_submarine(self,
+ model_type,
+ model,
+ artifact_path,
+ registered_model_name=None):
+ pattern = r"[0-9A-Za-z][0-9A-Za-z-_]*[0-9A-Za-z]|[0-9A-Za-z]"
+ if not re.fullmatch(pattern, artifact_path):
+ raise Exception(
+ "Artifact_path must only contains numbers, characters, hyphen and underscore. \
+ Artifact_path must starts and ends with numbers or characters.")
+ local_dir = os.path.join(self.root, artifact_path)
+ if os.path.exists(local_dir):
+ version = len(os.listdir(local_dir)) + 1
+ else:
+ version = 1
Review comment:
version check must use remote folder info
##########
File path: submarine-sdk/pysubmarine/submarine/models/client.py
##########
@@ -101,6 +107,39 @@ def save_model(self,
else:
raise MlflowException("No valid type of model has been matched")
+ def save_model_submarine(self,
+ model_type,
+ model,
+ artifact_path,
+ registered_model_name=None):
+ pattern = r"[0-9A-Za-z][0-9A-Za-z-_]*[0-9A-Za-z]|[0-9A-Za-z]"
+ if not re.fullmatch(pattern, artifact_path):
+ raise Exception(
+ "Artifact_path must only contains numbers, characters, hyphen and underscore. \
+ Artifact_path must starts and ends with numbers or characters.")
+ local_dir = os.path.join(self.root, artifact_path)
+ if os.path.exists(local_dir):
+ version = len(os.listdir(local_dir)) + 1
+ else:
+ version = 1
+ artifact_path = os.path.join(artifact_path, str(version))
+ local_dir = os.path.join(local_dir, str(version))
+ os.makedirs(local_dir)
+ if model_type == "pytorch":
+ import submarine.models.pytorch
+
+ submarine.models.pytorch.save_model(model, local_dir)
+ elif model_type == "tensorflow":
+ import submarine.models.tensorflow
+
+ submarine.models.tensorflow.save_model(model, local_dir)
+ else:
+ raise Exception(
+ "No valid type of model has been matched to {}".format(
+ model_type))
+ self.artifact_repo.log_artifacts(local_dir, artifact_path)
Review comment:
require to delete the local model
--
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: commits-unsubscribe@submarine.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [submarine] jeff-901 commented on pull request #733: SUBMARINE-1000. Add submarine save model method
Posted by GitBox <gi...@apache.org>.
jeff-901 commented on pull request #733:
URL: https://github.com/apache/submarine/pull/733#issuecomment-912891724
@pingsutw I add the kubectl to wait for the minio pod ready. Repository uses the environmental variables. User can set the environmental variables by providing args when creating the ModelsClient.
--
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: commits-unsubscribe@submarine.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [submarine] pingsutw commented on a change in pull request #733: SUBMARINE-1000. Add submarine save model method
Posted by GitBox <gi...@apache.org>.
pingsutw commented on a change in pull request #733:
URL: https://github.com/apache/submarine/pull/733#discussion_r701508776
##########
File path: dev-support/docker-images/submarine/create_bucket.sh
##########
@@ -0,0 +1,33 @@
+#!/usr/bin/env bash
+#
+# 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.
+
+set -euo pipefail
+
+S3_ENDPOINT_URL="http://submarine-minio-service:9000"
+AWS_ACCESS_KEY_ID="submarine_minio"
+AWS_SECRET_ACCESS_KEY="submarine_minio"
+
+/bin/bash -c "sleep 60; mc config host add minio ${S3_ENDPOINT_URL} ${AWS_ACCESS_KEY_ID} ${AWS_SECRET_ACCESS_KEY}"
Review comment:
Why do we need to sleep 60 here? Could you add a comment?
##########
File path: submarine-sdk/pysubmarine/submarine/artifacts/Repository.py
##########
@@ -0,0 +1,87 @@
+# Licensed to the Apache Software Foundation (ASF) under one or more
Review comment:
The file name should be lowercase. `Repository.py` -> `repository.py`
--
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: commits-unsubscribe@submarine.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [submarine] asfgit closed pull request #733: SUBMARINE-1000. Add submarine save model method
Posted by GitBox <gi...@apache.org>.
asfgit closed pull request #733:
URL: https://github.com/apache/submarine/pull/733
--
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: commits-unsubscribe@submarine.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [submarine] jeff-901 commented on a change in pull request #733: SUBMARINE-1000. Add submarine save model method
Posted by GitBox <gi...@apache.org>.
jeff-901 commented on a change in pull request #733:
URL: https://github.com/apache/submarine/pull/733#discussion_r699249324
##########
File path: submarine-sdk/pysubmarine/submarine/models/client.py
##########
@@ -101,6 +107,39 @@ def save_model(self,
else:
raise MlflowException("No valid type of model has been matched")
+ def save_model_submarine(self,
+ model_type,
+ model,
+ artifact_path,
+ registered_model_name=None):
+ pattern = r"[0-9A-Za-z][0-9A-Za-z-_]*[0-9A-Za-z]|[0-9A-Za-z]"
+ if not re.fullmatch(pattern, artifact_path):
+ raise Exception(
+ "Artifact_path must only contains numbers, characters, hyphen and underscore. \
+ Artifact_path must starts and ends with numbers or characters.")
+ local_dir = os.path.join(self.root, artifact_path)
+ if os.path.exists(local_dir):
+ version = len(os.listdir(local_dir)) + 1
+ else:
+ version = 1
+ artifact_path = os.path.join(artifact_path, str(version))
+ local_dir = os.path.join(local_dir, str(version))
+ os.makedirs(local_dir)
+ if model_type == "pytorch":
+ import submarine.models.pytorch
Review comment:
No. I use the function on 131 line.
##########
File path: submarine-sdk/pysubmarine/submarine/models/client.py
##########
@@ -101,6 +107,39 @@ def save_model(self,
else:
raise MlflowException("No valid type of model has been matched")
+ def save_model_submarine(self,
+ model_type,
+ model,
+ artifact_path,
+ registered_model_name=None):
+ pattern = r"[0-9A-Za-z][0-9A-Za-z-_]*[0-9A-Za-z]|[0-9A-Za-z]"
+ if not re.fullmatch(pattern, artifact_path):
+ raise Exception(
+ "Artifact_path must only contains numbers, characters, hyphen and underscore. \
+ Artifact_path must starts and ends with numbers or characters.")
+ local_dir = os.path.join(self.root, artifact_path)
+ if os.path.exists(local_dir):
+ version = len(os.listdir(local_dir)) + 1
+ else:
+ version = 1
+ artifact_path = os.path.join(artifact_path, str(version))
+ local_dir = os.path.join(local_dir, str(version))
+ os.makedirs(local_dir)
+ if model_type == "pytorch":
+ import submarine.models.pytorch
+
+ submarine.models.pytorch.save_model(model, local_dir)
+ elif model_type == "tensorflow":
+ import submarine.models.tensorflow
Review comment:
No. I use the function on 135 line.
##########
File path: submarine-sdk/pysubmarine/submarine/models/client.py
##########
@@ -101,6 +107,39 @@ def save_model(self,
else:
raise MlflowException("No valid type of model has been matched")
+ def save_model_submarine(self,
+ model_type,
+ model,
+ artifact_path,
+ registered_model_name=None):
+ pattern = r"[0-9A-Za-z][0-9A-Za-z-_]*[0-9A-Za-z]|[0-9A-Za-z]"
+ if not re.fullmatch(pattern, artifact_path):
+ raise Exception(
+ "Artifact_path must only contains numbers, characters, hyphen and underscore. \
+ Artifact_path must starts and ends with numbers or characters.")
+ local_dir = os.path.join(self.root, artifact_path)
+ if os.path.exists(local_dir):
+ version = len(os.listdir(local_dir)) + 1
+ else:
+ version = 1
Review comment:
That sounds good. I will edit it.
##########
File path: submarine-sdk/pysubmarine/submarine/models/client.py
##########
@@ -101,6 +107,39 @@ def save_model(self,
else:
raise MlflowException("No valid type of model has been matched")
+ def save_model_submarine(self,
+ model_type,
+ model,
+ artifact_path,
+ registered_model_name=None):
+ pattern = r"[0-9A-Za-z][0-9A-Za-z-_]*[0-9A-Za-z]|[0-9A-Za-z]"
+ if not re.fullmatch(pattern, artifact_path):
+ raise Exception(
+ "Artifact_path must only contains numbers, characters, hyphen and underscore. \
+ Artifact_path must starts and ends with numbers or characters.")
+ local_dir = os.path.join(self.root, artifact_path)
+ if os.path.exists(local_dir):
+ version = len(os.listdir(local_dir)) + 1
+ else:
+ version = 1
+ artifact_path = os.path.join(artifact_path, str(version))
+ local_dir = os.path.join(local_dir, str(version))
+ os.makedirs(local_dir)
+ if model_type == "pytorch":
+ import submarine.models.pytorch
+
+ submarine.models.pytorch.save_model(model, local_dir)
+ elif model_type == "tensorflow":
+ import submarine.models.tensorflow
+
+ submarine.models.tensorflow.save_model(model, local_dir)
+ else:
+ raise Exception(
+ "No valid type of model has been matched to {}".format(
+ model_type))
+ self.artifact_repo.log_artifacts(local_dir, artifact_path)
Review comment:
That sounds correct. I will delete all the local model.
--
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: commits-unsubscribe@submarine.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org