Skip to content
This repository has been archived by the owner on Sep 18, 2024. It is now read-only.

Refactor NNI manager globals (step 1) - argparse #4510

Merged
merged 5 commits into from
Mar 20, 2022

Conversation

liuzhe-lz
Copy link
Contributor

@liuzhe-lz liuzhe-lz commented Jan 25, 2022

Description

Refactor NNI manager's argparse with de facto standard library yargs.

Changed args and options:

  • start_mode -> action because "mode" has too many meanings in NNI manager.
    • new -> create because the command is called "nnictl create".
    • resume + readonly -> view because the command is called "nnictl view".
  • log_dir -> experiments-directorey because it is not log dir at all.
    • In config file it is called experimentWorkingDirectory but... it was a mistake. I thought it was ~/nni-experiments/<ID> but no, it is just ~/nni-experiments.

Checklist

  • test case
  • doc

How to test

Test together with following up PRs.

@J-shang
Copy link
Contributor

J-shang commented Jan 27, 2022

Description

Refactor NNI manager's argparse with de facto standard library yargs.

Changed args and options:

  • start_mode -> action because "mode" has too many meanings in NNI manager.

    • new -> create because the command is called "nnictl experiment create".
    • resume + readonly -> view because the command is called "nnictl view". (Why it's not "nnictl experiment view"? 🤔)
  • log_dir -> experiments-directorey because it is not log dir at all.

    • In config file it is called experimentWorkingDirectory but... it was a mistake. I thought it was ~/nni-experiments/<ID> but no, it is just ~/nni-experiments.

Checklist

  • test case
  • doc

How to test

Test together with following up PRs.

the command is nnictl create and we don't have nnictl experiment create 😂
maybe experimentsWorkingDirectory?

@liuzhe-lz
Copy link
Contributor Author

the command is nnictl create and we don't have nnictl experiment create 😂 maybe experimentsWorkingDirectory?

Ooops, seems there was a bug in my mind.
experimentsWorkingDirectory sounds better. I will update.

@QuanluZhang QuanluZhang self-requested a review February 15, 2022 00:58
def to_command_line_args(self):
ret = []
for field in fields(self):
value = getattr(self, field.name)
if value is not None:
ret.append('--' + field.name)
ret.append('--' + field.name.replace('_', '-'))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why this update?

@J-shang
Copy link
Contributor

J-shang commented Mar 1, 2022

There is a conflict, pls fix it

@liuzhe-lz liuzhe-lz merged commit 3b27ac7 into microsoft:master Mar 20, 2022
@liuzhe-lz liuzhe-lz deleted the refactor-global-s1 branch March 20, 2022 17:40
@J-shang J-shang mentioned this pull request Mar 23, 2022
51 tasks
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants