feat(components): AWS SageMaker - Add optional parameter to allow training component to accept parameters related to Debugger #4283

dstnluong · 2020-07-28T23:16:49Z

Add functionality to accept and process debugger related parameters
Updated KFP logs to show debugger rules status and to wait until all rules have finished
Added and updated unit tests and integration tests
Added a new sample pipeline which uses the newly added parameters with a README that explains how to run the sample
Minor typo fixes in various files

Checklist:

The title for your pull request (PR) should follow our title convention. Learn more about the pull request title convention used in this repository.

PR titles examples:
- fix(frontend): fixes empty page. Fixes #1234
  Use fix to indicate that this PR fixes a bug.
- feat(backend): configurable service account. Fixes #1234, fixes #1235
  Use feat to indicate that this PR adds a new feature.
- chore: set up changelog generation tools
  Use chore to indicate that this PR makes some changes that users don't need to know.
- test: fix CI failure. Part of #1234
  Use part of to indicate that a PR is working on an issue, but shouldn't close the issue when merged.
Do you want this pull request (PR) cherry-picked into the current release branch?

If yes, use one of the following options:
- (Recommended.) Ask the PR approver to add the cherrypick-approved label to this PR. The release manager adds this PR to the release branch in a batch update.
- After this PR is merged, create a cherry-pick PR to add these changes to the release branch. (For more information about creating a cherry-pick PR, see the Kubeflow Pipelines release guide.)

…t tests, and integration test

kubeflow-bot · 2020-07-28T23:16:55Z

This change is

google-cla · 2020-07-28T23:16:55Z

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project (if not, look below for help). Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please visit https://cla.developers.google.com/ to sign.

Once you've signed (or fixed any issues), please reply here with @googlebot I signed it! and we'll verify it.

What to do if you already signed the CLA

Individual signers

It's possible we don't have your GitHub username or you're using a different email address on your commit. Check your existing CLA data and verify that your email is set on your git commits.

Corporate signers

Your company has a Point of Contact who decides which employees are authorized to participate. Ask your POC to be added to the group of authorized contributors. If you don't know who your Point of Contact is, direct the Google project maintainer to go/cla#troubleshoot (Public version).
The email used to register you as an authorized contributor must be the email used for the Git commit. Check your existing CLA data and verify that your email is set on your git commits.
The email used to register you as an authorized contributor must also be attached to your GitHub account.

ℹ️ Googlers: Go here for more info.

k8s-ci-robot · 2020-07-28T23:16:59Z

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

dstnluong · 2020-07-29T00:52:21Z

/assign @RedbackThomson

components/aws/sagemaker/common/_utils.py

samples/contrib/aws-samples/debugger_component_demo/debugger-component-demo.py

…e more succinct, removed hardcoding from integration tests

google-cla · 2020-07-30T18:15:07Z

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project (if not, look below for help). Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please visit https://cla.developers.google.com/ to sign.

Once you've signed (or fixed any issues), please reply here with @googlebot I signed it! and we'll verify it.

What to do if you already signed the CLA

Individual signers

It's possible we don't have your GitHub username or you're using a different email address on your commit. Check your existing CLA data and verify that your email is set on your git commits.

Corporate signers

Your company has a Point of Contact who decides which employees are authorized to participate. Ask your POC to be added to the group of authorized contributors. If you don't know who your Point of Contact is, direct the Google project maintainer to go/cla#troubleshoot (Public version).
The email used to register you as an authorized contributor must be the email used for the Git commit. Check your existing CLA data and verify that your email is set on your git commits.
The email used to register you as an authorized contributor must also be attached to your GitHub account.

ℹ️ Googlers: Go here for more info.

components/aws/sagemaker/common/_utils.py

samples/contrib/aws-samples/debugger_component_demo/README.md

samples/contrib/aws-samples/debugger_component_demo/debugger-component-demo.py

…ple README, refactored _utils.py for fstrings and fixed offset for errors

