Add native support for v2 config #3315

liuzhe-lz · 2021-01-18T08:45:42Z

Recreated as PR #3466

J-shang · 2021-02-21T14:07:52Z

nni/experiment/config/v1.py

+    _drop_field(v1, 'versionCheck')
+    _move_field(v1, v2, 'logLevel', 'log_level')
+    _drop_field(v1, 'logCollection')
+    _drop_field(v1, 'useAnnotation')  # FIXME


We don’t support multiThread and useAnnotation in this version? And do we still support these by nnictl?

useAnnotation is handled in nnictl with some hacks. I think it's not elegant but don't want to solve it in this release.
multiThread is not supported. It's easy to implement since the value is directly passed to dispatcher, but I don't want to expose this interface in v2 schema.

J-shang · 2021-02-21T15:18:50Z

nni/tools/nnictl/common_utils.py

@@ -44,6 +44,7 @@ def get_json_content(file_path):
 def print_error(*content):
    '''Print error information to screen'''
    print(Fore.RED + ERROR_INFO + ' '.join([str(c) for c in content]) + Fore.RESET)
+    raise


someplace continuous use twice print_error, like L26 and L39 in this file.

This is my debug code, removed.

J-shang · 2021-02-21T15:24:21Z

nni/tools/nnictl/launcher.py

+    from nni.experiment.config.v1 import convert_to_v2
+    v2 = convert_to_v2(experiment_config).json()
+    response = rest_post(experiment_url(port), json.dumps(v2), REST_TIME_OUT, show_error=True)
+    #print(response.text)


del the debug comments?

J-shang · 2021-02-21T15:41:37Z

nni/tools/nnictl/launcher.py

@@ -520,8 +203,7 @@ def launch_experiment(args, experiment_config, mode, experiment_id):
        exit(1)
    if mode != 'view':
        # set platform configuration
-        set_platform_config(experiment_config['trainingServicePlatform'], experiment_config, args.port,\
-                            experiment_id, rest_process)
+        raise Exception('TODO')


will we do this in this release? nnictl create/resume need to set platform configuration.

The branch was outdated. Implementation merged from dev branch.

J-shang · 2021-02-22T04:05:53Z

ts/nni_manager/core/nnimanager.ts

