New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Support view/resume experiment from external folder #3870
Conversation
nni/tools/nnictl/launcher.py
Outdated
if not os.path.isdir(args.experiment_dir): | ||
print_error('Path %s is not folder directory!' % args.experiment_dir) | ||
exit(1) | ||
experiment_id = os.path.basename(args.experiment_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.
This might not work for ~/nni/foo/
(ends with slash). Please check.
And since the experiment ID is not included in logDir / experimentWorkingDirectory, I think the argument is a little stange.
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.
add validation logic.
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.
nnictl view -e /home/lz/nni-experiments/Su83qgOR/
cannot pass validation.
I think it's frustrating since bash will automatically add the tail space with tab completion.
Suggest use Path(experiment_dir).name
to replace os.path.basement
.
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 to Path(args.experiment_dir).name
.
nni/tools/nnictl/nnictl.py
Outdated
@@ -167,6 +167,12 @@ def parse_args(): | |||
parser_load_experiment.add_argument('--searchSpacePath', '-s', required=False, help='the path of search space file for \ | |||
loaded experiment, this path contains file name. Default in $codeDir/search_space.json') | |||
parser_load_experiment.set_defaults(func=load_experiment) | |||
#view an 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.
I'm a little confused. Where is the old view
? As far as I remembered, there has already been a view
.
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.
Seems there are nnictl experiment view
and nnictl view
.
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.
That's really confusing. Why can't we merge these two?
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.
unity command into nnictl view
@@ -166,6 +166,10 @@ nnictl resume | |||
- False | |||
- | |||
- set foreground mode, print log content to terminal | |||
* - --experiment_dir, -e | |||
- False |
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 default value is None?
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.
Yes, nnictl view
will not use --experiment_dir
by default.
nnictl experiment delete