Mlj.jl: Check number of levels of y_train before calling fit

[tlienart]

In #540 the issue can be summarized as follows:

• user gets data from somewhere, target has 3 classes
• user decides to drop a class in some way, doesn't adjust the levels
• target vector still has 3 classes but length(unique(y)) == 2
• user passes this to a model, the interface uses the blueprint which has nclasses = length(MMI.classes(y[1])) (which is 3)
• the model fit function works on the encoded data and uses the fact that there's only really 2 classes

ambiguity ensues and things crash at prediction time (fitresult considers binary class, but the passed class labels have length 3)

There are a number of things that could be done to mitigate this from the model side but it seems to me that this could happen more than once so we should probably think about it from the MLJ side too. A possibility would be to squeeze y to its non-empty classes before passing it to fit.

Edit: the below is outdated now, see comment at the end
In MLJLM I'm going to go with the following fix for the moment:

https://github.com/alan-turing-institute/MLJLinearModels.jl/blob/2dc0dc5cc5b69cba9b3e275c4bfa62f09d5afdd9/src/mlj/interface.jl#L47-L50

~~to prevent the error but it's not entirely satisfactory and should probably be revisited. ~~

[CameronBieganek]

I'm still pretty new to the MLJ ecosystem, so I'm not sure of the right approach. But if you're doing 5-fold cross-validation on a small data set, there's a decent chance that sometimes you'll get a training fold where not all levels are represented in the target vector. In that situation, you probably still want to treat the model as an, e.g., 3-class model rather than a 2-class model. In other words, the fit should probably always use the number of levels indicated by levels(y).

the model fit function works on the encoded data and uses the fact that there's only really 2 classes

So, if I understand correctly, maybe MLJLM should be changed to use the number of levels indicated by levels(y) rather than length(unique(y))?

[tlienart]

Yes that’s fair just a couple of notes:

1. If a specific implementation style is expected from package devs for handling number of classes (like: explicitly use nclasses and nothing else bc it might be wrong) then this should be made explicit
2. It may be that MLJ can provide ways to mitigate the risk of such errors

There may even be models that just don’t work if there are no examples of a given class.

I’m willing to adjust some things in MLJLM but it’s not entirely straightforward (and a bit annoying) to pass a number of class from the interface to the actual fit method. This got me thinking that the problem might arise again and so that it might be worth considering it from the MLJ side too

[tlienart]

MLJLM 0.4 now does this properly in the sense that it keeps track of the number of levels passed. @CameronBieganek that also means that your initial example (without the droplevel) now works (in fact it's now a unit test). Thanks for raising the issue in the first place!

[CameronBieganek]

Awesome! If it turns out that I steered you in the wrong direction, I'll take the blame. :)