feat(pytorch): Support elastic training #1453

gaocegege · 2021-10-28T08:09:26Z

This PR:

Updates the kubeflow/common to 0.4.1 to add LabelSelector in status, which supports scale subresources
Implements ElasticPolicy in PyTorchJob
Adds two elastic examples

This PR does not:

Fixes the signal problem in PyTorch, which may fail the entire PyTorchJob improperly [torch/elastic] Scale down does not work correctly when agent is killed with SIGINT, SIGTERM pytorch/pytorch#67742 (The upstream PR is merged but it is not released in PyTorch 1.10)

aws-kf-ci-bot · 2021-10-28T08:09:33Z

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

google-oss-robot · 2021-10-28T08:09:43Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: gaocegege

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~~ [gaocegege]

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

coveralls · 2021-11-05T07:16:10Z

Pull Request Test Coverage Report for Build 1505952988

220 of 568 (38.73%) changed or added relevant lines in 12 files are covered.
1 unchanged line in 1 file lost coverage.
Overall coverage increased (+2.6%) to 26.627%

Changes Missing Coverage	Covered Lines	Changed/Added Lines	%
pkg/controller.v1/xgboost/xgboostjob_controller.go	0	1	0.0%
pkg/controller.v1/pytorch/master.go	21	23	91.3%
pkg/apis/pytorch/v1/defaults.go	16	21	76.19%
pkg/controller.v1/pytorch/pytorch.go	27	33	81.82%
pkg/apis/mxnet/v1/openapi_generated.go	0	8	0.0%
pkg/apis/tensorflow/v1/openapi_generated.go	0	8	0.0%
pkg/apis/xgboost/v1/openapi_generated.go	0	8	0.0%
pkg/controller.v1/pytorch/elastic.go	95	117	81.2%
pkg/controller.v1/pytorch/pytorchjob_controller.go	53	77	68.83%
pkg/controller.v1/pytorch/hpa.go	8	56	14.29%

Files with Coverage Reduction	New Missed Lines	%
pkg/apis/pytorch/validation/validation.go	1	91.67%

Totals
Change from base Build 1493027296:	2.6%
Covered Lines:	1600
Relevant Lines:	6009

💛 - Coveralls

gaocegege · 2021-11-12T07:03:56Z

46eeff9 implemented the feature.

Overall coverage increased (+7.1%) to 15.252%, PyTorch related test coverage is increased from 0% to 80%

PTAL

go.mod

Signed-off-by: Ce Gao <ce.gao@outlook.com>

gaocegege · 2021-11-24T07:57:06Z

The PR is ready to review. PTAL

/assign @kubeflow/wg-training-leads

gaocegege · 2021-11-24T07:58:27Z

/assign @zw0610

zw0610 · 2021-11-24T07:59:14Z

I suppose it's ready to review, right?

gaocegege · 2021-11-24T07:59:33Z

�Yeah it is ready to review.

terrytangyuan

/cc @zw0610

Signed-off-by: Ce Gao <ce.gao@outlook.com>

gaocegege · 2021-11-25T07:59:33Z

Close #1483

zw0610 · 2021-11-25T14:14:26Z

examples/pytorch/elastic/etcd.yaml

+---
+
+apiVersion: v1
+kind: Pod


with only 1 pod for etcd instead of 3?

is this etcd instance deployed per (elastic) pytorch job or all elastic pytorch job will shall the etcd instance?

You can share one. And u do not need one if you are using c10d

examples/pytorch/elastic/imagenet/Dockerfile

examples/pytorch/elastic/imagenet/imagenet.yaml

zw0610 · 2021-11-25T14:22:50Z

manifests/base/crds/kubeflow.org_tfjobs.yaml

@@ -6916,6 +6916,53 @@ spec:
                      description: The number of pods which reached phase Failed.
                      format: int32
                      type: integer
+                    labelSelector:


while I do understand this change will definitely be introduced after we update the kubeflow/common apis definition, may be we should let other apis/controller to accept this changes in another pr.

If other operators does not support scale subresource, then it is not needed. Do you mean we should support the subresource for other operators?

After the update to kubeflow/common, every time we run make generates in this repo, it shall always add labelSelector to other APIs. I would suggest to ignore such changes from APIs except PyTorchJob in this pr.

It is hard to ignore it since the code generator reads the API definition and generate the corresponding code automatically. And the field labelSelector is a pointer thus it is optional. It does not affect existing CRDs

pkg/apis/pytorch/v1/types.go

pkg/controller.v1/pytorch/hpa.go

hack/update-codegen.sh

pkg/controller.v1/pytorch/pytorch.go

Signed-off-by: Ce Gao <ce.gao@outlook.com>

gaocegege · 2021-11-26T03:27:59Z

The comments are addressed, PTAL @zw0610

zw0610

/lgtm

google-oss-prow · 2021-11-26T06:35:10Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: gaocegege, zw0610

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~~ [gaocegege]

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

google-oss-robot added the do-not-merge/work-in-progress label Oct 28, 2021

google-oss-robot added the size/L label Oct 28, 2021

google-oss-robot added the approved label Oct 28, 2021

google-oss-robot requested review from ChanYiLin, jinchihe and terrytangyuan October 28, 2021 08:09

google-oss-robot added size/XXL and removed size/L labels Oct 29, 2021

gaocegege force-pushed the pytorchelastic branch from df2c1ed to cdfd1be Compare November 4, 2021 08:31

gaocegege force-pushed the pytorchelastic branch 3 times, most recently from 2bfbb3d to 46eeff9 Compare November 12, 2021 07:01

gaocegege marked this pull request as ready for review November 12, 2021 07:04

google-oss-prow bot removed the do-not-merge/work-in-progress label Nov 12, 2021

gaocegege commented Nov 12, 2021

View reviewed changes

go.mod Outdated Show resolved Hide resolved

This was referenced Nov 12, 2021

[request] Do we have plan to merge Kubernetes part to kubeflow/pytorch-operator? pytorch/elastic#117

Open

feat(pytorch): Add elastic proposal kubeflow/community#522

Open

gaocegege mentioned this pull request Nov 23, 2021

feat: Add LabelSelector kubeflow/common#177

Merged

gaocegege added 4 commits November 24, 2021 11:32

chore: Update codegen and vendor

e704497

Signed-off-by: Ce Gao <ce.gao@outlook.com>

fix: Update git config

be25172

Signed-off-by: Ce Gao <ce.gao@outlook.com>

fix: Codegen

04538cc

Signed-off-by: Ce Gao <ce.gao@outlook.com>

feat: Add elastic examples

3790fd2

Signed-off-by: Ce Gao <ce.gao@outlook.com>

gaocegege force-pushed the pytorchelastic branch from 46eeff9 to b963cbe Compare November 24, 2021 03:32

gaocegege added 3 commits November 24, 2021 15:46

chore: Update go mod

284e980

Signed-off-by: Ce Gao <ce.gao@outlook.com>

chore: Update manifest

fd03f1b

Signed-off-by: Ce Gao <ce.gao@outlook.com>

feat: Implement elstic policy

00a5c89

Signed-off-by: Ce Gao <ce.gao@outlook.com>

gaocegege force-pushed the pytorchelastic branch from 3763152 to 00a5c89 Compare November 24, 2021 07:47

chore: Update vendor

8e2db9c

Signed-off-by: Ce Gao <ce.gao@outlook.com>

gaocegege mentioned this pull request Nov 24, 2021

[bug] Missing init container in PyTorchJob #1482

Closed

google-oss-prow bot assigned zw0610 Nov 24, 2021

terrytangyuan reviewed Nov 25, 2021

View reviewed changes

google-oss-prow bot requested a review from zw0610 November 25, 2021 06:39

chore: Update swagger

877110d

Signed-off-by: Ce Gao <ce.gao@outlook.com>

zw0610 reviewed Nov 25, 2021

View reviewed changes

gaocegege force-pushed the pytorchelastic branch from a69a7a6 to 2fc6c57 Compare November 26, 2021 02:25

fix: Address comments

2940450

Signed-off-by: Ce Gao <ce.gao@outlook.com>

gaocegege force-pushed the pytorchelastic branch from 2fc6c57 to 2940450 Compare November 26, 2021 02:32

zw0610 approved these changes Nov 26, 2021

View reviewed changes

google-oss-prow bot added the lgtm label Nov 26, 2021

google-oss-prow bot merged commit 3e11ac3 into kubeflow:master Nov 26, 2021

gaocegege deleted the pytorchelastic branch November 26, 2021 06:35

johnugeorge mentioned this pull request Dec 20, 2022

HPA support for PyTorch Elastic #1701

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(pytorch): Support elastic training #1453

feat(pytorch): Support elastic training #1453

gaocegege commented Oct 28, 2021 •

edited

Loading

aws-kf-ci-bot commented Oct 28, 2021

google-oss-robot commented Oct 28, 2021

coveralls commented Nov 5, 2021 •

edited

Loading

gaocegege commented Nov 12, 2021

gaocegege commented Nov 24, 2021

gaocegege commented Nov 24, 2021

zw0610 commented Nov 24, 2021

gaocegege commented Nov 24, 2021

terrytangyuan left a comment

gaocegege commented Nov 25, 2021

zw0610 Nov 25, 2021

gaocegege Nov 26, 2021

zw0610 Nov 25, 2021

gaocegege Nov 26, 2021

zw0610 Nov 26, 2021

gaocegege Nov 26, 2021

gaocegege commented Nov 26, 2021

zw0610 left a comment

google-oss-prow bot commented Nov 26, 2021

feat(pytorch): Support elastic training #1453

feat(pytorch): Support elastic training #1453

Conversation

gaocegege commented Oct 28, 2021 • edited Loading

aws-kf-ci-bot commented Oct 28, 2021

google-oss-robot commented Oct 28, 2021

coveralls commented Nov 5, 2021 • edited Loading

Pull Request Test Coverage Report for Build 1505952988

💛 - Coveralls

gaocegege commented Nov 12, 2021

gaocegege commented Nov 24, 2021

gaocegege commented Nov 24, 2021

zw0610 commented Nov 24, 2021

gaocegege commented Nov 24, 2021

terrytangyuan left a comment

Choose a reason for hiding this comment

gaocegege commented Nov 25, 2021

zw0610 Nov 25, 2021

Choose a reason for hiding this comment

gaocegege Nov 26, 2021

Choose a reason for hiding this comment

zw0610 Nov 25, 2021

Choose a reason for hiding this comment

gaocegege Nov 26, 2021

Choose a reason for hiding this comment

zw0610 Nov 26, 2021

Choose a reason for hiding this comment

gaocegege Nov 26, 2021

Choose a reason for hiding this comment

gaocegege commented Nov 26, 2021

zw0610 left a comment

Choose a reason for hiding this comment

google-oss-prow bot commented Nov 26, 2021

gaocegege commented Oct 28, 2021 •

edited

Loading

coveralls commented Nov 5, 2021 •

edited

Loading