add support for using Intel MPI(2019.7) and MVAPICH2 #283

milkybird98 · 2020-07-27T09:04:50Z

Ther major differences in the launching process between the ompi, intel-mpi and mvapich are:

the envrionment variable name
the ssh/rsh launching command and hostfile format

For intel-mpi and mvapich, hydra process manager is used, instead of orted. However, there's no major difference in the ssh/rsh command that launches a remote process manager, with the only thing needed caring is the position of remote pod name arugment.
(Unfortunately, it seems that in the intel-mpi@2020 version, the ssh/rsh command has changed again and the prefix arguments "-q -x" have been abandoned. It's such a mess.)

As for the the envrionment variable name, both intel-mpi and mvapich have the samily functional one.

And as I have not found a effective way to auto-detect the mpi distribution, so a new argument is added to the api.

there're some docker images that I build and use to test the code.
https://hub.docker.com/repository/docker/milkybird98/ompi-osu
https://hub.docker.com/repository/docker/milkybird98/intel-mpi-osu
https://hub.docker.com/repository/docker/milkybird98/mvapich2-osu

+ local minikube test pass + add new Spec "mpiDistribution" @ 2020/7/27

* change email address

kubeflow-bot · 2020-07-27T09:04:58Z

This change is

k8s-ci-robot · 2020-07-27T09:05:02Z

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

terrytangyuan

I'll leave this open for others to comment as well. It might be better to think about how to make this framework-agnostic rather than having to maintain the logic for various frameworks.

terrytangyuan · 2020-07-27T18:13:15Z

pkg/apis/kubeflow/v1alpha2/types.go

+
+	// MPIDistribution specifies name of the mpi framwork which is used
+	// Deafults to "OpenMPI"
+	// Option includes "OpenMPI", "IntelMPI" and "MPICH"


Option -> Options

Perhaps make them all lowercase and separate words with underscores?

terrytangyuan · 2020-07-27T18:17:14Z

pkg/apis/kubeflow/v1alpha2/types.go

@@ -72,6 +72,11 @@ type MPIJobSpec struct {
 	// active. The policies specified in `RunPolicy` take precedence over
 	// the following fields: `BackoffLimit` and `ActiveDeadlineSeconds`.
 	RunPolicy *common.RunPolicy `json:"runPolicy,omitempty"`
+
+	// MPIDistribution specifies name of the mpi framwork which is used


terrytangyuan · 2020-07-27T18:17:21Z

pkg/apis/kubeflow/v1alpha2/types.go

@@ -72,6 +72,11 @@ type MPIJobSpec struct {
 	// active. The policies specified in `RunPolicy` take precedence over
 	// the following fields: `BackoffLimit` and `ActiveDeadlineSeconds`.
 	RunPolicy *common.RunPolicy `json:"runPolicy,omitempty"`
+
+	// MPIDistribution specifies name of the mpi framwork which is used
+	// Deafults to "OpenMPI"


terrytangyuan · 2020-07-27T18:19:01Z

pkg/controllers/v1alpha2/mpi_job_controller.go

+		kubexec = fmt.Sprintf(`#!/bin/sh
+set -x
+POD_NAME=$3
+shift 3


Could you add some notes in the code on the differences?

* update notes about hostfile generating

gaocegege

Thanks for your contribution! 🎉 👍

gaocegege · 2020-07-28T01:53:48Z

pkg/apis/kubeflow/v1alpha2/types.go

+	// MPIDistribution specifies name of the MPI framwork which is used
+	// Defaults to "open_mpi"
+	// Options includes "open_mpi", "intel_mpi" and "mpich"
+	MPIDistribution string `json:"mpiDistribution,omitempty"`


Can we define it as a type instead of a string? (PS, personally, prefer OpenMPI, IntelMPI and MPICH here)

type MPIDistribution string const ( MPIDistributionOpenMPI = "openMPI" ... )

milkybird98 · 2020-07-28T09:13:39Z

I'm sorry that, because for the convenience of generating docker images, I used two separate directories to generate mpi-operator and kebuctl-delivery images.
I found that I did not commit the changed code used to generate the kebuctl-delivery image.
And some functions have been moved to the mpi-operator controller code in order to implement functions more efficiently.

+ move hosts sending and merging here * use special type instaed of string

gaocegege

/assign @carmark @zw0610

pkg/apis/kubeflow/v1alpha2/types.go

k8s-ci-robot · 2020-07-28T09:36:57Z

@gaocegege: GitHub didn't allow me to assign the following users: zw0610.

Note that only kubeflow members, repo collaborators and people who have commented on this issue/PR can be assigned. Additionally, issues/PRs can only have 10 assignees at the same time.
For more information please see the contributor guide

In response to this:

/assign @carmark @zw0610

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.

gaocegege · 2020-07-28T09:50:06Z

/ok-to-test
/lgtm

carmark · 2020-07-31T01:26:24Z

@milkybird98 Could you please add some unit test to make coverage/coveralls happy?

coverage/coveralls — Coverage decreased (-4.04%) to 21.788%

milkybird98 · 2020-07-31T08:52:42Z

No problem. I'll take care of it this weekend.

gaocegege · 2020-08-03T07:54:51Z

/lgtm
/assign @terrytangyuan @carmark

carmark · 2020-08-03T08:01:42Z

/lgtm

terrytangyuan · 2020-08-03T11:27:09Z

/approve

k8s-ci-robot · 2020-08-03T11:27:16Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: terrytangyuan

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

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

milkybird98 and others added 3 commits July 27, 2020 12:25

+ support for IntelMPI and MPICH

642cb02

+ local minikube test pass + add new Spec "mpiDistribution" @ 2020/7/27

* fix ineffectual assignment

b3c5dcc

* change email address

* update variable name

2be7003

k8s-ci-robot requested review from carmark and everpeace July 27, 2020 09:04

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

k8s-ci-robot added the size/M label Jul 27, 2020

terrytangyuan reviewed Jul 27, 2020

View reviewed changes

milkybird98 added 3 commits July 28, 2020 09:06

* fix some spelling and naming problems

603ec8d

+ add more notes

35c16a5

+ auto filter prefix parameters

d2dad84

milkybird98 force-pushed the master branch from e233971 to d2dad84 Compare July 28, 2020 01:29

* fix some spelling problem

a996d60

* update notes about hostfile generating

gaocegege reviewed Jul 28, 2020

View reviewed changes

milkybird98 added 3 commits July 28, 2020 17:05

+ mpich-format hostfile split

d2af63a

+ generate hosts for hydra to resolve hostname

a7a3d61

* update notes

af47f8f

milkybird98 added 2 commits July 28, 2020 17:19

* fix sh script

03d687f

+ move hosts sending and merging here * use special type instaed of string

* check return value

ba72818

k8s-ci-robot added size/L and removed size/M labels Jul 28, 2020

gaocegege reviewed Jul 28, 2020

View reviewed changes

pkg/apis/kubeflow/v1alpha2/types.go Outdated Show resolved Hide resolved

pkg/apis/kubeflow/v1alpha2/types.go Show resolved Hide resolved

k8s-ci-robot assigned carmark Jul 28, 2020

* update options' name

50aaf5a

k8s-ci-robot assigned gaocegege Jul 28, 2020

k8s-ci-robot added ok-to-test lgtm and removed needs-ok-to-test labels Jul 28, 2020

+ add unit test for generateHosts

aa89133

k8s-ci-robot removed the lgtm label Aug 2, 2020

^ fixed lint reported errors

e4bfa60

k8s-ci-robot assigned terrytangyuan Aug 3, 2020

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

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

k8s-ci-robot merged commit 07bbb45 into kubeflow:master Aug 3, 2020

This was referenced May 17, 2023

Add default Intel MPI env variables to MPIJob kubeflow/training-operator#1804

Merged

IntelMPI support kubeflow/training-operator#1807

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

add support for using Intel MPI(2019.7) and MVAPICH2 #283

add support for using Intel MPI(2019.7) and MVAPICH2 #283

milkybird98 commented Jul 27, 2020

kubeflow-bot commented Jul 27, 2020

k8s-ci-robot commented Jul 27, 2020

terrytangyuan left a comment

terrytangyuan Jul 27, 2020

terrytangyuan Jul 27, 2020

terrytangyuan Jul 27, 2020

terrytangyuan Jul 27, 2020

gaocegege left a comment

gaocegege Jul 28, 2020

milkybird98 commented Jul 28, 2020

gaocegege left a comment

k8s-ci-robot commented Jul 28, 2020

gaocegege commented Jul 28, 2020

carmark commented Jul 31, 2020

milkybird98 commented Jul 31, 2020

gaocegege commented Aug 3, 2020

carmark commented Aug 3, 2020

terrytangyuan commented Aug 3, 2020

k8s-ci-robot commented Aug 3, 2020

add support for using Intel MPI(2019.7) and MVAPICH2 #283

add support for using Intel MPI(2019.7) and MVAPICH2 #283

Conversation

milkybird98 commented Jul 27, 2020

kubeflow-bot commented Jul 27, 2020

k8s-ci-robot commented Jul 27, 2020

terrytangyuan left a comment

Choose a reason for hiding this comment

terrytangyuan Jul 27, 2020

Choose a reason for hiding this comment

terrytangyuan Jul 27, 2020

Choose a reason for hiding this comment

terrytangyuan Jul 27, 2020

Choose a reason for hiding this comment

terrytangyuan Jul 27, 2020

Choose a reason for hiding this comment

gaocegege left a comment

Choose a reason for hiding this comment

gaocegege Jul 28, 2020

Choose a reason for hiding this comment

milkybird98 commented Jul 28, 2020

gaocegege left a comment

Choose a reason for hiding this comment

k8s-ci-robot commented Jul 28, 2020

gaocegege commented Jul 28, 2020

carmark commented Jul 31, 2020

milkybird98 commented Jul 31, 2020

gaocegege commented Aug 3, 2020

carmark commented Aug 3, 2020

terrytangyuan commented Aug 3, 2020

k8s-ci-robot commented Aug 3, 2020