update nnicli #2713

JunweiSUN · 2020-07-22T06:31:10Z

update nnicli with more flexibility. Add nnicli docs

QuanluZhang · 2020-07-24T13:07:50Z

docs/en_US/nnicli_ref.md

+
+nc.start_experiment('nni/examples/trials/mnist-pytorch/config.yml', port=9090) # start an experiment
+
+nc.set_endpoint('http://localhost:9090') # set the experiment's endpoint, i.e., the url of Web UI


what is this API used for? after an experiment is created, the endpoint is fixed, right? why we need to set it?

i see, need to set this endpoint in order to query restful APIs.

is it possible to automatically detect this endpoint? because it is counter intuitive that user is required to set endpoint

Agree with you. Maybe we can get the endpoint when user use start_experiment. But if user wants to conduct some operation on other experiments, they still need to set the endpoint by their own.

QuanluZhang · 2020-07-24T13:08:27Z

docs/en_US/nnicli_ref.md

@@ -0,0 +1,41 @@
+# NNI Client
+
+NNI client is a python API of `nnictl`, which implements the most common used commands. Users can use this API to control their experiments, collect experiment results and conduct advanced analyses based on experiment results in python code directly instead of using command line. Here is an example:


common used -> commonly used

QuanluZhang · 2020-07-24T13:34:14Z

@JunweiSUN this pr looks good. we can schedule a quick meeting to see how to further improve nnicli

JunweiSUN · 2020-07-24T18:18:40Z

@QuanluZhang Sure. I'll make arrangements for this.

src/sdk/pycli/nnicli/nni_client.py

QuanluZhang · 2020-08-10T08:43:56Z

src/sdk/pycli/nnicli/nni_client.py

+        if _create_process(cmd) != 0:
+            raise RuntimeError('Failed to start experiment, please check your config.')
+        else:
+            if port:
+                self.port = port
+            else:
+                self.port = 8080
+            self.endpoint = 'http://localhost:{}'.format(self.port)
+            self.exp_id = self.get_experiment_profile()['id']


suggest to put this part into a dedicated private member function

add a new private function _exec_command

QuanluZhang · 2020-08-10T08:49:13Z

tools/nni_cmd/updater.py

@@ -14,7 +14,7 @@
 def validate_digit(value, start, end):
    '''validate if a digit is valid'''
    if not str(value).isdigit() or int(value) < start or int(value) > end:
-        raise ValueError('%s must be a digit from %s to %s' % (value, start, end))
+        raise ValueError('%s must be a digit from %s to %s' % ('value', start, end))


it is strange that you add quote on value

I think there used to be a typo here. If the value is 1, it's strange to raise a error like ValueError('1 must be a digit from 1 to 1000' % (value, start, end)). Instead, it should be ValueError('value must be a digit from 1 to 1000' % (value, start, end))

then it should be raise ValueError('value must be a digit from %s to %s' % (start, end)). i still suggest the following:
raise ValueError('value (%s) must be a digit from %s to %s' % (value, start, end))

Goog idea. Will fix.

QuanluZhang · 2020-08-10T08:49:40Z

test/config/integration_tests.yml

@@ -135,17 +135,6 @@ testCases:
  validator:
    class: ExportValidator 

- name: nnicli


why remove all the tests?

I used to think that it is difficult to split the test to launchCommand and stopCommand since we use an instance to handle an experiment. Maybe I could connect to an existing experiment in stopCommand. Will add the tests again.

QuanluZhang · 2020-08-10T08:50:16Z

@JunweiSUN this pr looks good now, please resolve the comments.

QuanluZhang · 2020-08-11T03:44:18Z

src/sdk/pycli/nnicli/nni_client.py

+        hyper parameters for this trial
+    value:
+        final result
+    id:


what is the relation between this id and trialJobId in NNITrialMetricData?

I follow the keys defined in the api, here they should be same value.

QuanluZhang · 2020-08-11T03:45:32Z

src/sdk/pycli/nnicli/nni_client.py

+
+    Attributes
+    ----------
+    parameter:


please add type for attributes and parameters

what is the relation between parameter and NNITrialHyperParameters?

parameter in TrialResult is the same as parameters in TrialHyperParameters

ultmaster · 2020-08-11T03:40:30Z

docs/en_US/nnicli_ref.md

+NNI client is a python API of `nnictl`, which implements the most commonly used commands. Users can use this API to control their experiments, collect experiment results and conduct advanced analyses based on experiment results in python code directly instead of using command line. Here is an example:
+
+```
+from nnicli import NNIExperiment


I prefer Experiment instead of NNIExperiment

Sounds good. Will change

ultmaster · 2020-08-11T03:41:25Z

src/sdk/pycli/nnicli/nni_client.py

+
+    Attributes
+    ----------
+    parameter:


Please add types for attributes and parameters

ultmaster · 2020-08-11T03:41:37Z

src/sdk/pycli/nnicli/nni_client.py

+        self.sequence = None
+        self.data = None
+        for key in json_obj.keys():
+            self.__setattr__(key, json_obj[key])


setattr(self, key,

ultmaster · 2020-08-11T03:44:32Z

src/sdk/pycli/nnicli/nni_client.py

+                    self.startTime, self.endTime, self.finalMetricData, self.stderrPath)
+
+class NNIExperiment:
+    def __init__(self):


Suggest ban this __init__ method and implement several class methods including start_experiment, from_existing_experiment.

not suggest to use class methods, member function is enough, which is simpler

ultmaster · 2020-08-11T03:46:47Z

src/sdk/pycli/nnicli/nni_client.py

+            cmd += '--port {}'.format(port).split(' ')
+        if debug:
+            cmd += ['--debug']
+        self._exec_command(cmd, port)


Better if this is a classmethod that creates and returns a new experiment object.

it is okay to write in this way :)

ultmaster · 2020-08-11T03:47:12Z

src/sdk/pycli/nnicli/nni_client.py

+
+class NNIExperiment:
+    def __init__(self):
+        self.endpoint = None


endpoint, port, exp_id should be properties that does not allow external modification.

ultmaster · 2020-08-11T03:47:29Z

src/sdk/pycli/nnicli/nni_client.py

+        parameter index
+    """
+    def __init__(self, json_obj):
+        self.id = None


I think these all should be properties.

I think we should allow users to modify these "data".

ultmaster · 2020-08-11T03:47:53Z

src/sdk/pycli/nnicli/nni_client.py

+        self.finalMetricData = None
+        self.stderrPath = None
+        for key in json_obj.keys():
+            self.__setattr__(key, json_obj[key])


Check hasattr(self, key) before setattr

ultmaster · 2020-08-11T03:48:52Z

test/config/integration_tests_tf2.yml

@@ -110,8 +110,8 @@ testCases:
  config:
    maxTrialNum: 4
    trialConcurrency: 4
-  launchCommand: python3 -c 'import nnicli as nc; nc.start_nni("$configFile")'
-  stopCommand: python3 -c 'import nnicli as nc; nc.stop_nni()'
+  launchCommand: python3 -c 'from nnicli import NNIExperiment; exp = NNIExperiment(); exp.start_experiment("$configFile")'


nnicli.Experiment.start_experiment(...) preferred.

ultmaster · 2020-08-11T03:49:14Z

test/config/integration_tests_tf2.yml

-  launchCommand: python3 -c 'import nnicli as nc; nc.start_nni("$configFile")'
-  stopCommand: python3 -c 'import nnicli as nc; nc.stop_nni()'
+  launchCommand: python3 -c 'from nnicli import NNIExperiment; exp = NNIExperiment(); exp.start_experiment("$configFile")'
+  stopCommand: python3 -c 'from nnicli import NNIExperiment; exp = NNIExperiment(); exp.connect_experiment("http://localhost:8080/"); exp.stop_experiment()'


nnicli.Experiment.from_existing_experiment(...).stop() preferred.

chicm-ms · 2020-08-11T05:05:50Z

examples/notebooks/retrieve_nni_info_with_python.ipynb needs to be updated.

chicm-ms · 2020-08-11T05:06:39Z

test/nni_test/nnitest/validators.py

+        exp.connect_experiment(rest_endpoint)
+        print(exp.get_job_statistics())
+        print(exp.get_experiment_status())
+        print(exp.list_trial_jobs())


missing end of line.

(cherry picked from commit f82ef62)

JunweiSUN added 10 commits July 21, 2020 23:06

enhance nnicli, add ref to readthedoc

a2b1920

enhance nnicli, add ref to readthedoc

9c8a67f

enhance nnicli; add ref of nnicli

4892077

update updater.py

dff0871

enhance nnicli

baa5525

enhance nnicli

4cff1c5

remove binary files

6844e98

update docs

10b842a

update tests

8266390

fix pylint error

99159ea

chicm-ms closed this Jul 23, 2020

chicm-ms reopened this Jul 23, 2020

ultmaster requested a review from chicm-ms July 24, 2020 09:08

scarlett2018 mentioned this pull request Jul 24, 2020

July iteration plan #2608

Closed

66 tasks

QuanluZhang reviewed Jul 24, 2020

View reviewed changes

JunweiSUN added 6 commits July 25, 2020 02:25

fix typo

bae6522

Merge branch 'master' into dev-nnicli-enhancement

9db2253

update nnicli

7fd8ef7

update nnicli

f4c3b97

update nnicli

01bdd00

fix pylint

b91948d

QuanluZhang reviewed Aug 10, 2020

View reviewed changes

src/sdk/pycli/nnicli/nni_client.py Show resolved Hide resolved

QuanluZhang reviewed Aug 10, 2020

View reviewed changes

update nnicli

50f2e58

JunweiSUN added 4 commits August 11, 2020 09:48

update tests

d87387b

update nnicli

46d4742

update docs

53ecd86

update nnicli

7997dfb

QuanluZhang requested a review from ultmaster August 11, 2020 03:40

QuanluZhang reviewed Aug 11, 2020

View reviewed changes

ultmaster reviewed Aug 11, 2020

View reviewed changes

chicm-ms reviewed Aug 11, 2020

View reviewed changes

JunweiSUN added 3 commits August 11, 2020 13:34

remove prefix

8b67511

update nnicli

c0c3ada

update nnicli

88cdd5c

QuanluZhang approved these changes Aug 12, 2020

View reviewed changes

ultmaster approved these changes Aug 12, 2020

View reviewed changes

ultmaster merged commit f82ef62 into microsoft:master Aug 12, 2020

LovPe pushed a commit to LovPe/nni that referenced this pull request Aug 17, 2020

update nnicli (microsoft#2713)

f7c6ef3

(cherry picked from commit f82ef62)


		nc.start_experiment('nni/examples/trials/mnist-pytorch/config.yml', port=9090) # start an experiment

		nc.set_endpoint('http://localhost:9090') # set the experiment's endpoint, i.e., the url of Web UI

		@@ -0,0 +1,41 @@
		# NNI Client

		NNI client is a python API of `nnictl`, which implements the most common used commands. Users can use this API to control their experiments, collect experiment results and conduct advanced analyses based on experiment results in python code directly instead of using command line. Here is an example:

update nnicli #2713

update nnicli #2713

Conversation

JunweiSUN commented Jul 22, 2020

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

QuanluZhang commented Jul 24, 2020

JunweiSUN commented Jul 24, 2020

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

QuanluZhang Aug 10, 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

QuanluZhang commented Aug 10, 2020

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

JunweiSUN Aug 11, 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

chicm-ms commented Aug 11, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

QuanluZhang Aug 10, 2020 •

edited

Loading

JunweiSUN Aug 11, 2020 •

edited

Loading