Group metrics by labels #245
Conversation
There was a problem hiding this comment.
Overall looks pretty clean to me - all comments are nits. My biggest concern with this change is that I don't want to overcomplicate the case where a user doesn't care about class specific results and just wants to aggregate results. As such, I wonder if it would be beneficial to change the interface of the aggregate_results method to return a scalar and to have all metric classes implement it?
| value = f1_score(gt, predicted, average=self.f1_method) | ||
| return ScalarResult(value) | ||
| results = {} | ||
| results["macro"] = f1_score(gt, predicted, average="macro") |
There was a problem hiding this comment.
:nit: avoid hardcoded strings and instead make them constants
| @@ -80,7 +82,7 @@ def eval( | |||
|
|
|||
| def __init__( | |||
| self, | |||
| enforce_label_match: bool = False, | |||
| enforce_label_match: bool = True, | |||
There was a problem hiding this comment.
:nit: please adjust comment below
| ScalarResult(109.0 / 300, 3), | ||
| {"enforce_label_match": False}, | ||
| ), | ||
| # ( |
There was a problem hiding this comment.
remove commented code before merging
Yeah, that is a valid concern. It definitely warrants further concern to take a look at the interface of how and where we choose to group_by and aggregate. I'll try to figure out a suggestion today. |
| # TODO(gunnar): Enforce label match -> Why is that a parameter? Should we generally allow IOU matches | ||
| # between different labels?!? |
There was a problem hiding this comment.
In general we should have an option to allow this. E.g. you need to compute matches across the classes for the confusion matrix.
Context:
We want to support class-specific results from our metrics. As a part of that we're essentially opening up the interface from the metrics to return multiple values with a label per value. For our standard functions we will return a value per label for the polygon metrics.
Before:
We'd get a single number per
DatasetItemand then a singleaggregate_scoreper evaluation.After:
We get a {"key_1": number, ... , "key_N": number} per
DatasetItemand then another {"another_key_1": number, ..., "another_key_N": number } from aggregate_score.As a part of this we also add
extra_infowhich is astring -> stringdictionary which you can add to each dataset item in the metric anderrorfield which we use if evaluation of a singleDatasetItemfails. That way we don't fail the whole evaluation.This PR
This changes the metrics to allow returning multiple values per metrics and changes the default metrics to return results grouped by label.
This also adds an extra method on results that allow us to pass more data than floats to the frontend via
extra_info, example is that we send the weight of each dataset item with theScalarResultright now.