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

Katib v1beta1 documentation update #2312

Merged

Conversation

andreyvelich
Copy link
Member

Related: kubeflow/katib#1360.
Blocked by: kubeflow/manifests#1593.
I updated docs for Katib v1beta1. I made few changes:

  1. Update reference docs for v1beta1. @8bitmp3 @RFMVasconcelos Do we want to have v1alpha3 reference since all docs are written for v1beta1 ?
  2. Fix few broken links.
  3. Rename hyperparameter-tuning section to katib to make it clearer, since we are building AutoML project. What do you think about it @8bitmp3 @RFMVasconcelos ?
  4. Add missing links for the examples, controller components, etc.
  5. Update images.

I am still waiting for the pending PRs (I need to update docker images and few UI images., but it would be great if you can start to review it!

/assign @gaocegege @johnugeorge
/cc @8bitmp3 @RFMVasconcelos

@kubeflow-bot
Copy link

This change is Reviewable

@rui-vas
Copy link
Contributor

rui-vas commented Oct 28, 2020

Hi @andreyvelich !

Thank you for this update! Replying by point:

Update reference docs for v1beta1. @8bitmp3 @RFMVasconcelos Do we want to have v1alpha3 reference since all docs are written for v1beta1 ?

You're asking if we want to keep a v1alpha3 version of the docs, now that we're updating Katib to v1beta1, correct?
If so:

  • When will these docs be effective? If we're launching v1beta1 with Kubeflow v1.2, would it make sense to include this in v1.2 of the docs?
  • If so, we can simply add a reference to v1alpha3 saying that docs can be found in Kubeflow v1.1 docs (with a link)

WDYT?

Rename hyperparameter-tuning section to katib to make it clearer, since we are building AutoML project. What do you think about it @8bitmp3 @RFMVasconcelos ?

LGTM.

Copy link
Contributor

@rui-vas rui-vas left a comment

Choose a reason for hiding this comment

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

One other point was the name change from Katib to something else. If we plan to do that, should we already make changes accordingly?

Any decision on that front?

@andreyvelich
Copy link
Member Author

Thank you for the comment @RFMVasconcelos!

You're asking if we want to keep a v1alpha3 version of the docs, now that we're updating Katib to v1beta1, correct?
If so:

  • When will these docs be effective? If we're launching v1beta1 with Kubeflow v1.2, would it make sense to include this in v1.2 of the docs?

These docs will be effective after we merge: kubeflow/manifests#1593.
Including docs in V1.2 makes sense to me. I assume that website master branch will be point to 1.2 Kubeflow release, right ? If so, I am writing these docs to the appropriate branch.

  • If so, we can simply add a reference to v1alpha3 saying that docs can be found in Kubeflow v1.1 docs (with a link)

I think, it's a good idea. I will add v1alpha3 reference docs and add link to 1.1. Is there any way to get link for 1.1 docs? I found only 1.0 link: https://v1-0-branch.kubeflow.org/.

@andreyvelich
Copy link
Member Author

One other point was the name change from Katib to something else. If we plan to do that, should we already make changes accordingly?

Any decision on that front?

I think we can stick with Katib name for Kubeflow 1.2.
If we decide to rename the project after 1.2, I will submit separate PR to make changes in the docs.

WDYT @gaocegege @johnugeorge ?

@andreyvelich
Copy link
Member Author

Preview: https://deploy-preview-2312--competent-brattain-de2d6d.netlify.app/.

I have updated doc with v1alpha3 reference.

/cc @RFMVasconcelos @gaocegege @johnugeorge

Copy link
Contributor

@rui-vas rui-vas left a comment

Choose a reason for hiding this comment

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

Looking great!

You are indeed writing to the right branch! What we need to do is create the v1.1 branch and update master to be v1.2.

Let me create an issue about it :)

@rui-vas
Copy link
Contributor

rui-vas commented Oct 30, 2020

Oh, it looks like there is one already -> #2322

@andreyvelich
Copy link
Member Author

Thanks @RFMVasconcelos.
Do you have any comments/objections on the doc changes in this PR?

@andreyvelich
Copy link
Member Author

