[Sample] CI Sample: mnist #3013

dldaisy · 2020-02-07T12:40:07Z

#2 piece for CI samples:
<To make the repo more easily reviewable, pr 2784(#2784) will be broken into smaller prs>
This sample is the mnist sample for versioned pipeline CI. Data scientists can make the least effort to migrate their original code to implement KFP CI by wrapping the whole training code in one step and other adds-on like visualizations in other steps.

Use cloudbuild for CI process
Use KFP SDK to connect to KFP API server

This change is

k8s-ci-robot · 2020-02-07T12:40:22Z

Hi @dldaisy. 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.

dldaisy · 2020-02-07T12:46:38Z

@numerology @jingzhang36

dldaisy · 2020-02-08T07:31:40Z

/retest

k8s-ci-robot · 2020-02-08T07:32:06Z

@dldaisy: Cannot trigger testing until a trusted user reviews the PR and leaves an /ok-to-test message.

In response to this:

/retest

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.

jingzhang36 · 2020-02-10T07:29:04Z

samples/contrib/versioned-pipeline-ci-samples/mnist-ci-sample/README.md

@@ -0,0 +1,25 @@
+# Mnist CI Pipeline


The first occurrence of a terminology better be fully spelled out. Continuous Integration (CI)

jingzhang36 · 2020-02-10T07:29:37Z

samples/contrib/versioned-pipeline-ci-samples/mnist-ci-sample/README.md

+# Mnist CI Pipeline
+
+## What you can learn in this sample
+* CI process of a simple but general ML pipeline.


Also mention that this CI is using cloud build service.

jingzhang36 · 2020-02-10T07:30:40Z

Thanks for the sample! A couple of minor comments. Otherwise, /lgtm

numerology · 2020-02-10T23:44:06Z

/ok-to-test

numerology · 2020-02-10T23:45:24Z

samples/contrib/versioned-pipeline-ci-samples/mnist-ci-sample/README.md

+
+This sample uses cloud build to implement the continuous integration process of a basic machine learning pipeline that trains and visualizes model in tensorboard. Once all set up, you can push your code to github repo, then the build process in cloud build will be triggered automatically, then a run will be created in kubeflow pipeline. You can view your pipeline and the run in kubeflow pipelines. 
+
+We use *Kubeflow Pipeline(KFP) SDK** to interact with kubeflow pipeline to create a new version and a run in this sample.


Suggested change

We use *Kubeflow Pipeline(KFP) SDK** to interact with kubeflow pipeline to create a new version and a run in this sample.

We use *Kubeflow Pipeline(KFP) SDK* to interact with kubeflow pipeline to create a new version and a run in this sample.

Thanks. Will modify.

numerology · 2020-02-10T23:49:14Z

samples/contrib/versioned-pipeline-ci-samples/mnist-ci-sample/README.md

+
+
+## What needs to be done before run
+* Create a secret following the troubleshooting parts in [https://github.com/kubeflow/pipelines/tree/master/manifests/kustomize]()


Maybe put the link in the brackets to make it work?

Yes, Thanks.

numerology

Thanks @dldaisy ! Left some comment

numerology · 2020-02-19T00:47:46Z

samples/contrib/versioned-pipeline-ci-samples/mnist-ci-sample/README.md

@@ -0,0 +1,31 @@
+# Mnist Continuous Integration(CI) Pipeline
+
+## ## Overview


Suggested change

## ## Overview

## Overview

numerology · 2020-02-19T00:56:30Z

samples/contrib/versioned-pipeline-ci-samples/mnist-ci-sample/pipeline.py

+   train_step = train_op(storage_bucket=storage_bucket).apply(use_gcp_secret('user-gcp-sa'))
+
+   visualize_op = components.load_component_from_file('./tensorboard/component.yaml')
+   visualize_step = visualize_op(logdir='%s' % train_step.outputs['logdir']).apply(use_gcp_secret('user-gcp-sa'))


nit: With hosted pipeline beta and workload identity, user-gcp-sa will be deprecated. Do you mind adding a comment (and perhaps also a TODO) to track this?

Of course. I'll leave a comment in readme for this.

numerology · 2020-02-19T00:59:00Z

samples/contrib/versioned-pipeline-ci-samples/mnist-ci-sample/tensorboard/tensorboard.py

+
+    parser = argparse.ArgumentParser()
+    parser.add_argument('--logdir', type=str)
+    parser.add_argument('--output_path', type=str, default='/')


Quick Q: what if a user specify --output_path. Then in line 17 it seems to me that mlpipeline-ui-metadata.json will no longer at /mlpipeline-ui-metadata.json

That's correct. I'll make it configurable.

numerology · 2020-02-19T01:01:20Z

/approve
LGTM modulo some nit issues.
@jingzhang36 can LGTM in case I'm asleep. Thanks! @dldaisy

Ark-kun · 2020-02-20T01:48:34Z

samples/contrib/versioned-pipeline-ci-samples/mnist-ci-sample/tensorboard/tensorboard.py

+        'source': args.logdir,
+      }]
+    }
+    with open(args.output_path+'mlpipeline-ui-metadata.json', 'w') as f:


