[Do NOT Squash] Merge retiarii dev branch to master #3155
Conversation
Co-authored-by: liuzhe <zhe.liu@microsoft.com>
* First commit for WS in Retiarii * Refactor ENAS trainer * Fix DARTS trainer * Fix ENAS trainer * Fix issues in DARTS and Proxyless * Fix ProxylessNAS and Random trainer * Refactor mask
|
Must review: |
Merge master commit history into dev-retiarii
nni/retiarii/integration.py
Outdated
| from .graph import MetricData | ||
|
|
||
|
|
||
| _logger = logging.getLogger('nni.msg_dispatcher_base') |
There was a problem hiding this comment.
It should be removed. I added it for debug.
There was a problem hiding this comment.
To print message to dispatcher log? Any suggestions?
| super().__init__(*args) | ||
|
|
||
| @staticmethod | ||
| def _debug_dump_graph(graph): |
There was a problem hiding this comment.
Why is this a static method of error class?
nni/retiarii/graph.py
Outdated
| self.metric: Optional[MetricData] = None | ||
| self.intermediate_metrics: List[MetricData] = [] | ||
|
|
||
| self._last_uid: int = 0 # FIXME: this should be global, not model-wise |
There was a problem hiding this comment.
Need to fix. (my bad)
| self.sampler = recorder | ||
| new_model = self.apply(model) | ||
| self.sampler = sampler_backup | ||
| return recorder.recorded_candidates, new_model |
There was a problem hiding this comment.
As an user API, I think the return type is too complicated.
I suggest to make this method "static" and accept multiple mutators:
class Mutator:
# @staticmethod (optionally)
def dry_run(mutator: Union[Mutator, Iterable[Mutator]], model: Model):
...
Mutator.dry_run([mutator1, mutator2, mutator3], model)
Mutator.dry_run(single_mutator, model)
single_mutator.dry_run(model) # if "@staticmethod" decorator is not used| """ | ||
|
|
||
|
|
||
| class TrainingConfig: |
There was a problem hiding this comment.
Is this class still in use?
I copied it from artifact and don't know how it works.
If yes, it need to be documented. Current doc delivers no information.
nni/retiarii/graph.py
Outdated
| """ | ||
| Graph metrics like loss, accuracy, etc. | ||
|
|
||
| # Maybe we can assume this is a single float number for first iteration. |
There was a problem hiding this comment.
What's the real format? Float or dict? Or both?
| assert isinstance(self.operation, Cell) | ||
| return self.graph.model.graphs[self.operation.parameters['cell']] | ||
|
|
||
| def update_label(self, label: str) -> None: |
There was a problem hiding this comment.
What is label and why it's needed?
What is label in framework's perspective?
What is label in end-user's perspective?
| def nodes(self) -> List['Node']: | ||
| return [self.input_node, self.output_node] + self.hidden_nodes | ||
|
|
||
| def _add_input(self, input_name) -> None: |
There was a problem hiding this comment.
This is for PyTorch converter only. Suggest move into converter package.
| assert head[0].graph is self and tail[0].graph is self | ||
| return Edge(head, tail, _internal=True)._register() | ||
|
|
||
| def del_edge(self, edge: 'Edge') -> None: |
There was a problem hiding this comment.
Please sync the name. Either rename this to remove_edge, or rename Edge.remove to del.
nni/retiarii/graph.py
Outdated
| return new_node | ||
|
|
||
| # mutation | ||
| def add_edge(self, head: Tuple['Node', Optional[int]], tail: Tuple['Node', Optional[int]]) -> 'Edge': |
There was a problem hiding this comment.
The type hint is too confusing. I'll deal with it.
|
|
||
| def _request_trial_jobs_callback(self, num_trials: int) -> None: | ||
| for listener in self._listeners: | ||
| listener.on_resource_available(1 * num_trials) |
test/ut/retiarii/advisor_entry.py
Outdated
| import os | ||
| import sys | ||
|
|
||
| from nni.retiarii.integration import RetiariiAdvisor |
There was a problem hiding this comment.
is this file still necessary?
There was a problem hiding this comment.
We don't need it any more I think.
| @@ -0,0 +1,3 @@ | |||
| # these should be experiment config in release | |||
|
|
|||
| framework = 'pytorch' | |||
There was a problem hiding this comment.
So are we at release now?
No description provided.