-
Notifications
You must be signed in to change notification settings - Fork 1.8k
Conversation
fix remote bug (microsoft#523)
merge master
merge master
merge master
merge master
merge master
merge master
merge master
merge master
merge master
merge master
merge master
merge master
merge master
merge V0.8
merge master
Add sklearn installation in setup.py (microsoft#1157)
merge master
merge master
tools/nni_cmd/nnictl_utils.py
Outdated
break | ||
#clean local data | ||
home = str(Path.home()) | ||
local_dir = os.path.join(home, 'nni', 'experiments', str(args.id)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you assume that the data is under ~/nni/experiments. I remembered that users can config the path where the data/log is stored.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed, use logDir first.
parser_platform_subparsers = parser_platform.add_subparsers() | ||
parser_platform_clean = parser_platform_subparsers.add_parser('clean', help='clean up the experiment data') | ||
parser_platform_clean.add_argument('--config', '-c', required=True, dest='config', help='the path of yaml config file') | ||
parser_platform_clean.set_defaults(func=platform_clean) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
doc
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed, added doc in Nnictl.md
tools/nni_cmd/nnictl_utils.py
Outdated
if match_result: | ||
output_host = match_result.group('host') | ||
output_directory = match_result.group('baseDir') | ||
if output_host == host: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
do you need to give warning when output_host and host do not match?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed, give warning in code.
tools/nni_cmd/nnictl_utils.py
Outdated
print_error('Please set correct config path!') | ||
exit(1) | ||
experiment_config = get_yml_content(config_path) | ||
platform = experiment_config.get('trainingServicePlatform') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
'trainingServicePlatform'? What is the spec for the config file?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The config file is same as the yaml file to create an experiment, like
authorName: default
experimentName: example_mnist
trialConcurrency: 1
maxExecDuration: 1h
maxTrialNum: 10
#choice: local, remote, pai
trainingServicePlatform: local
searchSpacePath: search_space.json
#choice: true, false
useAnnotation: false
tuner:
#choice: TPE, Random, Anneal, Evolution, BatchTuner, MetisTuner
#SMAC (SMAC should be installed through nnictl)
builtinTunerName: TPE
classArgs:
#choice: maximize, minimize
optimize_mode: maximize
trial:
command: python3 mnist.py
codeDir: .
gpuNum: 0
tools/nni_cmd/nnictl_utils.py
Outdated
remote_dir = '/' + '/'.join(['tmp', 'nni', 'experiments', nni_config.get_config('experimentId')]) | ||
dir_list.append(host + ':' + str(port) + remote_dir) | ||
elif platform == 'pai': | ||
user_name = nni_config.get_config('experimentConfig').get('paiConfig').get('userName') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
do you need to check whether this pai config is the same as the command arg config?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
no, this function will get all of directory in the related platform, the args config only have one directory.
docs/en_US/Nnictl.md
Outdated
|
||
* Description | ||
|
||
Delete the data generated by nni experiment. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Change it like below,
Delete one or all experiments, it includes log, result, environment information and cache. It uses to delete useless experiment result, or save disk space.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed.
docs/en_US/Nnictl.md
Outdated
|
||
* Description | ||
|
||
clean up platform data. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Change it like below,
It uses to clean up disk on a target platform. The provided YAML file includes the information of target platform, and it follows the same schema as the NNI configuration file.
Note, if the target platform is being used by other users, it may cause unexpected errors to others.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed.
tools/nni_cmd/ssh_utils.py
Outdated
@@ -57,3 +57,17 @@ def create_ssh_sftp_client(host_ip, port, username, password): | |||
return sftp | |||
except Exception as exception: | |||
print_error('Create ssh client error %s\n' % exception) | |||
|
|||
def remote_remove_directory(sftp, directory): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
remove_remote_directory is better. verb+noun is right pattern for function names.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed.
tools/nni_cmd/nnictl_utils.py
Outdated
#clean local data | ||
home = str(Path.home()) | ||
local_dir = nni_config.get_config('experimentConfig').get('logDir') | ||
if not local_dir: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
make sure local clean happens on end, so if there is any error, it has a chance to delete again.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed.
tools/nni_cmd/nnictl_utils.py
Outdated
|
||
def experiment_clean(args): | ||
'''clean up the experiment data''' | ||
nni_config = Config(get_config_filename(args)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Make sure metadata in .local/nnictl/.experiment and corresponding folders are also removed.
issue: #1123
Support local, remote and PAI platform, kubeflow and frameworkcontroller not supported yet.