[MRG] Enhancement: Add MAPE as an evaluation metric by mohamed-ali · Pull Request #10711 · scikit-learn/scikit-learn

mohamed-ali · 2018-02-26T22:51:42Z

Reference Issues/PRs

Fixes #10708
Closes #6605

What does this implement/fix? Explain your changes.

Implements sklearn.metrics.mean_absolute_percentage_error
Implements the associated neg_mape scorer for regression problems.
Includes tests configurations in tests/test_common.py and tests/test_score_objects.py
Add specific tests with y_true that doesn't include zeros in tests/test_common.py and tests/test_score_objects.py.
Adds docstring + example.
Updates the documentation at doc/modules/model_evaluation.rst and doc/modules/classes.rst

Any other comments?

We have to reach a consensus on the name of the scorer: either neg_mean_absolute_percentage_error_scorer or neg_mape_scorer

jnothman

I think you should note the sensitivity to small y_true, and perhaps we need to validate that y_true is non-zero.

Thanks for the thoroughness!

jnothman · 2018-02-26T23:00:30Z

 **Regression**
 'explained_variance'              :func:`metrics.explained_variance_score`
 'neg_mean_absolute_error'         :func:`metrics.mean_absolute_error`
+'neg_mean_absolute_percentage_error' :func:`metrics.mean_absolute_percentage_error`


@amueller suggested neg_mape here. I'm not entirely sure which is better. But you'll need to widen the table if we go with this name!

Yes, I am open to change the name or keep it as is. But I'll the necessary changes once we agree on a name ;)

@jnothman @amueller I am going to switch from the scorer fname neg_mean_absolute_percentage_error to the neg_mape since MAPE is already a famous acronym in the community and also because it's the only way to avoid pep8 problems.

jnothman · 2018-02-26T23:08:44Z

    >>> cross_val_score(model, X, y, scoring='wrong_choice')
    Traceback (most recent call last):
-    ValueError: 'wrong_choice' is not a valid scoring value. Valid options are ['accuracy', 'adjusted_mutual_info_score', 'adjusted_rand_score', 'average_precision', 'balanced_accuracy', 'brier_score_loss', 'completeness_score', 'explained_variance', 'f1', 'f1_macro', 'f1_micro', 'f1_samples', 'f1_weighted', 'fowlkes_mallows_score', 'homogeneity_score', 'mutual_info_score', 'neg_log_loss', 'neg_mean_absolute_error', 'neg_mean_squared_error', 'neg_mean_squared_log_error', 'neg_median_absolute_error', 'normalized_mutual_info_score', 'precision', 'precision_macro', 'precision_micro', 'precision_samples', 'precision_weighted', 'r2', 'recall', 'recall_macro', 'recall_micro', 'recall_samples', 'recall_weighted', 'roc_auc', 'v_measure_score']
+    ValueError: 'wrong_choice' is not a valid scoring value. Valid options are ['accuracy', 'adjusted_mutual_info_score', 'adjusted_rand_score', 'average_precision', 'balanced_accuracy', 'brier_score_loss', 'completeness_score', 'explained_variance', 'f1', 'f1_macro', 'f1_micro', 'f1_samples', 'f1_weighted', 'fowlkes_mallows_score', 'homogeneity_score', 'mutual_info_score', 'neg_log_loss', 'neg_mean_absolute_error', 'neg_mean_absolute_percentage_error', 'neg_mean_squared_error', 'neg_mean_squared_log_error', 'neg_median_absolute_error', 'normalized_mutual_info_score', 'precision', 'precision_macro', 'precision_micro', 'precision_samples', 'precision_weighted', 'r2', 'recall', 'recall_macro', 'recall_micro', 'recall_samples', 'recall_weighted', 'roc_auc', 'v_measure_score']


You have inspired #10712. Thanks!

jnothman · 2018-02-26T23:09:20Z

+    if y_type == 'continuous-multioutput':
+        raise ValueError("Multioutput not supported "
+                         "in mean_absolute_percentage_error")
+    return np.average(np.abs((y_true - y_pred)/y_true))*100


space around / and * please

Call mean rather than average just to match the name

sklearn-lgtm · 2018-02-26T23:26:20Z

This pull request introduces 2 alerts when merging 7c516ef into 2e30df3 - view on lgtm.com

new alerts:

1 for Explicit export is not defined
1 for Implicit string concatenation in a list

Comment posted by lgtm.com

mohamed-ali · 2018-02-27T08:37:14Z

@jnothman, @amueller, should I raise an error when y_true contains zeros, or do you suggest handling it differently? if so how?

Also, if you have a convention for the error division error message, please suggest it so I can use it, Otherwise, I can define a generic message.

jnothman · 2018-02-27T09:50:37Z

Yes, I think raise an error, but I don't know much about MAPE so I can't say if there are alternative approaches (except the one on Wikipedia which suggests getting differences relative to the mean of y_true). "mean_absolute_percentage_error requires y_true to never be zero" would be sufficient.

mohamed-ali · 2018-02-27T14:30:53Z

@amueller, @jnothman, @qinhanmin2014

The test scenarios in:

sklearn/metrics/tests/test_score_objects.py
sklearn/metrics/tests/test_common.py

generate a y_true sample with zeros , which fails the following check in metrics.mean_absolute_percentage_error :

    if (y_true == 0).any():
        raise ValueError("mean_absolute_percentage_error requires"
                         " y_true to never be zero")

this explains the failures in Travis.CI, could you please suggest what to do?

jnothman · 2018-02-27T22:08:06Z

Pep8 consistency is a very poor excuse for naming inconsistency. I'm unsure what to do with the name, but pep8 is the last of our concerns (stick # noqa at the end of the line and Travis won't fail)

jnothman · 2018-02-27T22:10:47Z

You might need to add a new category in the common tests for regression metrics that require non-zero y, and update y in the tests accordingly.

mohamed-ali · 2018-02-27T23:03:52Z

@jnothman thanks for the tip (# noqa) about travis that's helpful.
regarding the name, I think it's easy to change once we agree on one. So, I'll prioritize fixing the tests.

jnothman · 2018-02-28T01:50:41Z

-                        # matrix instead of a number. Testing of
-                        # confusion_matrix with sample_weight is in
-                        # test_classification.py
+    "confusion_matrix",  # Left this one here because the tests in this file do


Please do not change unrelated things. It makes your contribution harder to review and may introduce merge conflicts to other pull requests.

Ok, my bad. I used autopep8 on the file.

See #10645!

…hamed-ali/scikit-learn into Add-MAPE-as-evaluation-metric

mohamed-ali · 2018-03-27T19:58:21Z

@lesteve travis build finished successfully, so there is no regression with the new changes that you pushed. Thanks!

lesteve

A few comments.

Just curious, maybe for @amueller who opened the associated issue, Wikipedia page does not seem very nice with MAPE, e.g.:
"Although the concept of MAPE sounds very simple and convincing, it has major drawbacks in practical application". Is is still used despite its drawbacks?

lesteve · 2018-03-28T08:24:29Z

 **Regression**
 'explained_variance'              :func:`metrics.explained_variance_score`
 'neg_mean_absolute_error'         :func:`metrics.mean_absolute_error`
+'neg_mape'                        :func:`metrics.mean_absolute_percentage_error`


Why not spell it out fully here since like all the other metrics? i.e. neg_mean_absolute_percentage_error

@lesteve I clarified in the PR description above that the name has to be chosen/voted by all of us. Initially I used neg_mean_absolute_percentage_error but then, since mape is already a famous acronym which, also, makes the metric cleaner, I chose to switch to neg_mape. However, we can change back to the long version, If most of us think that's the right thing to do.

I would be in favour of neg_mean_absolute_error version personally. It is more consistent with neg_mean_absolute_error and more consistent with the metric name ( metrics.mean_absolute_percentage_error). Happy to hear what others think.

I would also be in favor using the explicit expanded name by default and introduce neg_mape as an alias as we do for neg_mse.

Actually we do not have neg_mse. I thought we had.

lesteve · 2018-03-28T08:30:14Z

 - Added the :func:`metrics.balanced_accuracy_score` metric and a corresponding
  ``'balanced_accuracy'`` scorer for binary classification.
  :issue:`8066` by :user:`xyguo` and :user:`Aman Dalmia <dalmia>`.
+- Added the :func:`metrics.mean_absolute_percentage_error` metric and the associated


"Model evaluation" is not the best section for this I would say, in doc/whats_new/v0.19.0.rst there is a "Metrics" section. I think you can do the same here.

Nice catch, I will do that.

lesteve · 2018-03-28T08:34:18Z

 mean_absolute_error_scorer._deprecation_msg = deprecation_msg
+neg_mape_scorer = make_scorer(mean_absolute_percentage_error,
+                              greater_is_better=False)
+


Maybe remove the new line here. When there is no clear rule, my advice would be to follow the same implicit convention as the code you are changing.

mohamed-ali · 2018-03-28T22:02:11Z

@amueller can you approve if all the changes that you requested have been addressed? Thanks!

mohamed-ali · 2018-03-29T08:19:45Z

@jnothman, @amueller, could you please cast you vote on whether to name the scorer neg_mean_absolute_percentage_error_scorer or neg_mape_scorer. So far @lesteve is for the long version whereas I am slightly inclined towards the shorter one. What do you think?

jnothman · 2018-03-29T23:06:13Z

'scorer' isn't part of the name, firstly. Secondly, I think the conventional name of the metric is stupid when I think it's better described as "mean relative error". The only "absolute" thing about it is that it's not squared, as far as I can tell. (And if this measure is known in the literature as "mean relative error" as well, I'd be tempted to use that name. Given the weirdness of its name, I'm tempted to say that the name "MAPE" has a special meaning beyond the words that constitute it, and hence we should consider the short name despite the inconsistency But I don't like the inconsistency either. I also don't think this is something to agonize over.

mohamed-ali · 2018-04-05T13:12:05Z

@lesteve @amueller, after @jnothman comment I assume there is no need to change the name. So I guess my work is done on this issue. @amueller @lesteve could you approve the PR?
Thanks.

mohamed-ali · 2019-05-16T09:03:12Z

@jnothman @amueller Is this PR still considered for merging? if so, let me know so I fix the conflicts. Thanks

jnothman · 2019-05-21T08:51:15Z

I don't see any reason this shouldn't be considered for merge, @mohamed-ali.

I think the docs should emphasise a little more that the metric is not reported as a percentage here.

amueller · 2019-08-16T20:22:42Z

would you mind fixing the conflicts please?

mohamed-ali · 2019-08-16T21:09:35Z

would you mind fixing the conflicts please?

Yes sure, I'll spend some time in the next two days to fix them. Thanks for letting me know.

amueller · 2019-08-16T21:11:41Z

thanks and sorry about the delay in reviewing

amueller · 2019-08-22T14:16:37Z

hm looks like there's a bunch of errors now :-/

mohamed-ali · 2019-08-23T09:15:04Z

hm looks like there's a bunch of errors now :-/

Yes, many things changed since this PR was created. I'll spend more time debugging. If not, I might recreate the PR with a new branch from master.

ogrisel · 2020-03-04T10:43:15Z

Closing in favor of #15007.

mohamed-ali added 10 commits February 26, 2018 21:22

adding MAPE as a new regression loss

d45fb57

adding MAPE api reference

762fa19

adding MAPE scorer

9a6b8fd

adding MAPE to metrics/__init__.py

ea62611

configuring tests for MAPE scorer

70ab1e0

configuring common tests for MAPE metric

962a8e2

correcting import in MAPE example

4f16a45

adding documentation under model_evaluation

e2e93b0

fixing pep8

6df2446

adding more MAPE regression tests

7c516ef

mohamed-ali mentioned this pull request Feb 26, 2018

Add MAPE as evaluation metric #10708

Closed

jnothman reviewed Feb 26, 2018

View reviewed changes

mohamed-ali added 2 commits February 27, 2018 00:30

add whitespace around operators

cb7d875

fix bug: missing comma

8194eb0

mohamed-ali mentioned this pull request Feb 27, 2018

Update Readme and docstring for perceptron #10559

Merged

mohamed-ali added 5 commits February 27, 2018 15:59

avoiding division by zero in test scenario

fd28645

adding check to validate y_true contains no zeros

d10af3c

documenting the new feature in whats_new

61ea463

precising that MAPE is a non symetric metric

088467f

change scorer to neg_mape and fix pep8

f9ac4e4

mohamed-ali added 2 commits February 28, 2018 01:05

adding a metrics category for non-zero y

595bcf6

fix pep8 issues

7575d0e

jnothman reviewed Feb 28, 2018

View reviewed changes

Merge branch 'Add-MAPE-as-evaluation-metric' of https://github.com/mo…

96d6d4a

…hamed-ali/scikit-learn into Add-MAPE-as-evaluation-metric

lesteve reviewed Mar 28, 2018

View reviewed changes

mohamed-ali added 2 commits March 28, 2018 22:07

remove line to keep code consistent

7aac4c0

put mape in metrics section

6f1ab55

qinhanmin2014 mentioned this pull request Oct 2, 2018

[MRG+2] Add max_error to the existing set of metrics for regression #12232

Merged

amueller added the Waiting for Reviewer label Aug 6, 2019

mohamed-ali added 6 commits August 16, 2019 23:30

resolve conflicts

0ac8bc4

fix syntax error

e2184f3

fix merge conflicts in metrics.scorer

3dbe763

fix merge conflicts

83c4aa5

fix merge conflicts

6b350a5

add what's new

bb36550

jnothman mentioned this pull request Sep 18, 2019

Added mean_absolute_percentage_error in metrics fixes #10708 #15007

Merged

github-actions Bot added the module:metrics label Mar 2, 2020

ogrisel closed this Mar 4, 2020

cmarmo removed the Waiting for Reviewer label Mar 4, 2020

cmarmo mentioned this pull request May 2, 2022

[WIP] Add Absolute Mean Percentage Error as available loss function in SGDR… #6605

Closed

Conversation

mohamed-ali commented Feb 26, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Reference Issues/PRs

What does this implement/fix? Explain your changes.

Any other comments?

Uh oh!

jnothman left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sklearn-lgtm commented Feb 26, 2018

Uh oh!

mohamed-ali commented Feb 27, 2018

Uh oh!

jnothman commented Feb 27, 2018 via email

Uh oh!

mohamed-ali commented Feb 27, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jnothman commented Feb 27, 2018

Uh oh!

jnothman commented Feb 27, 2018

Uh oh!

mohamed-ali commented Feb 27, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mohamed-ali commented Mar 27, 2018

Uh oh!

lesteve left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mohamed-ali Mar 28, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mohamed-ali commented Mar 28, 2018

Uh oh!

mohamed-ali commented Mar 29, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jnothman commented Mar 29, 2018 via email

Uh oh!

mohamed-ali commented Apr 5, 2018

Uh oh!

mohamed-ali commented May 16, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mohamed-ali commented Feb 26, 2018 •

edited

Loading

mohamed-ali commented Feb 27, 2018 •

edited

Loading

mohamed-ali commented Feb 27, 2018 •

edited

Loading

lesteve left a comment •

edited

Loading

mohamed-ali Mar 28, 2018 •

edited

Loading

mohamed-ali commented Mar 29, 2018 •

edited

Loading

mohamed-ali commented May 16, 2019 •

edited

Loading