Confusion matrices as measures

[c42f]

Hi, I'm doing some model building for small data classification using MLJ.

I would like to use the confusion matrix directly for evaluation by summing it over cross validation folds. (Personally I often like to use the confusion matrix directly to assess model performance as it avoids many of the caveats which come from trying to summarize it into a single number.)

With that in mind, I tried something like

labels, features = unpack(...)

leave_one_out_cv = CV(nfolds=length(labels)) # small data!
evaluate!(machine(LinearSVC(), features, labels),
          resampling=leave_one_out_cv,
          measure=confusion_matrix
          )

However, this fails with an error deep in the aggregation machinery (confusion matrices cannot be summed etc)

You seem to almost have the pieces in place for this to work, so for now I worked around this with some type piracy:

# Hacks to allow us to treat the confusion matrix as an MLJBase measure.

MLJBase.aggregation(::Type{typeof(confusion_matrix)}) = MLJBase.Sum()
Base.round(m::MLJBase.ConfusionMatrix; kws...) = m

function Base.:+(m1::MLJBase.ConfusionMatrix, m2::MLJBase.ConfusionMatrix)
    if m1.labels != m2.labels
        throw(ArgumentError("Confusion matrix labels must agree"))
    end
    MLJBase.ConfusionMatrix(m1.mat + m2.mat, m1.labels)
end

Is this questionable? Can it be made to "just work" using something like the above, or am I just abusing the machinery in some way?

(Side note - I'm finding MLJ/MLJBase fantastic so far. In particular I'm greatly enjoying the way that metadata about labels and features is naturally preserved throughout the training pipeline. Bravo, this is the way it should be!)

[c42f]

I see there was some prior discussion about this in #88 (comment) but I couldn't see that a definitive decision was made there.

[tlienart]

Thanks for the feedback! Yeah I think @ablaom considered it so he's better placed than I to answer that.

There's heavy refactoring of MLJBase going on (#166) so such a modification could reasonably easily be added in the mix and be released soonish.

[ablaom]

@c42f Thanks for the positive feedback. Your request sounds reasonable. Here's what makes it tricky. You would like to make confmat a measure, but there is currently an inconsistency in the way confmat works and the way measures work, having to do with how parameters, like rev (for reverse class order) are set. For measures, a parameter is passed during construction of the measure, as in my_true_positive_rate = TruePositiveRate(rev=true). Now you can call my_true_positive_rate on (yhat, y) but you cannot give it any kwargs. Measures are not designed to accept kyargs. On the other hand, there is no analogous constructor for a confusion_matrix callable object, just the confmat function (which takes the kwargs, rev=... or perm=...).

To fix this is not a simple refactor, so will probably not happen soon. Here's a proposal:

First, note it is problematic to have measures whose definition depends on seeing the data ahead of time, eg, depends on the number of classes (or worse, the names of the classes). One wants to be able to fix a measure without necessarily knowing such information. So the new confusion matrix measure cannot have perm as a parameter. As a compromise, we could have a single parameter rev for reversing the order of the classes with this behaviour: rev=nothing: use order of levels(y), issuing a warning if y is Multiclass and not OrderedFactor (as we do elsewhere); rev=true or rev=false suppresses the warnings and either uses the order levels(y) or the reverse. This makes sense even for classes > 2. If you really want a different order, you can change the levels yourself with levels!(y, ...).

So, we do something like this:

• leave the existing confusion matrix stuff alone: ConfusionMatrix type, constructor, and the function confmatrix (and any aliases?)

• define new measure type Confusion{R} <: Measure where R is nothing, true or false with a constructor Confusion(; rev=nothing) = Confusion{R}(), and confusion = Confusion(), alias conf.

• make instances of Confusion callable:

(::Confusion{nothing})(yhat, y) = confusion_matrix(yhat, y)
(::Confusion{false})(yhat, y) = confusion_matrix(yhat, y, perm = 1:length(levels(y))
(::Confusion{true})(yhat, y) = confusion_matrix(yhat, y, perm = reverse(1:length(levels(y)))

• give the Confusion type the required measure traits, including Sum aggregation along the lines suggested by @c42f . Probably orientation should be set to :other, as this is not a single number.

[c42f]

Right, those changes sound a bit deeper than the amount of time I've got to spend on this right now, but thanks for the ideas!

First, note it is problematic to have measures whose definition depends on seeing the data ahead of time, eg, depends on the number of classes (or worse, the names of the classes).

Currently ConfusionMatrix{L} seems to encode the number of labels in the type parameter L. I assume that also constitutes knowing about the classes? TBH I was a bit surprised that this was in the type parameter so maybe it can just be left out of Confusion.

[tlienart]

@ablaom maybe one way forward that would allow to just use

labels, features = unpack(...)

leave_one_out_cv = CV(nfolds=length(labels)) # small data!
evaluate!(machine(LinearSVC(), features, labels),
          resampling=leave_one_out_cv,
          measure=confusion_matrix
          )

as Chris assumed he could, would be to have a special case for the aggregate function for when the keyword confusion_matrix is used (or one of its aliases) and to clarify in the docs that when this is the case the current ordering of levels is assumed to be the one to use & that if they want to change the ordering, they should do so with levels! ?

In terms of usability I think it's better to do something like that than export another type with a confusion name which would really get people confused (...).

[c42f]

In terms of usability I think it's better to do something like that than export another type with a confusion name which would really get people confused

That sounds like a much nicer surface API if it can work out internally 👍

[ablaom]

@tlienart Thanks for that.

Okay, if we are happy to forgo the possibility of changing the order, then there is no reason we cannot turn confusion_matrix directly along the lines suggested. One may have to live with the warnings about ordering of labels if using unordered data, but I suppose we can live with that.

[ablaom]

Implemented in this release: JuliaRegistries/General#9591

[c42f]

Great, thanks!