google-cla · 2020-08-04T23:43:38Z

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project (if not, look below for help). Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please visit https://cla.developers.google.com/ to sign.

Once you've signed (or fixed any issues), please reply here with @googlebot I signed it! and we'll verify it.

What to do if you already signed the CLA

Individual signers

It's possible we don't have your GitHub username or you're using a different email address on your commit. Check your existing CLA data and verify that your email is set on your git commits.

Corporate signers

Your company has a Point of Contact who decides which employees are authorized to participate. Ask your POC to be added to the group of authorized contributors. If you don't know who your Point of Contact is, direct the Google project maintainer to go/cla#troubleshoot (Public version).
The email used to register you as an authorized contributor must be the email used for the Git commit. Check your existing CLA data and verify that your email is set on your git commits.
The email used to register you as an authorized contributor must also be attached to your GitHub account.

ℹ️ Googlers: Go here for more info.

google-cla · 2020-08-07T20:48:03Z

We found a Contributor License Agreement for you (the sender of this pull request), but were unable to find agreements for all the commit author(s) or Co-authors. If you authored these, maybe you used a different email address in the git commits than was used to sign the CLA (login here to double check)? If these were authored by someone else, then they will need to sign a CLA as well, and confirm that they're okay with these being contributed to Google.
In order to pass this check, please resolve this problem and then comment @googlebot I fixed it.. If the bot doesn't comment, it means it doesn't think anything has changed.

ℹ️ Googlers: Go here for more info.

google-cla · 2020-08-07T21:07:58Z

We found a Contributor License Agreement for you (the sender of this pull request), but were unable to find agreements for all the commit author(s) or Co-authors. If you authored these, maybe you used a different email address in the git commits than was used to sign the CLA (login here to double check)? If these were authored by someone else, then they will need to sign a CLA as well, and confirm that they're okay with these being contributed to Google.
In order to pass this check, please resolve this problem and then comment @googlebot I fixed it.. If the bot doesn't comment, it means it doesn't think anything has changed.

ℹ️ Googlers: Go here for more info.

akartsky · 2020-08-07T22:10:02Z

components/aws/sagemaker/tests/integration_tests/utils/__init__.py

@@ -8,6 +8,7 @@
 import shutil

 from sagemaker.amazon.amazon_estimator import get_image_uri


deprecated :
https://github.com/aws/sagemaker-python-sdk/blame/master/CHANGELOG.md#L38

update : https://github.com/kubeflow/pipelines/blob/master/components/aws/sagemaker/tests/integration_tests/environment.yml#L20

use image_uris.retrieve()

That deprecated version is for SageMaker SDK 2.0 @akartsky

you are right we can choose to keep using the old version

for some reason i thought this was added to _utils.py file as a new dependency so i thought of using the latest one https://github.com/kubeflow/pipelines/blob/master/components/aws/sagemaker/common/_utils.py#L39

surajkota · 2020-08-19T00:20:10Z

components/aws/sagemaker/common/_utils.py

