Python SDK for katib #1177

prem0912 · 2020-05-05T12:51:43Z

1, Added Python SDK for katib
2. Added Documents and Readme

review-notebook-app · 2020-05-05T12:51:50Z

Check out this pull request on

Review Jupyter notebook visual diffs & provide feedback on notebooks.

Powered by ReviewNB

kubeflow-bot · 2020-05-05T12:51:51Z

This change is

k8s-ci-robot · 2020-05-05T12:51:54Z

Hi @prem0912. Thanks for your PR.

I'm waiting for a kubeflow member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

prem0912 · 2020-05-05T13:02:06Z

/assign @johnugeorge

andreyvelich

@prem0912 Thank you for doing this!
I left comments.

andreyvelich · 2020-05-05T13:39:19Z

sdk/python/README.md

+Install via [Setuptools](http://pypi.python.org/pypi/setuptools).
+
+```sh
+python setup.py install --user


Do we want to move Katib SDK to pip @johnugeorge ?

It is better to recommend pip way of installation

ok .. working on it

Added pip way installation and updated Readme.md

sdk/python/README.md

andreyvelich · 2020-05-05T14:02:14Z

sdk/python/README.md

+[KatibClient](docs/KatibClient.md) | [get_optimal_hyperparmeters](docs/KatibClient.md#get_optimal_hyperparmeters) | Get status, currentOptimalTrial with paramaterAssignments of an Experiment|
+
+
+## Documentation For Models


Maybe add script to update models?

will add it in next PR

andreyvelich · 2020-05-05T14:07:10Z

sdk/python/examples/bayesianoptimization-katib-sdk.ipynb

+   "metadata": {},
+   "outputs": [],
+   "source": [
+    "! python setup.py install"


I am not sure that we need this setup script in Example, since we already mentioned how to install SDK in README.

removed from examples

sdk/python/examples/bayesianoptimization-katib-sdk.ipynb

andreyvelich · 2020-05-05T14:23:49Z

sdk/python/examples/bayesianoptimization-katib-sdk.ipynb

+    "  )\n",
+    "\n",
+    "# Metric Collector\n",
+    "collector = V1alpha3CollectorSpec(kind = \"TensorFlowEvent\")\n",


For BO I think you can delete this Metrics Collector.

deleted metrics Collector

andreyvelich · 2020-05-05T14:29:19Z

sdk/python/kubeflow/katib/api/katib_client.py

+
+    katibexp = self.get_experiment(name, namespace=namespace)
+    result = {}
+    result["status"] = katibexp.get("status", {}).get("conditions", [])[-1].get("type")


Do we want to return Experiment status here? Since we have get_experiment_status API ?

I think it is needed because we are able to get optimized hyperparameter even only some of the trials are completed.

In that case, maybe users should handle this situation? For example, if get_optimal_hyperparmeters return [] run list_trials to check Trial statuses.
How experiment status can help user with it?
Experiment status can be running, but some Trials can be finished.

ok will remove status from optimized hyperparameter API

removed status from optimized hyperparameter API

sdk/python/OWNERS

johnugeorge · 2020-05-06T03:06:40Z

@prem0912 Looks like you cannot be added to owners without being added to Kubeflow org. Can we add owners in a separate PR then?

johnugeorge · 2020-05-06T03:14:34Z

sdk/python/README.md

@@ -0,0 +1,92 @@
+# Kubeflow Katib SDK 


Can we also link this SDK page from the main Katib Readme file?

Updated main Readme file

sperlingxx · 2020-05-06T08:07:26Z

sdk/python/kubeflow/katib/api/katib_client.py

+  def create_experiment(self, name, namespace=None):
+    """
+    Create the katib experiment
+    :param name: experiment object


I think body (or object) is more appropriate than name.

changes updated

sperlingxx · 2020-05-06T08:10:15Z

sdk/python/kubeflow/katib/api/katib_client.py

+
+    self.api_instance = client.CustomObjectsApi()
+
+  def _is_ipython(self):


Unifying indents to 4 ?

k8s-ci-robot · 2020-05-08T04:44:03Z

The following users are mentioned in OWNERS file(s) but are not members of the kubeflow org.

Once all users have been added as members of the org, you can trigger verification by writing /verify-owners in a comment.

prem0912
- sdk/python/OWNERS

prem0912 · 2020-05-08T11:29:18Z

@prem0912 Looks like you cannot be added to owners without being added to Kubeflow org. Can we add owners in a separate PR then?

@johnugeorge removed OWNERS file

gaocegege · 2020-05-13T01:46:20Z

/ok-to-test

gaocegege · 2020-06-02T09:37:59Z

/cc @johnugeorge @andreyvelich @sperlingxx

Can you please give another review? Thanks

gaocegege · 2020-06-02T09:38:08Z

/retest

johnugeorge · 2020-06-02T10:10:10Z

@prem0912 can you rebase this PR?

andreyvelich · 2020-06-02T17:36:44Z

sdk/python/examples/bayesianoptimization-katib-sdk.ipynb

+    "from kubernetes.client import V1PodTemplateSpec\n",
+    "from kubernetes.client import V1ObjectMeta\n",
+    "from kubernetes.client import V1PodSpec\n",
+    "from kubernetes.client import V1Container"


I didn't find where you use V1PodTemplateSpec, V1PodSpec, V1Container in bayesianoptimization-katib-sdk.ipynb example.

removed unused imports

andreyvelich · 2020-06-02T17:43:42Z

sdk/python/examples/bayesianoptimization-katib-sdk.ipynb

+   },
+   "outputs": [],
+   "source": [
+    "import kubeflow.katib as kc\n",


Maybe instead of importing kubeflow.katib as kc, just import required components.
I believe, for your example you can left:

from kubeflow.katib import KatibClient from kubeflow.katib import utils from kubeflow.katib import V1alpha3AlgorithmSetting from kubeflow.katib import V1alpha3AlgorithmSpec from kubeflow.katib import V1alpha3Experiment from kubeflow.katib import V1alpha3ExperimentSpec from kubeflow.katib import V1alpha3FeasibleSpace from kubeflow.katib import V1alpha3GoTemplate from kubeflow.katib import V1alpha3ObjectiveSpec from kubeflow.katib import V1alpha3ParameterSpec from kubeflow.katib import V1alpha3TrialTemplate

So it will not confuse user with other imports. What do you think?

Its better to import all the components because people will know what are the other components available to make use.

Ok, thanks.

andreyvelich · 2020-06-02T18:28:28Z

sdk/python/examples/bayesianoptimization-katib-sdk.ipynb

+    }
+   ],
+   "source": [
+    "kclient.get_experiment(name=\"bayesianoptimization\", namespace=None)"


I think we can use namespace=namespace here, like you did with other API calls to be consistent.

andreyvelich · 2020-06-02T18:30:50Z

sdk/python/examples/bayesianoptimization-katib-sdk.ipynb

+   "metadata": {},
+   "outputs": [],
+   "source": [
+    " kclient.delete_experiment(name=\"bayesianoptimization\", namespace=namespace)"


It is better to move delete_experiment to the latest command in example.
You are run few commands after deleting Experiment.

andreyvelich · 2020-06-02T18:33:48Z

sdk/python/examples/tfjob-katib-sdk.ipynb

+    "from kubernetes.client import V1PodTemplateSpec\n",
+    "from kubernetes.client import V1ObjectMeta\n",
+    "from kubernetes.client import V1PodSpec\n",
+    "from kubernetes.client import V1Container"


The same suggestion as I left for BO example.

removed unused imports

…mal_hyperparamater API

prem0912 · 2020-06-03T10:14:37Z

@johnugeorge rebased to upstream

andreyvelich

@prem0912 Thank you for doing this!
/lgtm
/cc @gaocegege @johnugeorge

johnugeorge · 2020-06-03T16:00:09Z

/approve

k8s-ci-robot · 2020-06-03T16:00:19Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: johnugeorge

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

~~OWNERS~~ [johnugeorge]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

johnugeorge · 2020-06-03T16:10:56Z

@jlewi How can we remove the label - invalid-owners-file? This was added by an earlier commit which is fixed. Can we re trigger?

andreyvelich · 2020-06-03T18:54:51Z

/verify-owners

gaocegege · 2020-06-04T02:20:24Z

/verify-owners

jlewi · 2020-06-04T02:28:09Z

@andreyvelich @johnugeorge You might want to look at GitHub permissions to see if there are fine grained roles that would allow key owners to add labels using the labels (e.g. the issue triage role, or maybe maintainers). And then create a GitHub team with appropriate owners in that team

andreyvelich · 2020-06-04T16:25:10Z

@jlewi I can see that Triage role can apply labels (https://help.github.com/en/github/setting-up-and-managing-organizations-and-teams/repository-permission-levels-for-an-organization#repository-access-for-each-permission-level).
Do you know, can Triage role also removing labels from the PRs?

* python SDK for katib with docs and examples * Update README.md * Update README.md * Update bayesianoptimization-katib-sdk.ipynb * Update bayesianoptimization-katib-sdk.ipynb * Update tfjob-katib-sdk.ipynb * Create OWNERS * Update bayesianoptimization-katib-sdk.ipynb * Update tfjob-katib-sdk.ipynb * Update bayesianoptimization-katib-sdk.ipynb * Update tfjob-katib-sdk.ipynb * Update bayesianoptimization-katib-sdk.ipynb * Update OWNERS * Update README.md * updated changes as per review comments * Update README.md * Update README.md * Added pip installation for katib sdk and removed status from get_optimal_hyperparamater API * Update README.md * Updated changes for delete_exp and removed unused imports

k8s-ci-robot added the needs-ok-to-test label May 5, 2020

k8s-ci-robot requested review from Akado2009 and garganubhav May 5, 2020 12:51

k8s-ci-robot added the size/XXL label May 5, 2020

prem0912 changed the title ~~Python katib SDK~~ Python SDK for katib deployment May 5, 2020

prem0912 changed the title ~~Python SDK for katib deployment~~ Python SDK for katib May 5, 2020

k8s-ci-robot assigned johnugeorge May 5, 2020

andreyvelich reviewed May 5, 2020

View reviewed changes

sdk/python/OWNERS Outdated Show resolved Hide resolved

k8s-ci-robot added the do-not-merge/invalid-owners-file label May 5, 2020

johnugeorge reviewed May 6, 2020

View reviewed changes

sperlingxx reviewed May 6, 2020

View reviewed changes

k8s-ci-robot added ok-to-test and removed needs-ok-to-test labels May 13, 2020

k8s-ci-robot requested review from andreyvelich, johnugeorge and sperlingxx June 2, 2020 09:38

andreyvelich reviewed Jun 2, 2020

View reviewed changes

prem0912 and others added 10 commits June 3, 2020 15:36

Update tfjob-katib-sdk.ipynb

0c1c2b9

Update bayesianoptimization-katib-sdk.ipynb

6bf76b4

Update OWNERS

0cb08e8

Update README.md

d892866

updated changes as per review comments

5f9e854

Update README.md

d3d0108

Update README.md

ff005ec

Added pip installation for katib sdk and removed status from get_opti…

c42af35

…mal_hyperparamater API

Update README.md

ac3228b

Updated changes for delete_exp and removed unused imports

d19149d

prem0912 force-pushed the master branch from 078841f to d19149d Compare June 3, 2020 10:07

prem0912 requested a review from andreyvelich June 3, 2020 10:14

andreyvelich approved these changes Jun 3, 2020

View reviewed changes

k8s-ci-robot requested a review from gaocegege June 3, 2020 10:15

k8s-ci-robot assigned andreyvelich Jun 3, 2020

k8s-ci-robot added the lgtm label Jun 3, 2020

k8s-ci-robot added the approved label Jun 3, 2020

jlewi removed the do-not-merge/invalid-owners-file label Jun 4, 2020

k8s-ci-robot merged commit c2c5288 into kubeflow:master Jun 4, 2020

andreyvelich mentioned this pull request Jun 8, 2020

Katib cut release for Kubeflow 1.1 #1211

Closed

13 tasks

anencore94 mentioned this pull request Jul 27, 2022

[hotfix]: filter by name of experiment #1920

Merged

1 task

		[KatibClient](docs/KatibClient.md) \| [get_optimal_hyperparmeters](docs/KatibClient.md#get_optimal_hyperparmeters) \| Get status, currentOptimalTrial with paramaterAssignments of an Experiment\|


		## Documentation For Models


		self.api_instance = client.CustomObjectsApi()

		def _is_ipython(self):

Python SDK for katib #1177

Python SDK for katib #1177

Conversation

prem0912 commented May 5, 2020

review-notebook-app bot commented May 5, 2020

kubeflow-bot commented May 5, 2020

k8s-ci-robot commented May 5, 2020

prem0912 commented May 5, 2020

andreyvelich left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

johnugeorge commented May 6, 2020

Choose a reason for hiding this comment

prem0912 May 8, 2020 • edited Loading

Choose a reason for hiding this comment

sperlingxx May 6, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

k8s-ci-robot commented May 8, 2020

prem0912 commented May 8, 2020

gaocegege commented May 13, 2020

gaocegege commented Jun 2, 2020

gaocegege commented Jun 2, 2020

johnugeorge commented Jun 2, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

andreyvelich Jun 2, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

prem0912 commented Jun 3, 2020

andreyvelich left a comment

Choose a reason for hiding this comment

johnugeorge commented Jun 3, 2020

k8s-ci-robot commented Jun 3, 2020

johnugeorge commented Jun 3, 2020

andreyvelich commented Jun 3, 2020

gaocegege commented Jun 4, 2020

jlewi commented Jun 4, 2020

andreyvelich commented Jun 4, 2020

prem0912 May 8, 2020 •

edited

Loading

sperlingxx May 6, 2020 •

edited

Loading

andreyvelich Jun 2, 2020 •

edited

Loading