Merge pull request #104 from Microsoft/master

merge master
SparkSnail · Dec 24, 2018 · 2269688 · 2269688
2 parents c175e7d + 416b8b5
commit 2269688
Show file tree

Hide file tree

Showing 9 changed files with 38 additions and 36 deletions.
diff --git a/src/nni_manager/common/errors.ts b/src/nni_manager/common/errors.ts
@@ -26,12 +26,14 @@ export namespace NNIErrorNames {
 }
 
 export class NNIError extends Error {
+    public cause!: Error | undefined;
     constructor (name: string, message: string, err?: Error) {
         super(message);
         this.name = name;
         if (err !== undefined) {
             this.stack = err.stack;
         }
+        this.cause = err;
     }
 }
 

diff --git a/src/nni_manager/core/ipcInterface.ts b/src/nni_manager/core/ipcInterface.ts
@@ -103,7 +103,7 @@ class IpcInterface {
         try {
             const data: Buffer = encodeCommand(commandType, content);
             if (!this.outgoingStream.write(data)) {
-                this.logger.error('Commands jammed in buffer!');
+                this.logger.warning('Commands jammed in buffer!');
             }
         } catch (err) {
             throw new NNIError('Dispatcher Error', `Dispatcher Error: ${err.message}`, err);

diff --git a/src/nni_manager/core/nniDataStore.ts b/src/nni_manager/core/nniDataStore.ts
@@ -250,7 +250,14 @@ class NNIDataStore implements DataStore {
 
     private mergeHyperParameters(hyperParamList: string[], newParamStr: string): string[] {
         const mergedHyperParams: any[] = [];
-        const newParam: any = JSON.parse(newParamStr);
+        let newParam: any;
+        try {
+            newParam = JSON.parse(newParamStr);
+        } catch (err) {
+            this.log.error(`Hyper parameter needs to be in json format: ${newParamStr}`);
+
+            return hyperParamList;
+        }
         for (const hyperParamStr of hyperParamList) {
             const hyperParam: any = JSON.parse(hyperParamStr);
             mergedHyperParams.push(hyperParam);

diff --git a/src/nni_manager/core/test/ipcInterface.test.ts b/src/nni_manager/core/test/ipcInterface.test.ts
@@ -25,6 +25,7 @@ import { Deferred } from 'ts-deferred';
 import { cleanupUnitTest, prepareUnitTest } from '../../common/utils';
 import * as CommandType from '../commands';
 import { createDispatcherInterface, IpcInterface } from '../ipcInterface';
+import { NNIError } from '../../common/errors';
 
 let sentCommands: {[key: string]: string}[] = [];
 const receivedCommands: {[key: string]: string}[] = [];
@@ -105,8 +106,13 @@ describe('core/protocol', (): void => {
     });
 
     it('sendCommand() should throw on too long command', (): void => {
-        assert.equal((<Error>commandTooLong).name, 'RangeError');
-        assert.equal((<Error>commandTooLong).message, 'Command too long');
+        if (commandTooLong === undefined) {
+            assert.fail('Should throw error')
+        } else {
+            const err: Error | undefined =  (<NNIError>commandTooLong).cause;
+            assert(err && err.name === 'RangeError');
+            assert(err && err.message === 'Command too long');
+        }
     });
 
     it('sendCommand() should throw on wrong command type', (): void => {

diff --git a/src/nni_manager/core/test/nnimanager.test.ts b/src/nni_manager/core/test/nnimanager.test.ts
@@ -57,15 +57,15 @@ describe('Unit test for nnimanager', function () {
         trainingServicePlatform: 'local',
         searchSpace: '{"x":1}',
         tuner: {
-            className: 'EvolutionTuner',
+            className: 'TPE',
             classArgs: {
                 optimize_mode: 'maximize'
             },
             checkpointDir: '',
-            gpuNum: 1
+            gpuNum: 0
         },
         assessor: {
-            className: 'MedianstopAssessor',
+            className: 'Medianstop',
             checkpointDir: '',
             gpuNum: 1
         }
@@ -74,25 +74,19 @@ describe('Unit test for nnimanager', function () {
     before(async () => {
         await initContainer();
         nniManager = component.get(Manager);
+        const expId: string = await nniManager.startExperiment(experimentParams);
+        assert(expId);
     });
 
-    after(() => {
+    after(async () => {
+        await nniManager.stopExperiment();
         cleanupUnitTest();
     })
 
     it('test resumeExperiment', () => {
         //TODO: add resume experiment unit test
     })
 
-    it('test startExperiment', () => {
-
-        return nniManager.startExperiment(experimentParams).then(function (experimentId) {
-            expect(experimentId.length).to.be.equal(8);
-        }).catch(function (error) {
-            assert.fail(error);
-        })
-    })
-
     it('test listTrialJobs', () => {
         //FIXME: not implemented
         //return nniManager.listTrialJobs().then(function (trialJobDetails) {
@@ -147,7 +141,6 @@ describe('Unit test for nnimanager', function () {
         })
     })
 
-
     it('test addCustomizedTrialJob', () => {
         return nniManager.addCustomizedTrialJob('hyperParams').then(() => {
 

diff --git a/src/nni_manager/core/test/sqlDatabase.test.ts b/src/nni_manager/core/test/sqlDatabase.test.ts
@@ -64,10 +64,10 @@ const expParams2: ExperimentParams = {
 };
 
 const profiles: ExperimentProfile[] = [
-    { params: expParams1, id: '#1', execDuration: 0, startTime: Date.now(), endTime: undefined, revision: 1, maxSequenceId: 0 },
-    { params: expParams1, id: '#1', execDuration: 0, startTime: Date.now(), endTime: Date.now(), revision: 2, maxSequenceId: 0 },
-    { params: expParams2, id: '#2', execDuration: 0, startTime: Date.now(), endTime: Date.now(), revision: 2, maxSequenceId: 0 },
-    { params: expParams2, id: '#2', execDuration: 0, startTime: Date.now(), endTime: Date.now(), revision: 3, maxSequenceId: 0 }
+    { params: expParams1, id: '#1', execDuration: 0, logDir: '/log', startTime: Date.now(), endTime: undefined, maxSequenceId: 0, revision: 1,},
+    { params: expParams1, id: '#1', execDuration: 0, logDir: '/log', startTime: Date.now(), endTime: Date.now(), maxSequenceId: 0, revision: 2 },
+    { params: expParams2, id: '#2', execDuration: 0, logDir: '/log', startTime: Date.now(), endTime: Date.now(), maxSequenceId: 0, revision: 2 },
+    { params: expParams2, id: '#2', execDuration: 0, logDir: '/log', startTime: Date.now(), endTime: Date.now(), maxSequenceId: 0, revision: 3 }
 ];
 
 const events: TrialJobEventRecord[] = [

diff --git a/src/nni_manager/rest_server/restValidationSchemas.ts b/src/nni_manager/rest_server/restValidationSchemas.ts
@@ -114,9 +114,9 @@ export namespace ValidationSchemas {
                 codeDir: joi.string(),
                 classFileName: joi.string(),
                 className: joi.string(),
-                classArgs:joi.any(),
+                classArgs: joi.any(),
                 gpuNum: joi.number().min(0),
-                checkpointDir: joi.string()
+                checkpointDir: joi.string().allow('')
             }),
             tuner: joi.object({
                 builtinTunerName: joi.string().valid('TPE', 'Random', 'Anneal', 'Evolution', 'SMAC', 'BatchTuner', 'GridSearch', 'NetworkMorphism'),
@@ -125,7 +125,7 @@ export namespace ValidationSchemas {
                 className: joi.string(),
                 classArgs: joi.any(),
                 gpuNum: joi.number().min(0),
-                checkpointDir: joi.string()
+                checkpointDir: joi.string().allow('')
             }),
             assessor: joi.object({
                 builtinAssessorName: joi.string().valid('Medianstop', 'Curvefitting'),
@@ -134,7 +134,7 @@ export namespace ValidationSchemas {
                 className: joi.string(),
                 classArgs: joi.any(),
                 gpuNum: joi.number().min(0),
-                checkpointDir: joi.string()
+                checkpointDir: joi.string().allow('')
             }),
             clusterMetaData: joi.array().items(joi.object({
                 key: joi.string(),
@@ -149,7 +149,7 @@ export namespace ValidationSchemas {
         body: {
             id: joi.string().required(),
             revision: joi.number().min(0).required(),
-            params: joi.object(STARTEXPERIMENT.body).required(),
+            params: joi.object(STARTEXPERIMENT.body),
             execDuration: joi.number().required(),
             startTime: joi.number(),
             endTime: joi.number(),

diff --git a/src/nni_manager/rest_server/test/mockedNNIManager.ts b/src/nni_manager/rest_server/test/mockedNNIManager.ts
@@ -101,13 +101,7 @@ export class MockedNNIManager extends Manager {
     }
 
     public setClusterMetadata(key: string, value: string): Promise<void> {
-        const deferred: Deferred<void> = new Deferred<void>();
-        if (key === 'exception_test_key') {
-            deferred.reject(new Error('Test Error'));
-        }
-        deferred.resolve();
-
-        return deferred.promise;
+        return Promise.resolve();
     }
 
     public getTrialJob(trialJobId: string): Promise<TrialJobInfo> {

diff --git a/src/nni_manager/rest_server/test/restserver.test.ts b/src/nni_manager/rest_server/test/restserver.test.ts
@@ -133,7 +133,7 @@ describe('Unit test for rest server', () => {
         });
     });
 
-    it('Test PUT experiment/cluster-metadata exception', (done: Mocha.Done) => {
+    it('Test PUT experiment/cluster-metadata bad key', (done: Mocha.Done) => {
         const req: request.Options = {
             uri: `${ROOT_URL}/experiment/cluster-metadata`,
             method: 'PUT',
@@ -146,7 +146,7 @@ describe('Unit test for rest server', () => {
             if (err) {
                 assert.fail(err.message);
             } else {
-                expect(res.statusCode).to.equal(500);
+                expect(res.statusCode).to.equal(400);
             }
             done();
         });