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

Add metric logging #223

Merged
merged 5 commits into from
Aug 20, 2022
Merged

Add metric logging #223

merged 5 commits into from
Aug 20, 2022

Conversation

Dref360
Copy link
Member

@Dref360 Dref360 commented Jun 26, 2022

Summary:

Add some utils to keep track of metrics over time.

This is WIP, but what do you think of this API?
It would be quite simple for the users to get a metrics for each dataset size.

We can also add utilities for that such as MetricMixin.get_active_metric(metric_name="precision") : Dict[int, float]

Features:

Fixes #220

Checklist:

  • Your code is documented (To validate this, add your module to tests/documentation_test.py).
  • Your code is tested with unit tests.
  • You moved your Issue to the PR state.

@parmidaatg
Copy link
Collaborator

I like this. thanks a lot @Dref360. I was only wondering don't you think it is more descriptive to have "active_step" as the key for metrics and have "datasize" as a field ?

@GeorgePearse
Copy link
Collaborator

This looks great, will give it a try as soon as it's ready.

@Dref360
Copy link
Member Author

Dref360 commented Jul 24, 2022

I like this. thanks a lot @Dref360. I was only wondering don't you think it is more descriptive to have "active_step" as the key for metrics and have "datasize" as a field ?

Good idea, not sure how to keep track of the active step tho. Do you have an idea? Like just a field in ModelWrapper?

@Dref360 Dref360 marked this pull request as ready for review July 24, 2022 15:32
@Dref360
Copy link
Member Author

Dref360 commented Jul 24, 2022

I think this is ready for review.
I added the dataset size in the response of get_metrics.

@rafapi
Copy link
Collaborator

rafapi commented Jul 28, 2022

Is there an entry in the docs for this? Regardless of that, this looks good!

@Dref360
Copy link
Member Author

Dref360 commented Jul 31, 2022

@rafapi just added an example in our documentation.

@Dref360
Copy link
Member Author

Dref360 commented Aug 5, 2022

Quick video showcasing the feature.
https://www.loom.com/share/4b7d0eeee187463cac12ecd57828c5fe

Copy link
Collaborator

@rafapi rafapi left a comment

Choose a reason for hiding this comment

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

LGTM!

@Dref360 Dref360 merged commit ec82f30 into master Aug 20, 2022
@Dref360 Dref360 deleted the features/metric_logging branch August 20, 2022 14:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Better metrics logging
4 participants