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

[Retiarii] Export topk models #3464

Merged
merged 6 commits into from
Apr 3, 2021

Conversation

ultmaster
Copy link
Contributor

No description provided.

@@ -257,16 +259,30 @@ def stop(self) -> None:
self._dispatcher_thread = None
_logger.info('Experiment stopped')

def export_top_models(self, top_n: int = 1):
def export_top_models(self, top_k: int = 1, optimize_mode: str = 'maximize', formatter: str = 'code') -> Any:
Copy link
Contributor

Choose a reason for hiding this comment

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

it is better to obtain optimize_mode from strategy

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This requires re-designing of strategy as some strategy does not need to know what mode it is by design (e.g., random). I'll add a comment that optimize_mode is likely to be removed and defined in strategy in future.

@@ -43,6 +44,12 @@ def submit_models(*models: Model) -> None:
engine.submit_models(*models)


def list_models(*models: Model) -> Iterable[Model]:
Copy link
Contributor

Choose a reason for hiding this comment

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

I think it's good to make return type concrete, unless it's likely to change in future.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think iterable is fine. Some execution engine might favor to return a generator since there are a lot of models.

@QuanluZhang QuanluZhang merged commit aea98dd into microsoft:master Apr 3, 2021
acured pushed a commit to acured/nni that referenced this pull request Apr 7, 2021
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.

3 participants