Please make this path configurable (add a dedicated command-line argument).

OK, thanks!

Ark-kun · 2020-02-20T01:50:43Z

samples/contrib/versioned-pipeline-ci-samples/mnist-ci-sample/tensorboard/tensorboard.py

+        'source': args.logdir,
+      }]
+    }
+    with open(args.output_path+'mlpipeline-ui-metadata.json', 'w') as f:


You might need to create the directory fist.

from pathlib import Path Path(args.ui_metadata_path).parent.mkdir(parents=True, exist_ok=True)

OK, thank you!

Ark-kun · 2020-02-20T01:51:23Z

samples/contrib/versioned-pipeline-ci-samples/mnist-ci-sample/tensorboard/component.yaml

+    command: ['python', '/tensorboard.py']
+    args: ['--logdir', {inputValue: logdir}]
+    fileOutputs: 
+      MLPipeline UI metadata: /mlpipeline-ui-metadata.json


It's better to have all paths configurable.

Ark-kun · 2020-02-20T01:51:46Z

samples/contrib/versioned-pipeline-ci-samples/mnist-ci-sample/train/component.yaml

+    command: ['python', '/mnist.py']
+    args: ['--storage_bucket', {inputValue: storage_bucket}]
+    fileOutputs: 
+      logdir: /logdir.txt


It's better to have all paths configurable.

jingzhang36 · 2020-02-24T07:04:52Z

/lgtm
/approve

k8s-ci-robot · 2020-02-24T07:05:10Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: jingzhang36, numerology

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:

~~samples/OWNERS~~ [numerology]

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

jingzhang36 · 2020-02-24T09:20:47Z

/test kubeflow-pipeline-e2e-test

* mnist sample init * remove comma in component.yaml * image cannot use placeholder * fileOutputs instead of file_outputs * ui metadata * input reference * rm tensorboard_image input * train op with yaml * train component.yaml * fix typo * sed image name * sed quote * latest instead of commit sha * typo and logic sequence of push and execute * latest to commit_sha * rm debug print * more instructions * refined readme.md * fix bugs in readme.md * readme user-gcp-sa * configurable output path

dldaisy added 17 commits February 7, 2020 16:55

mnist sample init

6e55e57

remove comma in component.yaml

5ab325d

image cannot use placeholder

008bf03

fileOutputs instead of file_outputs

243ea71

ui metadata

3e0586d

input reference

2d831fb

rm tensorboard_image input

969d635

train op with yaml

2fd1eff

train component.yaml

2fbcbab

fix typo

5971cf8

sed image name

1f8c6d6

sed quote

13dd933

latest instead of commit sha

84755d0

typo and logic sequence of push and execute

8c5b445

latest to commit_sha

1d6b16f

rm debug print

f0726b5

more instructions

45a3f33

k8s-ci-robot added the size/L label Feb 7, 2020

k8s-ci-robot requested review from animeshsingh and numerology February 7, 2020 12:40

k8s-ci-robot added the needs-ok-to-test label Feb 7, 2020

jingzhang36 reviewed Feb 10, 2020

View reviewed changes

refined readme.md

ccd5726

k8s-ci-robot added ok-to-test and removed needs-ok-to-test labels Feb 10, 2020

numerology reviewed Feb 10, 2020

View reviewed changes

fix bugs in readme.md

96eb7ee

numerology reviewed Feb 19, 2020

View reviewed changes

k8s-ci-robot added the approved label Feb 19, 2020

Ark-kun reviewed Feb 20, 2020

View reviewed changes

dldaisy added 2 commits February 21, 2020 21:30

readme user-gcp-sa

f4d2969

configurable output path

da247c8

k8s-ci-robot assigned jingzhang36 Feb 24, 2020

k8s-ci-robot added the lgtm label Feb 24, 2020

k8s-ci-robot merged commit 7905856 into kubeflow:master Feb 24, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Sample] CI Sample: mnist #3013

[Sample] CI Sample: mnist #3013

dldaisy commented Feb 7, 2020 •

edited by jlewi

Loading

k8s-ci-robot commented Feb 7, 2020

dldaisy commented Feb 7, 2020

dldaisy commented Feb 8, 2020

k8s-ci-robot commented Feb 8, 2020

jingzhang36 Feb 10, 2020

dldaisy Feb 10, 2020

jingzhang36 Feb 10, 2020

dldaisy Feb 10, 2020

jingzhang36 commented Feb 10, 2020

numerology commented Feb 10, 2020

numerology Feb 10, 2020

dldaisy Feb 13, 2020

numerology Feb 10, 2020

dldaisy Feb 14, 2020

numerology left a comment

numerology Feb 19, 2020

dldaisy Feb 21, 2020

numerology Feb 19, 2020

dldaisy Feb 21, 2020

numerology Feb 19, 2020

dldaisy Feb 21, 2020

numerology commented Feb 19, 2020

Ark-kun Feb 20, 2020

dldaisy Feb 21, 2020

Ark-kun Feb 20, 2020

dldaisy Feb 21, 2020

Ark-kun Feb 20, 2020

dldaisy Feb 21, 2020

Ark-kun Feb 20, 2020

dldaisy Feb 21, 2020

jingzhang36 commented Feb 24, 2020

k8s-ci-robot commented Feb 24, 2020

jingzhang36 commented Feb 24, 2020


		This sample uses cloud build to implement the continuous integration process of a basic machine learning pipeline that trains and visualizes model in tensorboard. Once all set up, you can push your code to github repo, then the build process in cloud build will be triggered automatically, then a run will be created in kubeflow pipeline. You can view your pipeline and the run in kubeflow pipelines.

		We use Kubeflow Pipeline(KFP) SDK* to interact with kubeflow pipeline to create a new version and a run in this sample.

	We use Kubeflow Pipeline(KFP) SDK* to interact with kubeflow pipeline to create a new version and a run in this sample.
	We use Kubeflow Pipeline(KFP) SDK to interact with kubeflow pipeline to create a new version and a run in this sample.



		## What needs to be done before run
		* Create a secret following the troubleshooting parts in [https://github.com/kubeflow/pipelines/tree/master/manifests/kustomize]()

		@@ -0,0 +1,31 @@
		# Mnist Continuous Integration(CI) Pipeline

		## ## Overview

[Sample] CI Sample: mnist #3013

[Sample] CI Sample: mnist #3013

Conversation

dldaisy commented Feb 7, 2020 • edited by jlewi Loading

k8s-ci-robot commented Feb 7, 2020

dldaisy commented Feb 7, 2020

dldaisy commented Feb 8, 2020

k8s-ci-robot commented Feb 8, 2020

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

jingzhang36 commented Feb 10, 2020

numerology commented Feb 10, 2020

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

numerology 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

numerology commented Feb 19, 2020

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

jingzhang36 commented Feb 24, 2020

k8s-ci-robot commented Feb 24, 2020

jingzhang36 commented Feb 24, 2020

dldaisy commented Feb 7, 2020 •

edited by jlewi

Loading