DLTS integration #1945

Gerhut · 2020-01-10T12:24:11Z

This PR added DLTS integration to NNI.

Key Points:

Run both NNI mamager / NNI Trial in DLTS job, as manager job / trial job.
User should copy the trial code / NNI config to Samba, which will be synced to NFS by DLTS.
Both manager job and trial job will share the same NFS mounting, includes user's trial code and config, so no copy operation needed.

Depends on DLWorkspace#837

chicm-ms · 2020-02-20T03:08:28Z

src/nni_manager/training_service/dlts/dltsJobConfig.ts

@@ -0,0 +1,42 @@
+import { DLTSClusterConfig } from "./dltsClusterConfig";


Would you please add license at file beginning and a empty line at end?

chicm-ms · 2020-02-20T03:21:39Z

src/nni_manager/training_service/dlts/dltsTrainingService.ts

+            this.dltsRestServerPort = restServer.clusterRestServerPort;
+        }
+
+        // Step 1. Prepare PAI job configuration


This comment is out of date

chicm-ms · 2020-02-20T03:27:52Z

src/nni_manager/training_service/dlts/dltsTrainingService.ts

+        const parameterFileMeta = {
+            experimentId: this.experimentId,
+            trialId: trialJobId,
+            // filePath: hdfsHpFilePath


remove the commented field?

SparkSnail · 2020-02-20T07:03:35Z

examples/trials/mnist-tfv1/config_dlts.yml

+    #choice: maximize, minimize
+    optimize_mode: maximize
+trial:
+  command: python3 /work/nni-code/mnist.py


python3 mninst.py

Done and tested

examples/trials/mnist-tfv1/config_dlts.yml

chicm-ms · 2020-02-20T07:10:24Z

Will you add document about dlts training service?

SparkSnail · 2020-02-20T07:12:41Z

src/nni_manager/training_service/dlts/dltsTrainingService.ts

+                break;
+            case 'pausing':
+                dltsTrialJob.status = "RUNNING";
+                dltsTrialJob.status = "RUNNING";


duplicated line. If the job in dlts is in 'pausing' status, means the job is running?

In DLTS job could be temporary "paused" (stop, release resources but easy to restart). I have not found a more suitable NNI status here.

@Gerhut just curious, how users restart a paused job? the job starts from the beginning or could automatically resume the state?

Starts from beginning, user should handle checkpoints by themselves.

You could add a new status in NNI, I think RUNNING is not suitable here, since the job is not running.

Can I make it WAITING?

I think it is different from 'waiting'. if we show this state, users will be confused: why a job becomes waiting again after running.

I'm not sure how much affection if I introduced a new state into NNI? At least shall I provide a new style of the PAUSED state to the NNI dashboard?

as we discussed, if a job is in PAUSED state, simply cancel it.

src/nni_manager/rest_server/restValidationSchemas.ts

QuanluZhang · 2020-02-20T07:19:43Z

src/nni_manager/training_service/common/trialConfigMetadataKey.ts

    VERSION_CHECK = 'version_check',
-    LOG_COLLECTION = 'log_collection'
+    LOG_COLLECTION = 'log_collection',


remove the comma

SparkSnail · 2020-02-20T07:22:41Z

src/nni_manager/training_service/dlts/dltsTrainingService.ts

+        while (!this.stopping) {
+            while (!this.stopping && this.jobQueue.length > 0) {
+                const trialJobId: string = this.jobQueue[0];
+                this.log.info('Got job ' + trialJobId);


format to Got job ${trialJobId}

SparkSnail · 2020-02-20T07:36:21Z

src/nni_manager/training_service/dlts/dltsTrainingService.ts

+                if (!this.dltsClusterConfig.cluster) {
+                    this.dltsClusterConfig.cluster = '.default'
+                }
+                if (!this.dltsClusterConfig.email && process.env['DLWS_USER_EMAIL']) {


If users does not set email in config file, and the environment does not contain DLWS_USER_EMAIL, what will happen?

403 Forbidden

Suggest add validation in nnictl, and give meaningful error information.

SparkSnail · 2020-02-20T07:39:10Z

src/nni_manager/training_service/dlts/dltsTrainingService.ts

+                        resolve(gpus[0])
+                    })
+                });
+                this.dltsClusterConfig.gpuType = gpu;


Can users set gpuType in config file?

Yes and no since there is only one choice at present, but it have to be configured correctly or DLTS could not schedule it correctly.

DLTS plans to support cluster with multiple gpuTypes in the future.

SparkSnail · 2020-02-20T07:45:06Z

src/nni_manager/training_service/dlts/dltsTrainingService.ts

+        this.log.info('Stopping DLTS training service...');
+        this.stopping = true;
+
+        const deferred: Deferred<void> = new Deferred<void>();


remove Deferred, refer

SparkSnail · 2020-02-20T07:50:25Z

src/nni_manager/training_service/dlts/dltsTrainingService.ts