@RFMVasconcelos @8bitmp3 @gaocegege @johnugeorge I removed WIP status, since kubeflow/manifests#1593 is ready to merge. If you have any objections and comments on this PR, please let me know.
If not, we can merge it and start work on other Katib documentation PRs

@andreyvelich andreyvelich changed the title [WIP] Katib v1beta1 documentation update Katib v1beta1 documentation update Nov 2, 2020
Copy link
Contributor

@8bitmp3 8bitmp3 left a comment

Choose a reason for hiding this comment

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

@andreyvelich Thanks for the PR!

I've added a few suggestions and comments. Feel free to disregard them 👍

Maybe in the Katib explanation in /components/katib/overview.md we could also add a small link for all the new ML users about AutoML (e.g. https://www.fast.ai/2018/07/16/auto-ml2/) and/or Microsoft Azure, Google Cloud and AWS AutoML solution docs?

Thank you for your help!

Update: also maybe we should move the "about" section up in overview.md such that:

Katib is a Kubernetes-native project for automated machine learning (AutoML) —
it's a system for hyperparameter tuning and neural architecture search. 
Katib supports a number of machine learning frameworks, including
TensorFlow, MXNet, PyTorch, XGBoost, and others.
The [Katib project](https://github.com/kubeflow/katib) is open source.
The [developer guide](https://github.com/kubeflow/katib/blob/master/docs/developer-guide.md)
is a good starting point for developers who want to contribute to the project.

Comment on lines 8 to 11
This page describes information about
[Katib config](https://github.com/kubeflow/katib/blob/master/manifests/v1beta1/katib-controller/katib-config.yaml).

Katib config is the Kubernetes
[Config Map](https://kubernetes.io/docs/tasks/configure-pod-container/configure-pod-configmap/) that contains information about:
Copy link
Contributor

Choose a reason for hiding this comment

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

Thank you for the PR @andreyvelich

Hi, what do you think of this suggestion?

  • "refactor" to 'This page describes Katib config — the Kubernetes Config Map that contains information about:'
Suggested change
This page describes information about
[Katib config](https://github.com/kubeflow/katib/blob/master/manifests/v1beta1/katib-controller/katib-config.yaml).
Katib config is the Kubernetes
[Config Map](https://kubernetes.io/docs/tasks/configure-pod-container/configure-pod-configmap/) that contains information about:
This page describes
[Katib config](https://github.com/kubeflow/katib/blob/master/manifests/v1beta1/katib-controller/katib-config.yaml)
the Kubernetes
[Config Map](https://kubernetes.io/docs/tasks/configure-pod-container/configure-pod-configmap/) that contains information about:

Copy link
Member Author

Choose a reason for hiding this comment

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

Agree, changed.


You can edit this Config Map even after deploying Katib.

If you deploy Katib in Kubeflow namespace, to edit Katib config run this:
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe just change the order here, use a different (present continuous) tense, and "the" (the definite article) (I checked with the K8s guide, as I'm not 100% sure about these):

Suggested change
If you deploy Katib in Kubeflow namespace, to edit Katib config run this:
If you are deploying Katib in the Kubeflow namespace, run this command to edit your Katib config:

Copy link
Member Author

Choose a reason for hiding this comment

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

Agree, changed.

Comment on lines 24 to 28

`kubectl edit configMap katib-config -n kubeflow`

Copy link
Contributor

Choose a reason for hiding this comment

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

Suggestion - use code style/code font/code block/codefence (K8s style guide, Google style guide:

Suggested change
`kubectl edit configMap katib-config -n kubeflow`
```shell
kubectl edit configMap katib-config -n kubeflow
```

(without the tabs that indent the code block - they render differently if I remove them)

Copy link
Member Author

@andreyvelich andreyvelich Nov 2, 2020

Choose a reason for hiding this comment

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

Copy link
Member Author

Choose a reason for hiding this comment

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

@8bitmp3 I've just seen you comment: https://github.com/kubeflow/website/pull/2320/files#r516300401 about it.
Will do the change in this PR.

Copy link
Member Author

Choose a reason for hiding this comment

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

Done in fe91e74.

content/en/docs/components/katib/katib-config.md Outdated Show resolved Hide resolved

All of these settings except **`image`** can be omitted. If you don't specify any other settings, default value is set.

1. `image` - Docker image for the `File` metrics collector's container.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
1. `image` - Docker image for the `File` metrics collector's container.
1. `image` - a Docker image for the `File` metrics collector's container.

Copy link
Member Author

Choose a reason for hiding this comment

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

Agree, changed.
@8bitmp3 I made all list items with not capital letter to be consistent (e.g. https://github.com/kubeflow/website/blame/62aba5ce7a065eb2b60d6b7884f4ab075be33fcc/content/en/docs/components/katib/katib-config.md#L66)
WDYT ?

content/en/docs/components/katib/katib-config.md Outdated Show resolved Hide resolved
content/en/docs/components/katib/katib-config.md Outdated Show resolved Hide resolved
content/en/docs/components/katib/overview.md Outdated Show resolved Hide resolved
content/en/docs/components/katib/katib-config.md Outdated Show resolved Hide resolved
Comment on lines 11 to 12
Katib is Kubernetes-native project for automated machine learning (AutoML).
Use Katib for automated tuning of your machine learning (ML) model's hyperparameters and architecture.
Copy link
Contributor

@8bitmp3 8bitmp3 Nov 2, 2020

Choose a reason for hiding this comment

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

Suggestions:

I think lines 87-95 about "the Katib project" should be right in the beginning of overview.md:

Suggested change
Katib is Kubernetes-native project for automated machine learning (AutoML).
Use Katib for automated tuning of your machine learning (ML) model's hyperparameters and architecture.
Katib is a Kubernetes-native project for automated machine learning (AutoML) —
it's a system for hyperparameter tuning and neural architecture search.
Katib supports a number of machine learning frameworks, including
TensorFlow, MXNet, PyTorch, XGBoost, and others.
The [Katib project](https://github.com/kubeflow/katib) is open source.
The [developer guide](https://github.com/kubeflow/katib/blob/master/docs/developer-guide.md)
is a good starting point for developers who want to contribute to the project.

@andreyvelich
Copy link
Member Author

Thank you for the another review @8bitmp3 !
I've addressed your comments.
Preview: https://deploy-preview-2312--competent-brattain-de2d6d.netlify.app/.

@8bitmp3
Copy link
Contributor

8bitmp3 commented Nov 10, 2020

@andreyvelich Thanks. If you're OK with the suggestions/own changes, feel free to get the LGTM and approve labels from the AutoML/Katib experts (which I am not 😄)

/assign @animeshsingh @Bobgy @joeliedtke
(approvers)

@andreyvelich
Copy link
Member Author

@8bitmp3 Thank you again for the help on this PR!
If you don't have any other suggestions on the trial-template doc, I'll remove /hold label and we can merge this PR.

/cc @gaocegege @johnugeorge

@@ -1,13 +1,13 @@
+++
title = "Trial Template Overview"
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
title = "Trial Template Overview"
title = "Overview of trial template parameters in Katib"

(Sentence case, unless it's a product name, I think)

@@ -1,13 +1,13 @@
+++
title = "Trial Template Overview"
description = "How to specify trial template parameters and support custom CRD in Katib"
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
description = "How to specify trial template parameters and support custom CRD in Katib"
description = "How to specify trial template parameters and support a custom resource (CRD) in Katib"

(I had to look up CRD on the K8s docs - correct me if I'm wrong. Just wanted to help some new users)

Comment on lines 10 to 13
in Katib. Follow this page to know more about changing trial template specification,
how to use [Kubernetes ConfigMaps](https://kubernetes.io/docs/concepts/configuration/configmap/)
to store templates and how to modify Katib controller to support your
Kubernetes CRD in Katib experiments.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
in Katib. Follow this page to know more about changing trial template specification,
how to use [Kubernetes ConfigMaps](https://kubernetes.io/docs/concepts/configuration/configmap/)
to store templates and how to modify Katib controller to support your
Kubernetes CRD in Katib experiments.
in Katib. You will learn about changing trial template specification,
how to use [Kubernetes ConfigMaps](https://kubernetes.io/docs/concepts/configuration/configmap/)
to store templates and how to modify Katib controller to support your
Kubernetes CRD in Katib experiments.

@@ -232,12 +235,12 @@ to know more about it.
It is possible to use your own Kubernetes CRD or other Kubernetes resource
(e.g. [Kubernetes `Deployment`](https://kubernetes.io/docs/concepts/workloads/controllers/deployment/))
as a trial worker without modifying Katib controller source code and building
the new image. As long as your CRD creates Kubernetes pods, allows to inject
the new image. As long as your CRD creates Kubernetes Pods, allows to inject
Copy link
Contributor

Choose a reason for hiding this comment

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

is it Pods or pods in K8s? I thought it's "pods" 😄

Copy link
Member Author

@andreyvelich andreyvelich Nov 11, 2020

Choose a reason for hiding this comment

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

I think it's called Kubernetes Pods. 😄
Ref: https://kubernetes.io/docs/concepts/workloads/pods/#using-pods.

the [sidecar container](https://kubernetes.io/docs/concepts/workloads/pods/) on
these pods and has succeeded and failed status, you can use it in Katib.
these Pods and has succeeded and failed status, you can use it in Katib.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
these Pods and has succeeded and failed status, you can use it in Katib.
these Pods and has succeeded and failed status, you can use it in Katib.

is it Pods or pods in K8s? I thought it's "pods" 😄

@@ -247,11 +250,11 @@ Follow these two simple steps to integrate your custom CRD in Katib:

1. Modify Katib controller
[ClusterRole's rules](https://github.com/kubeflow/katib/blob/master/manifests/v1beta1/katib-controller/rbac.yaml#L5)
with the new rule to get access for all resources that are created
with the new rule to give Katib access for the all resources that are created
Copy link
Contributor

Choose a reason for hiding this comment

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

Correction:

Suggested change
with the new rule to give Katib access for the all resources that are created
with the new rule to give Katib access to all resources that are created

@@ -247,11 +250,11 @@ Follow these two simple steps to integrate your custom CRD in Katib:

1. Modify Katib controller
[ClusterRole's rules](https://github.com/kubeflow/katib/blob/master/manifests/v1beta1/katib-controller/rbac.yaml#L5)
with the new rule to get access for all resources that are created
with the new rule to give Katib access for the all resources that are created
by trial. To know more about ClusterRole, check
Copy link
Contributor

Choose a reason for hiding this comment

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

"by the trial"?

@@ -293,7 +296,7 @@ Expected output for the Tekton `Pipeline`:
```

If the above steps are successful, you are able to use your custom object YAML
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
If the above steps are successful, you are able to use your custom object YAML
If you ran the above steps successfully, you should be able to use your custom object YAML

Copy link
Contributor

@8bitmp3 8bitmp3 left a comment

Choose a reason for hiding this comment

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

@andreyvelich Just took a last look at katib/trial-template.md. We can revise more stuff in another PR, if necessary. Let's try to get a LGTM and /approve soon. Thanks for the patience

@andreyvelich
Copy link
Member Author

@andreyvelich Just took a last look at katib/trial-template.md. We can revise more stuff in another PR, if necessary. Let's try to get a LGTM and /approve soon. Thanks for the patience

Sure @8bitmp3.
/hold cancel
/approve

@gaocegege @johnugeorge Can you give your /lgtm please?

@andreyvelich
Copy link
Member Author

I think we need to get /approve from one of the root OWNERS for this PR.
@animeshsingh @Bobgy @joeliedtke Can you help with it, please ?

@Bobgy
Copy link
Contributor

Bobgy commented Nov 11, 2020

@andreyvelich
Can stakeholders lgtm first? I can help with the approval.

@Bobgy
Copy link
Contributor

Bobgy commented Nov 11, 2020

/approve

@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: andreyvelich, Bobgy

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:

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

@rui-vas
Copy link
Contributor

rui-vas commented Nov 11, 2020

/lgtm

@k8s-ci-robot k8s-ci-robot merged commit ba72604 into kubeflow:master Nov 11, 2020
@rui-vas
Copy link
Contributor

rui-vas commented Nov 11, 2020

Thank you @andreyvelich for this big push! It's merged 🚀

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.

10 participants