Skip to content
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

[Metric] Add metric framework #2742

Merged
merged 6 commits into from
Feb 24, 2022

Conversation

zhongchun
Copy link
Contributor

@zhongchun zhongchun commented Feb 22, 2022

What do these changes do?

There is no metrics in mars and it's useful and necessary to add a simple and convenient metric framework.

Related issue number

Issuse #2743

Check code requirements

  • tests added / passed (if needed)
  • Ensure all linting tests pass, see here for how to run them

@zhongchun zhongchun force-pushed the feat-add-metric-framework branch from 3aad232 to 6aed99d Compare February 22, 2022 12:57
@qinxuye
Copy link
Collaborator

qinxuye commented Feb 23, 2022

@zhongchun
Copy link
Contributor Author

@zhongchun zhongchun force-pushed the feat-add-metric-framework branch 2 times, most recently from 18ac68c to ae62107 Compare February 23, 2022 11:52
* Use relative import instead of absolute import of `test_metric_api.py`
* Add metric test config in pipelines
* Add more tests
Copy link
Collaborator

@qinxuye qinxuye left a comment

Choose a reason for hiding this comment

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

I left some comments

mars/metric/api.py Show resolved Hide resolved
mars/metric/backends/metric.py Outdated Show resolved Hide resolved
mars/metric/backends/metric.py Outdated Show resolved Hide resolved
mars/metric/backends/metric.py Show resolved Hide resolved
@qinxuye qinxuye added this to the v0.9.0b2 milestone Feb 24, 2022
qinxuye
qinxuye previously approved these changes Feb 24, 2022
Copy link
Collaborator

@qinxuye qinxuye left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Member

@wjsi wjsi left a comment

Choose a reason for hiding this comment

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

Generally LGTM, I add some comments.

mars/metric/backends/tests/test_metric.py Outdated Show resolved Hide resolved
mars/metric/backends/tests/test_metric.py Outdated Show resolved Hide resolved
mars/metric/backends/tests/test_metric.py Outdated Show resolved Hide resolved
mars/metric/backends/tests/test_metric.py Outdated Show resolved Hide resolved
mars/metric/backends/tests/test_metric.py Outdated Show resolved Hide resolved
mars/metric/backends/console/console_metric.py Outdated Show resolved Hide resolved
Copy link
Member

@wjsi wjsi left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Collaborator

@qinxuye qinxuye left a comment

Choose a reason for hiding this comment

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

LGTM

@qinxuye qinxuye merged commit dfcb1ce into mars-project:master Feb 24, 2022
@qinxuye qinxuye mentioned this pull request Feb 25, 2022
4 tasks
@zhongchun zhongchun changed the title Add metric framework [Metric]Add metric framework Feb 28, 2022
qinxuye pushed a commit to hekaisheng/mars that referenced this pull request Mar 1, 2022
@zhongchun zhongchun changed the title [Metric]Add metric framework [Metric] Add metric framework Mar 4, 2022
chaokunyang pushed a commit to chaokunyang/mars that referenced this pull request May 31, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants