Ping @arjoly. The test failure is unrelated and will be fixed by #3765 (I hope).

@FlorianWilhelm Thanks for handling this case.

It looks good. However, are you sure that is how it should be computed? I am not familiar with this metrics. Would it be better to have L1 space median? In doubt, I would be in favor of raising an error with multi-output data for now. Sorry if my previous comment weren't clear.

@arjoly I think in this case, there is no wrong and right, only consistency. It could also be argued that the way mean_absolute_error handles multi-output might be a bad idea depending on your use-case. What if for instance the first and the second column of y have a completely different scale? Averaging row-wise might then not be best idea.
This PR would do the same thing in case of median_absolute_error only in a robust way by using the median. I think this is a consistent solution and way better than raising an error.

I am +1 when what happens with multi-output data will be stated clearly in model_evaluation.rst.

Thanks for polishing the small details!

@arjoly I added a few words about our approach in model_evaluation.rst. What do you think?

I would be more precise in the median absolute error section. Something like

With multi-output data, the median absolute error is averaged over all outputs.

This current situation will change and improve whenever #3474 is merged.

Coverage decreased (-0.0%) when pulling 7f2b1a4 on FlorianWilhelm:median_absolute_error into 031a3fc on scikit-learn:master.

@arjoly Added a line directly to the median absolute error section.

+1 from my side !
Thanks @FlorianWilhelm !

I am not familiar about the use case of this metric. Intuitively, does it not make sense to find the median across every output and then find the median across all such medians?

However in all other metrics, it has been done the same way, average across all outputs, and then average across all samples, instead of averaging the metrics across all outputs, so at least the user has an idea of what to expect.

So I am +1 for consistency. Thanks,

On second thoughts, I'm not sure taking median across the outputs, (i.e axis=1 ) is meaningful (my limited knowledge, ofc). Unless there is a particular use case (or some reference) that you have in mind, I think just raising an error might be better if y_true.ndim > 1

@MechCoder I think the behavior is as wrong or right as in the case of applying the mean in the other metrics but it would be consistent and it is stated in the documentation like that. Why not just merge this as a first step and in a second larger step we should reconsider the whole multi-output handling anyway. Maybe the user should be able to supply a cost functional to judge the quality of a multi-output.
I think the metrics should be defined only on scalar output, and in case of mulit-output there should be default cost functionals provided that transform a vector into a scalar. Right now these two things are mixed together which is not clean in my opinion.

I agree that it is better not to support multi-output (here; don't worry
about the existing implementation) until someone who has expertise in that
area proposes an algorithm from the literature.

On 3 November 2014 21:29, Florian Wilhelm notifications@github.com wrote:

@MechCoder https://github.com/MechCoder I think the behavior is as
wrong or right as in the case of applying the mean in the other metrics but
it would be consistent and it is stated in the documentation like that. Why
not just merge this as a first step and in a second larger step we should
reconsider the whole multi-output handling anyway. Maybe the user should be
able to supply a cost functional to judge the quality of a multi-output.
I think the metrics should be defined only on scalar output, and in case
of mulit-output there should be default cost functionals provided that
transform a vector into a scalar. Right now these two things are mixed
together which is not clean in my opinion.

—
Reply to this email directly or view it on GitHub
#3764 (comment)
.

Sorry for letting you wait, was quite busy the last weeks. I removed the multioutput feature of median_absolute_error as requested.

Looks good. @arjoly please merge the next time, when you see it.

merged as 167e96e

thanks @FlorianWilhelm !