+
+    :output Trainingjob-1234567890-RuleName-a1112312
+    """
+    job_name = rule_evaluation_job_arn.split("/")[1].split("-")


rule_evaluation_job_arn.split("/")[1] should be sufficient ?

also the prefix might not be always Trainingjob-

We want to replace the initial "trainingjob" with "Trainingjob" and change the rule name as well. There is an edge case where the rule name contains the "-" character, just something to note.

nit: this method is no longer needed

components/aws/sagemaker/common/_utils.py

components/aws/sagemaker/train/README.md

surajkota · 2020-08-19T00:33:06Z

components/aws/sagemaker/tests/unit_tests/requirements.txt

@RedbackThomson can an improvement to reduce duplication be part of your PR ?

Dustin had to change sagemaker version at multiple places

components/aws/sagemaker/tests/integration_tests/resources/definition/training_pipeline.py

…ther small issues

…nused function

surajkota · 2020-08-19T16:26:18Z

/ok-to-test
/approve

k8s-ci-robot · 2020-08-19T16:26:24Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: surajkota

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:

~~components/aws/sagemaker/OWNERS~~ [surajkota]
~~samples/contrib/aws-samples/OWNERS~~ [surajkota]

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

akartsky · 2020-08-19T20:32:45Z

/retest

akartsky · 2020-08-19T21:19:37Z

/retry

dstnluong · 2020-08-19T21:28:44Z

/retest

akartsky · 2020-08-19T22:12:37Z

/lgtm

…ining component to accept parameters related to Debugger (kubeflow#4283) * Implemented debugger for training component with sample pipeline, unit tests, and integration test * Implemented changes from PR, refactored utils.py, made sample pipeline more succinct, removed hardcoding from integration tests * Added default parameter for sample pipeline and fixed grammar for sample README, refactored _utils.py for fstrings and fixed offset for errors * Removed aws secret lines * Terminate debug rules when terminating training job, Terminate debug rules if terminate is pressed after training job has completed, added integration tests for stop_debug_rules, updated READMEs for train and sample, renamed sample pipeline, removed tensorboard, updated sagemaker version to sagemaker 2.1.0. * Terminate debug rules when terminating training job, Terminate debug rules if terminate is pressed after training job has completed, added integration tests for stop_debug_rules, updated READMEs for train and sample, renamed sample pipeline, removed tensorboard, updated sagemaker version to sagemaker 2.1.0. * Removed extra files, cleaned integration test * Changed integration test to use sample debugger pipeline * Processing jobs created from debug rules will not terminate, fixing other small issues * Removed debug from pipeline definition, removed extra line, removed unused function * Changelog and image tag updates

Implemented debugger for training component with sample pipeline, uni…

77b5e9d

…t tests, and integration test

k8s-ci-robot requested review from Jeffwan and mameshini July 28, 2020 23:16

google-cla bot added the cla: no label Jul 28, 2020

k8s-ci-robot added the size/XL label Jul 28, 2020

k8s-ci-robot added the needs-ok-to-test label Jul 28, 2020

k8s-ci-robot assigned RedbackThomson Jul 29, 2020

RedbackThomson reviewed Jul 29, 2020

View reviewed changes

Implemented changes from PR, refactored utils.py, made sample pipelin…

d107604

…e more succinct, removed hardcoding from integration tests

RedbackThomson reviewed Aug 4, 2020

View reviewed changes

Added default parameter for sample pipeline and fixed grammar for sam…

a22249e

…ple README, refactored _utils.py for fstrings and fixed offset for errors

RedbackThomson approved these changes Aug 5, 2020

View reviewed changes

Removed aws secret lines

09664d3

google-cla bot added cla: yes and removed cla: no labels Aug 7, 2020

google-cla bot added cla: no and removed cla: yes labels Aug 7, 2020

dstnluong force-pushed the debugcomponent branch from 15d1861 to 09664d3 Compare August 7, 2020 21:12

google-cla bot added cla: yes and removed cla: no labels Aug 7, 2020

Merge remote-tracking branch 'upstream/master' into debugcomponent

d1c3691

akartsky reviewed Aug 7, 2020

View reviewed changes

dstnluong force-pushed the debugcomponent branch 3 times, most recently from ac4f2a5 to 8c95a04 Compare August 18, 2020 04:50

k8s-ci-robot added size/XXL and removed size/XL labels Aug 18, 2020

dstnluong force-pushed the debugcomponent branch from a69c442 to 8c95a04 Compare August 18, 2020 05:00

k8s-ci-robot added size/XL and removed size/XXL labels Aug 18, 2020

Changed integration test to use sample debugger pipeline

63b5f73

surajkota reviewed Aug 19, 2020

View reviewed changes

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

surajkota reviewed Aug 19, 2020

View reviewed changes

components/aws/sagemaker/train/README.md Show resolved Hide resolved

surajkota reviewed Aug 19, 2020

View reviewed changes

components/aws/sagemaker/tests/integration_tests/resources/definition/training_pipeline.py Outdated Show resolved Hide resolved

dstnluong added 2 commits August 18, 2020 18:13

Processing jobs created from debug rules will not terminate, fixing o…

41f228f

…ther small issues

Removed debug from pipeline definition, removed extra line, removed u…

22b180b

…nused function

k8s-ci-robot added ok-to-test and removed needs-ok-to-test labels Aug 19, 2020

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

Changelog and image tag updates

495b33c

k8s-ci-robot assigned akartsky Aug 19, 2020

k8s-ci-robot added the lgtm label Aug 19, 2020

k8s-ci-robot merged commit 3ebd075 into kubeflow:master Aug 19, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(components): AWS SageMaker - Add optional parameter to allow training component to accept parameters related to Debugger #4283

feat(components): AWS SageMaker - Add optional parameter to allow training component to accept parameters related to Debugger #4283

dstnluong commented Jul 28, 2020 •

edited

Loading

kubeflow-bot commented Jul 28, 2020

google-cla bot commented Jul 28, 2020

k8s-ci-robot commented Jul 28, 2020

dstnluong commented Jul 29, 2020

google-cla bot commented Jul 30, 2020

google-cla bot commented Aug 4, 2020

google-cla bot commented Aug 7, 2020

google-cla bot commented Aug 7, 2020

akartsky Aug 7, 2020 •

edited

Loading

RedbackThomson Aug 10, 2020

akartsky Aug 11, 2020

surajkota Aug 19, 2020 •

edited

Loading

dstnluong Aug 19, 2020

surajkota Aug 19, 2020

surajkota Aug 19, 2020

surajkota commented Aug 19, 2020

k8s-ci-robot commented Aug 19, 2020

akartsky commented Aug 19, 2020

akartsky commented Aug 19, 2020

dstnluong commented Aug 19, 2020

akartsky commented Aug 19, 2020

		@@ -8,6 +8,7 @@
		import shutil

		from sagemaker.amazon.amazon_estimator import get_image_uri

feat(components): AWS SageMaker - Add optional parameter to allow training component to accept parameters related to Debugger #4283

feat(components): AWS SageMaker - Add optional parameter to allow training component to accept parameters related to Debugger #4283

Conversation

dstnluong commented Jul 28, 2020 • edited Loading

kubeflow-bot commented Jul 28, 2020

google-cla bot commented Jul 28, 2020

What to do if you already signed the CLA

Individual signers

Corporate signers

k8s-ci-robot commented Jul 28, 2020

dstnluong commented Jul 29, 2020

google-cla bot commented Jul 30, 2020

What to do if you already signed the CLA

Individual signers

Corporate signers

google-cla bot commented Aug 4, 2020

What to do if you already signed the CLA

Individual signers

Corporate signers

google-cla bot commented Aug 7, 2020

google-cla bot commented Aug 7, 2020

akartsky Aug 7, 2020 • edited Loading

Choose a reason for hiding this comment

RedbackThomson Aug 10, 2020

Choose a reason for hiding this comment

akartsky Aug 11, 2020

Choose a reason for hiding this comment

surajkota Aug 19, 2020 • edited Loading

Choose a reason for hiding this comment

dstnluong Aug 19, 2020

Choose a reason for hiding this comment

surajkota Aug 19, 2020

Choose a reason for hiding this comment

surajkota Aug 19, 2020

Choose a reason for hiding this comment

surajkota commented Aug 19, 2020

k8s-ci-robot commented Aug 19, 2020

akartsky commented Aug 19, 2020

akartsky commented Aug 19, 2020

dstnluong commented Aug 19, 2020

akartsky commented Aug 19, 2020

dstnluong commented Jul 28, 2020 •

edited

Loading

akartsky Aug 7, 2020 •

edited

Loading

surajkota Aug 19, 2020 •

edited

Loading