-
Notifications
You must be signed in to change notification settings - Fork 1.8k
Conversation
Merge master
merge master
merge master
merge master
merge master
merge master
merge master
Chinese translation (microsoft#2458)
merge master
merge master
merge master
merge master
docs/en_US/Tutorial/Nnictl.md
Outdated
|Name, shorthand|Required|Default|Description| | ||
|------|------|------ |------| | ||
|--path, -p| True| |the full file path of nni package| | ||
|--codeDir, -c| True| |the path of codeDir| |
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.
could add more description for codeDir
. for example, "the path that you want to put the code of the saved experiment into". BTW, what if the saved experiment does not include code?
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.
updated, if the experiment package does not code files, NNI will do nothing, just use new codeDir to replace old one.
tools/nni_cmd/nnictl.py
Outdated
parser_save_data.set_defaults(func=save_experiment) | ||
#open experiment data | ||
parser_open_data = parser_experiment_subparsers.add_parser('load', help='load experiment data') | ||
parser_open_data.add_argument('--path', '-p', required=True, help='the path of nni package 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.
better to make it consistent with the 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.
updated
tools/nni_cmd/nnictl_utils.py
Outdated
logDir = os.path.join(os.path.expanduser("~"), 'nni-experiments', args.id) | ||
if nni_config.get_config('logDir'): | ||
logDir = os.path.join(nni_config.get_config('logDir'), 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.
better to use a function to generate logDir
, this function can be put in a file which store constants or meta-datas. 'nni-experiments' can be seen as a type of metadata
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.
updated
|
||
# Step2. Copy nnictl metadata to temp folder | ||
temp_nnictl_dir = os.path.join(temp_root_dir, 'nnictl') | ||
os.makedirs(temp_nnictl_dir, exist_ok=True) |
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.
is it possible that the dir already existed?
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 dir name is generate by random()
function, will not exist logically, I also add logic to handle this scenario, if the dir exist, nni will regenerate a new dir until it is not existed originally.
tools/nni_cmd/nnictl_utils.py
Outdated
# Step4. Copy code dir | ||
codeDir = os.path.expanduser(args.codeDir) | ||
if not os.path.isabs(codeDir): | ||
codeDir = os.path.join(os. getcwd(), codeDir) |
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.
os. getcwd()?
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.
additional whitespace after dot
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.
If the path is not a full path, nnictl will use current folder as a root path to find files.updated to os.getcwd()
tools/nni_cmd/common_utils.py
Outdated
temp_dir = generate_folder_name() | ||
while os.path.exists(temp_dir): | ||
temp_dir = generate_folder_name() | ||
os.makedirs(temp_dir, exist_ok=True) |
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.
as temp_dir
does not exist, so it is ok to remove exist_ok=True
?
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.
sure, updated
tools/nni_cmd/nnictl_utils.py
Outdated
if nnictl_exp_config.get('logDir'): | ||
logDir = nnictl_exp_config['logDir'] | ||
else: | ||
logDir = os.path.join(os.path.expanduser("~"), 'nni-experiments') |
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.
also use NNICTL_HOME_DIR
here
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.
updated
tools/nni_cmd/nnictl_utils.py
Outdated
exit(1) | ||
temp_root_dir = generate_temp_dir() | ||
shutil.unpack_archive(package_path, temp_root_dir) | ||
print_normal('Opening...') |
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.
Loading...
is better?
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.
updated
tools/nni_cmd/nnictl_utils.py
Outdated
experiment_metadata.get('experimentName'), | ||
experiment_metadata.get('endTime'), | ||
experiment_metadata.get('status')) | ||
print_normal('Open experiment %s succsss!' % experiment_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.
Open -> Load ?
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
codeDir = os.path.join(os.getcwd(), codeDir) | ||
print_normal('Expand codeDir to %s' % codeDir) | ||
nnictl_exp_config['trial']['codeDir'] = codeDir | ||
archive_code_dir = os.path.join(temp_root_dir, 'code') |
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.
why not copy entire 'code' dir using shutil.copytree?
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.
shutil.copytree
only support copy entire folder to another folder, does not support merging two folder contents. We need to copy the contents under code
folder, and do not change target folder name.
(cherry picked from commit d5072a2)
No description provided.