+        const version: string = this.versionCheck ? await getVersion() : '';
+        const nniDLTSTrialCommand: string = String.Format(
+            DLTS_TRIAL_COMMAND_FORMAT,
+            trialLocalFolder,


Why is NNI_SYS_DIR a local folder? Is the trial job running in local?

Both manager job and trial job will share the same NFS mounting, they share the same directory structure.

Is the path which const trialLocalFolder = path.join(getExperimentRootDir(), 'trials-local', trialJobId) specified a NFS sharing folder? Does users need to know this folder path and execute mount command before they start NNI job?

TLDR; we will provide a NNI manager job template in DLTS to handle these mounting for user.

In my understanding, there will be two directories in use in NNI:

Config directory, since users have to put config / codes to samba directory first, which will synced to NFS by DLTS.

tempdir to generate transpiled trial code, we temporarily set TMPDIR=/work/tmp to make NNI generate transpiled code to mounted directories, it's better to make it configurable.

src/nni_manager/training_service/dlts/dltsTrainingService.ts

SparkSnail · 2020-02-20T07:56:02Z

src/nni_manager/training_service/dlts/dltsTrialConfig.ts

+
+export class DLTSTrialConfig extends TrialConfig {
+  public constructor(
+    command: string,


Unify code style, all fields use public statement, or all of them does not use public.

image is actually Parameter properties and other 3 will be passed to the super constructor.

src/nni_manager/training_service/dlts/dltsTrainingService.ts

Gerhut · 2020-02-20T08:35:36Z

How did you workaround eslint/eslint#11899 ? I have lots of these lint errors currently.

scarlett2018 · 2020-02-20T10:09:41Z

docs/en_US/TrainingService/DLTSMode.md

@@ -0,0 +1,52 @@
+**Run an Experiment on Deep Learning Training Service**


shall we use the official name (DLWorkspace) of the project here? or the well known aka. DLTS.

Ping @hongzhili for suggestion.

Changed to DLTS

scarlett2018 · 2020-02-20T10:10:11Z

docs/en_US/TrainingService/DLTSMode.md

@@ -0,0 +1,52 @@
+**Run an Experiment on Deep Learning Training Service**
+===
+NNI supports running an experiment on [Deep Learning Training Service](https://github.com/microsoft/DLWorkspace.git) (aka DLTS), called dlts mode. Before starting to use NNI dlts mode, you should have an account to access DLTS dashboard.


[Deep Learning Training Service]
same comments about this name usage.

Changed to DLTS

scarlett2018 · 2020-02-20T10:13:43Z

docs/en_US/TrainingService/DLTSMode.md

+
+Step 2. Prepare a NNI config YAML like the following:
+
+```yaml


instead of directly post an yaml example, you might want to outline the new field like trainingServicePlatform: dlts, or additional keys comparing to LocalMode and RemoteMachineMode, refer to https://github.com/microsoft/nni/blob/master/docs/en_US/TrainingService/PaiMode.md

Removed all comments here other than DLTS specified ones.

scarlett2018 · 2020-02-20T10:14:15Z

docs/en_US/TrainingService/DLTSMode.md

+
+![Submit Job](../../img/dlts-step4.png)
+
+Step 5. Go to Endpoints tab of the newly created job, click the Port 40000 link to theck trial's information.


theck
typo?

chicm-ms · 2020-02-26T04:11:11Z

Would you please fix following eslint error reported by IT pipeline?

/home/vsts/work/1/s/src/nni_manager/training_service/dlts/dltsTrainingService.ts
   72:25  warning  'trialJobId' is assigned a value but never used                                                                               @typescript-eslint/no-unused-vars
  111:17  error    Possible race condition: `dltsTrialJob.status` might be reassigned based on an outdated value of `dltsTrialJob.status`        require-atomic-updates
  114:17  error    Possible race condition: `dltsTrialJob.status` might be reassigned based on an outdated value of `dltsTrialJob.status`        require-atomic-updates
  116:21  error    Possible race condition: `dltsTrialJob.startTime` might be reassigned based on an outdated value of `dltsTrialJob.startTime`  require-atomic-updates
  119:21  error    Possible race condition: `dltsTrialJob.url` might be reassigned based on an outdated value of `dltsTrialJob.url`              require-atomic-updates
  123:17  error    Possible race condition: `dltsTrialJob.status` might be reassigned based on an outdated value of `dltsTrialJob.status`        require-atomic-updates
  126:17  error    Possible race condition: `dltsTrialJob.status` might be reassigned based on an outdated value of `dltsTrialJob.status`        require-atomic-updates
  130:17  error    Possible race condition: `dltsTrialJob.status` might be reassigned based on an outdated value of `dltsTrialJob.status`        require-atomic-updates
  135:21  error    Possible race condition: `dltsTrialJob.status` might be reassigned based on an outdated value of `dltsTrialJob.status`        require-atomic-updates
  138:21  error    Possible race condition: `dltsTrialJob.status` might be reassigned based on an outdated value of `dltsTrialJob.status`        require-atomic-updates
  142:17  error    Possible race condition: `dltsTrialJob.status` might be reassigned based on an outdated value of `dltsTrialJob.status`        require-atomic-updates
  334:37  warning  'key' is defined but never used                                                                                               @typescript-eslint/no-unused-vars
  474:39  warning  'res' is defined but never used

QuanluZhang and others added 2 commits February 18, 2020 13:41

skeleton of dlts training service (microsoft#1844)

08d31c0

Hello, DLTS!

5426c11

Gerhut force-pushed the dlts-integration branch from 24e3fde to 5426c11 Compare February 18, 2020 05:47

Qixiang Cheng added 10 commits February 18, 2020 13:57

Revert version

c84bdc5

Remove fs-extra

a60ca84

Add some default cluster config

56cc475

schema

7f97021

fix

601f6a9

Optional cluster (default to .default)

2cbda0a

Depends on DLWorkspace#837

fix

38d7e89

fix

c08da76

optimize gpu type

d5933d5

No more copy

2f1beb2

Gerhut requested review from SparkSnail, QuanluZhang and scarlett2018 February 19, 2020 14:11

Gerhut marked this pull request as ready for review February 19, 2020 14:12

QuanluZhang requested a review from chicm-ms February 20, 2020 00:38

chicm-ms reviewed Feb 20, 2020

View reviewed changes

Qixiang Cheng added 2 commits February 20, 2020 14:58

Format

b58ae28

Code clean up

bca20fd

SparkSnail reviewed Feb 20, 2020

View reviewed changes

examples/trials/mnist-tfv1/config_dlts.yml Show resolved Hide resolved

SparkSnail reviewed Feb 20, 2020

View reviewed changes

QuanluZhang reviewed Feb 20, 2020

View reviewed changes

src/nni_manager/rest_server/restValidationSchemas.ts Show resolved Hide resolved

Issue fix

01b553c

QuanluZhang reviewed Feb 20, 2020

View reviewed changes

SparkSnail reviewed Feb 20, 2020

View reviewed changes

Gerhut added 2 commits February 20, 2020 15:45

Add optional fields in config

fc611ec

Issue fix

7251af0

SparkSnail reviewed Feb 20, 2020

View reviewed changes

QuanluZhang reviewed Feb 20, 2020

View reviewed changes

src/nni_manager/training_service/dlts/dltsTrainingService.ts Outdated Show resolved Hide resolved

SparkSnail reviewed Feb 20, 2020

View reviewed changes

QuanluZhang reviewed Feb 20, 2020

View reviewed changes

src/nni_manager/training_service/dlts/dltsTrainingService.ts Show resolved Hide resolved

Lint

3f1cbc4

Gerhut added 3 commits February 20, 2020 16:37

Lint

1a8f05c

Validate email, password and team

e4d8ab2

Doc

b0f4e41

scarlett2018 reviewed Feb 20, 2020

View reviewed changes

Gerhut added 3 commits February 21, 2020 11:02

Doc fix

cc5b28e

Set TMPDIR

d19ecab

Use metadata instead of gpu_capacity

4a8b290

SparkSnail approved these changes Feb 26, 2020

View reviewed changes

scarlett2018 mentioned this pull request Feb 28, 2020

Iteration Plan for release in Mar #2079

Closed

35 tasks

Gerhut added 4 commits February 28, 2020 21:29

Cancel paused DLTS job

860e0af

workaround lint rules

fecc619

pylint

a340df2

doc

c831f40

QuanluZhang approved these changes Feb 29, 2020

View reviewed changes

liuzhe-lz merged commit 134368f into microsoft:master Mar 2, 2020

		@@ -0,0 +1,42 @@
		import { DLTSClusterConfig } from "./dltsClusterConfig";

		@@ -0,0 +1,52 @@
		Run an Experiment on Deep Learning Training Service


		Step 2. Prepare a NNI config YAML like the following:

		```yaml


		![Submit Job](../../img/dlts-step4.png)

		Step 5. Go to Endpoints tab of the newly created job, click the Port 40000 link to theck trial's information.

DLTS integration #1945

DLTS integration #1945

Conversation

Gerhut commented Jan 10, 2020 • edited Loading

Key Points:

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

chicm-ms commented Feb 20, 2020

SparkSnail Feb 20, 2020 • edited Loading

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

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

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

SparkSnail Feb 20, 2020 • edited Loading

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

Gerhut Feb 20, 2020 • edited Loading

Choose a reason for hiding this comment

Gerhut commented Feb 20, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Gerhut Feb 21, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Gerhut Feb 21, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Gerhut Feb 21, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

chicm-ms commented Feb 26, 2020

Gerhut commented Jan 10, 2020 •

edited

Loading

SparkSnail Feb 20, 2020 •

edited

Loading

SparkSnail Feb 20, 2020 •

edited

Loading

Gerhut Feb 20, 2020 •

edited

Loading

Gerhut Feb 21, 2020 •

edited

Loading

Gerhut Feb 21, 2020 •

edited

Loading

Gerhut Feb 21, 2020 •

edited

Loading