+    if (exp.trainingService === undefined) {
+        return exp.maxExecDuration;
+    } else {
+        return 99999;


if v2, we don't use maxExperimentDuration?

maxExecDuration/maxExperimentDuration has always been optional. Previously the default value is handled by nnictl, which I think is improper.

J-shang · 2021-02-22T05:40:48Z

ts/nni_manager/training_service/remote_machine/remoteMachineTrainingService.ts

@@ -60,6 +60,7 @@ class RemoteMachineTrainingService implements TrainingService {
    private sshConnectionPromises: any[];

    constructor(@component.Inject timer: ObservableTimer) {
+        super()


remote haven't updated yet?

SparkSnail · 2021-02-22T06:44:31Z

nni/experiment/config/v1.py

+            _move_field(v1_machine, v2_machine, 'gpuIndices', 'gpu_indices')
+            _move_field(v1_machine, v2_machine, 'maxTrialNumPerGpu', 'max_trial_number_per_gpu')
+            _move_field(v1_machine, v2_machine, 'useActiveGpu', 'use_active_gpu')
+            _move_field(v1_machine, v2_machine, 'preCommand', 'trial_prepare_command')


we just rename this field to pythonPath in #3367 (comment)

SparkSnail · 2021-02-22T06:51:49Z

nni/experiment/config/v1.py

+        _move_field(v1_trial, ts, 'cpuNum', 'trial_cpu_number')
+        _move_field(v1_trial, ts, 'memoryMB', 'trial_memory_size')  # FIXME: unit
+        _move_field(v1_trial, ts, 'image', 'docker_image')
+        _drop_field(v1_trial, 'virtualCluster')  # FIXME: better error message


why drop virtualCluster?

SparkSnail · 2021-02-22T07:04:40Z

nni/tools/nnictl/algo_management.py

+            save_algo_meta_data(meta)
+        else:
+            print_error(f'Cannot overwrite builtin algorithm')
+            raise


If print_error() function contains raise, then does not need to add raise again. BTW, there is no exception here, what is the content of raise?

SparkSnail · 2021-02-22T07:05:44Z

nni/tools/nnictl/launcher.py

-        return result, message
-    #set trial_config
-    return set_trial_config(experiment_config, port, config_file_name), err_message
+#def set_dlts_config(experiment_config, port, config_file_name):


If don't support DLTS anymore, we could delete these code, not comment them.

SparkSnail · 2021-02-22T07:16:48Z

ts/nni_manager/main.ts

+
+    const foregroundArg: string = parseArg(['--foreground', '-f']);
+    if (!('true' || 'false').includes(foregroundArg.toLowerCase())) {
+        console.log(`FATAL: foreground property should only be true or false`);


why use console.log() ?

liuzhe-lz and others added 3 commits January 18, 2021 11:12

draft

6e3ed6b

support python launch

933bf05

support nnictl launch

7c46b62

liuzhe-lz changed the title ~~Add native v2 config support to local~~ Add native support for v2 config Jan 20, 2021

J-shang mentioned this pull request Jan 25, 2021

NNI 2021 Jan~Feb Iteration Planning #3308

Closed

94 tasks

J-shang closed this Jan 27, 2021

J-shang reopened this Jan 27, 2021

J-shang requested review from J-shang and SparkSnail February 1, 2021 02:48

Merge branch 'master' into dev-config

212baff

liuzhe-lz marked this pull request as ready for review February 3, 2021 02:25

liuzhe-lz force-pushed the dev-config branch from ef442f7 to 212baff Compare February 3, 2021 02:25

liuzhe-lz added 10 commits February 19, 2021 07:32

add converter

a14549f

Merge branch 'master' into dev-config

4a60d0d

temp fix webui

e11298a

fix lint

725e824

re-register algo

3c35878

fix register algo

0116818

fix typo

08eb20a

fix typo

c5cdf7b

fix bug

0b2aa18

fix resume

8d72e0a

J-shang reviewed Feb 22, 2021

View reviewed changes

SparkSnail reviewed Feb 22, 2021

View reviewed changes

liuzhe-lz added 2 commits February 22, 2021 18:14

unify type and rename pai to openpai

3d62649

debug openpai

83bc065

liuzhe-lz marked this pull request as draft February 23, 2021 08:52

kvartet mentioned this pull request Mar 11, 2021

NNI 2021 Mar~Apr Iteration Planning #3445

Closed

78 tasks

liuzhe-lz closed this Mar 22, 2021

liuzhe-lz deleted the dev-config branch June 17, 2021 03:26

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add native support for v2 config #3315

Add native support for v2 config #3315

liuzhe-lz commented Jan 18, 2021 •

edited

Loading

J-shang Feb 21, 2021 •

edited

Loading

liuzhe-lz Feb 23, 2021 •

edited

Loading

J-shang Feb 21, 2021

liuzhe-lz Feb 23, 2021

J-shang Feb 21, 2021

J-shang Feb 21, 2021

liuzhe-lz Feb 23, 2021

J-shang Feb 22, 2021

liuzhe-lz Feb 23, 2021

J-shang Feb 22, 2021

SparkSnail Feb 22, 2021

SparkSnail Feb 22, 2021

SparkSnail Feb 22, 2021 •

edited

Loading

SparkSnail Feb 22, 2021

SparkSnail Feb 22, 2021

Add native support for v2 config #3315

Add native support for v2 config #3315

Conversation

liuzhe-lz commented Jan 18, 2021 • edited Loading

J-shang Feb 21, 2021 • edited Loading

Choose a reason for hiding this comment

liuzhe-lz Feb 23, 2021 • 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

SparkSnail Feb 22, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

liuzhe-lz commented Jan 18, 2021 •

edited

Loading

J-shang Feb 21, 2021 •

edited

Loading

liuzhe-lz Feb 23, 2021 •

edited

Loading

SparkSnail Feb 22, 2021 •

edited

Loading