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

Add SageMaker create workteam and Ground Truth components, sample demo pipeline, other minor updates #1716

Merged
merged 5 commits into from
Aug 5, 2019

Conversation

carolynwang
Copy link
Contributor

@carolynwang carolynwang commented Aug 1, 2019

Summary

@Jeffwan @mbaijal @gautamkmr @kalyc

Add create workteam and Ground Truth (data labeling) components

  • Workteam component supports private workteam creation from an existing Amazon Cognito user pool and user groups
  • Ground Truth component includes all features except workteam creation within the GT config (separate component), and support for the option of using a random sample or selected subset of the input data will be added in a future update

Update HPO and train components

  • Additional parameters for data location (used in the channel definitions) added so that users can pipeline GT and HPO/training together and use GT output manifests as the HPO/train data inputs

Add a small image classification pipeline to demo workteam and GT components

  • Python script to prepare inputs for the sample pipeline
  • Pipeline itself and a README for steps

Update Docker image used for components

  • Update Dockerfile to include workteam and Ground Truth components

Testing

Component functionality

  • mini-image-classification-pipeline.py compiles and runs without errors using the default values set in the pipeline parameters

This change is Reviewable

…d train, add sample pipeline demo for workteam and GT, update images
@k8s-ci-robot
Copy link
Contributor

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

@Jeffwan
Copy link
Member

Jeffwan commented Aug 1, 2019

/assign @Jeffwan

Copy link
Contributor

@goswamig goswamig left a comment

Choose a reason for hiding this comment

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

1 Add status check on every client.request value. Its missing and some time it might take wait for xyz to complete function to realize.
over all LGTM

@@ -71,7 +71,7 @@ outputs:
- {name: output_location, description: 'S3 URI of the transform job results.'}
implementation:
container:
image: carowang/kubeflow-pipeline-aws-sm:20190728-01
image: carowang/kubeflow-pipeline-aws-sm:20190801-01
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we move this image is to somewhere in AWS official dockerhub ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes will do once I get more info on how to do so

components/aws/sagemaker/common/_utils.py Outdated Show resolved Hide resolved
components/aws/sagemaker/common/_utils.py Outdated Show resolved Hide resolved
components/aws/sagemaker/common/_utils.py Show resolved Hide resolved
components/aws/sagemaker/common/_utils.py Show resolved Hide resolved
components/aws/sagemaker/common/_utils.py Show resolved Hide resolved

auto_labeling_map = {'bounding box': 'object-detection',
'image classification': 'image-classification',
'text classification': 'text-classification'}

Choose a reason for hiding this comment

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

what if I want to stage a custom image labeling task? I don't see that case here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

in terms of automatic data labeling for custom labeling jobs - Ground Truth currently doesn't support that, which is why this map only contains the built-in algos that have auto data labeling support
in general though, custom tasks are handled while updating pre-human and annotation consolidation task lambda functions

@Jeffwan
Copy link
Member

Jeffwan commented Aug 5, 2019

/lgtm
/approve

@Jeffwan
Copy link
Member

Jeffwan commented Aug 5, 2019

/ok-to-test

@Jeffwan
Copy link
Member

Jeffwan commented Aug 5, 2019

/hold

@Jeffwan
Copy link
Member

Jeffwan commented Aug 5, 2019

@carolynwang Please move samples to https://github.com/kubeflow/pipelines/tree/master/samples/contrib/aws

Original path was refactored and all vendors related are moved to contrib path

@k8s-ci-robot k8s-ci-robot removed the lgtm label Aug 5, 2019
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: Jeffwan

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

@Jeffwan
Copy link
Member

Jeffwan commented Aug 5, 2019

/hold cancel

@Jeffwan
Copy link
Member

Jeffwan commented Aug 5, 2019

/lgtm

@k8s-ci-robot k8s-ci-robot merged commit 3f8bcff into kubeflow:master Aug 5, 2019
@carolynwang carolynwang deleted the sagemaker-gt-component branch August 5, 2019 22:51
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.

6 participants