-
Notifications
You must be signed in to change notification settings - Fork 1.3k
FEA Add macro-averaged mean absolute error #780
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
FEA Add macro-averaged mean absolute error #780
Conversation
Hello @AurelienMassiot! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:
Comment last updated at 2021-02-08 23:15:32 UTC |
Codecov Report
@@ Coverage Diff @@
## master #780 +/- ##
==========================================
- Coverage 98.55% 98.55% -0.01%
==========================================
Files 89 89
Lines 5681 5680 -1
Branches 475 477 +2
==========================================
- Hits 5599 5598 -1
Misses 81 81
Partials 1 1
Continue to review full report at Codecov.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I made a quick first pass.
imblearn/metrics/_classification.py
Outdated
all_mae = [] | ||
y_true = np.array(y_true) | ||
y_pred = np.array(y_pred) | ||
for class_to_predict in np.unique(y_true): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think that we should introduce a label
to be able to either give class that are not present in y_true
or select a subset of labels as in precision-recall: https://scikit-learn.org/stable/modules/generated/sklearn.metrics.precision_recall_fscore_support.html#sklearn.metrics.precision_recall_fscore_support
Would it make sense?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it does not make sense because all we want is the MAE for each class really in y_true
.
For example, if I have:
y_true = [1, 2, 2, 3]
I want the average MAE, which will be the MAE for classes 1,2,3. And if a class is missing in y_pred
, it doesn't matter, for example in the following example:
y_pred= [1, 2, 2, 2]
my MAEs will be 0 for class 1, 0 for class 2, 1 for class 3, and the MA-MAE will then be 0.33.
WDYT?
|
||
], | ||
) | ||
def test_macro_averaged_mean_absolute_error(y_true, y_pred, expected_ma_mae): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If we introduce labels, we will need another test with a bit more corner cases.
Otherwise, I think this is good.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See comment above for labels.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See label answer
imblearn/metrics/_classification.py
Outdated
all_mae = [] | ||
y_true = np.array(y_true) | ||
y_pred = np.array(y_pred) | ||
for class_to_predict in np.unique(y_true): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it does not make sense because all we want is the MAE for each class really in y_true
.
For example, if I have:
y_true = [1, 2, 2, 3]
I want the average MAE, which will be the MAE for classes 1,2,3. And if a class is missing in y_pred
, it doesn't matter, for example in the following example:
y_pred= [1, 2, 2, 2]
my MAEs will be 0 for class 1, 0 for class 2, 1 for class 3, and the MA-MAE will then be 0.33.
WDYT?
|
||
], | ||
) | ||
def test_macro_averaged_mean_absolute_error(y_true, y_pred, expected_ma_mae): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See comment above for labels.
Could anyone make a final review ? :-) |
@AurelienMassiot I promise you that it will be merged for the next release which will follow the release in scikit-learn 0.24 |
Thanks @glemaitre ! This is not urgent, good luck for the release of scikit-learn ;-). |
Thanks @AurelienMassiot Good to go |
Reference Issue
As detailed in the issue #18901 I wrote in Scikit-Learn main repository, Macro-Averaged MAE should be added to imbalanced-learn repository instead.
For ordinal classification, we can use multiple metrics, for example: MAE, MSE... As we would use for regression.
But when these classes are imbalanced, one way to deal with imbalance is to use macro-averaged MAE instead, as described on StackExchange and in the original paper.
The macro-averaged MAE is like the "classic" MAE, except we compute each MAE for each class and average them, giving equal weights to MAEs. Note that macro-averaged MAE == micro-averaged (or classic) MAE when class are balanced.
To illustrate this, let's consider:
Any other comments?