FEA add zero_division to matthews_corrcoef #28509
Conversation
|
@glemaitre WDUT? |
|
I'll had this PR for the new milestones. I'll provide a review. |
There was a problem hiding this comment.
So for consistency with other metrics and the fact that to have a single class is kind of an extreme "weird" use case in practice, I want that we add the zero_division parameter as it was previously decided: #23183 (comment)
|
@Redjest Are you able to address the comments? |
|
glemaitre
changed the title
There was a problem hiding this comment.
I push the modification directly in this branch. I added the zero_division parameter as discussed before and modify the test accordingly.
@adrinjalali you might want to have a look at this one. I think that we can include it in 1.6
|
Also @lucyleeow is aware of this topic ;) |
Reference Issues/PRs
Partially address: #29048
Fixes #25258
See also #19977
What does this implement/fix? Explain your changes.
The Matthews correlation coefficient is ill-defined (due to zero division) when only one class is present in either the true or predicted labels.
If only one of the true or predicted labels contains a single class, the limit value is 0 (can be shown using polar coordinates). This is sensible as it suggests that the model either provided constant predictions on non-constant data, or variable predictions on single-class data. In such cases, the metric should return a value of 0.
However, if both the true and predicted labels contain only a single class, the limit does not exist, rendering the metric undefined. Also, if both true and model labels are single class, the model succeeded in trivial task and we genuinely can't tell if the correlation is good or poor. Consequently, in this scenario, the metric should return a nan value. This behavior was chosen to avoid returning 0 for perfect predictions on single-class data.
Any other comments?
I noticed that for some other metrics the handling of zero division was based on
_prf_divide(), but it didn't seem a good fit for this case as its default behavior is to return 0 on both cases and IMHO directly returning nan or 0 yields more readable code and is simpler for users.@marctorsoc @glemaitre WDYT?