MLJ interface

[tlienart]

Code suggestions

• num_features_and_observations can just be reverse(size(X)) and the other one size(X), also given how you use it, it's probably best to just use size itself but YOLO
• you could make your MulticlassPerceptronClassifier object use the elemnt type as a parametric type and not carry around the element type

    if Xnew isa AbstractArray
        Xnew  = MLJBase.matrix(Xnew)
    elseif Tables.istable(Xnew) 
        Xnew  = MLJBase.matrix(Xnew, transpose=true)
    end

replace with

if Tables.istable(Xnew)
  Xnew = ...
end

• for readability it'd be better to put fit before predict
• same comment with the unnecessary abstractarray test in fit
• please do not explicitly specify that y::CategoricalArray (this is done upstream)
• please do not print lines that are not constrained by the verbosity element. and definitely for non informative lines like "training"

Why are there two implementations of predict ?

Tests

You currently don't have tests, I would suggest removing all notebooks and converting them in tests but even if you keep notebooks, please add rigorous tests, we will only add tested packages to the registry.

In the tests, please test the interface (again have a look at MLJLinearModels for examples)

In the tests please also consider benchmarking against sklearn's MLP implementation

Next step

Once all that is done, the steps are:

• release your package to the Julia registry
• open an issue on MLJModels for us to add your registered package to the registry

[davidbp]

Thank you for your notes,

I have some issues with respect to testing (with respect to the implementation of the learning algorihtm).

In the case of regression models you can use a closed form solution such as θ = (X1'X1 + λ*I)\(X1'y). This does not exist (as far as I know) for a multiclass perceptron.

A possible solution could be to generate a bunch of datasets of learning problems, ensure that they are linearly separable, and test that the model can get a perfect accuracy on those problems. What do you think about this possible solution? I actually opened an issue on MLJ for having a piece of code to generate datasets. I haven't done this yet but I would like to have something similar to this.

With respect to testing vs MLP in sklearn I guess you mean with the Perceptron implementation (not the MultilayerPerceptron) of sklearn (the implementation in this repo has no hidden units). This is not an apple to apple comparisson but it could give some confidence.

I have seen in the logistic case you use: @test mcr ≤ 0.2. Maybe I could train in the same dataset a perceptron version of sklearn and get a threshold?

[tlienart]

A possible solution could be to generate a bunch of datasets of learning problems, ensure that they are linearly separable, and test that the model can get a perfect accuracy on those problems. What do you think about this possible solution? I actually opened an issue on MLJ for having a piece of code to generate datasets but I did not code it.

Yes a first sanity check is to generate points in a low dimensional spaces that are very separated and make sure they're predicted to be in the right class with high accuracy

More generally you want two kinds of tests:

1. that the algorithm does what it's supposed to; indeed there's stochasticity but you should get a good idea, and comparing with Sklearn is an excellent way to do this to get comparable benchmarks
2. that the stuff you expose to the user "just works" (constructors, keywords, ...) this is unrelated to ML and is purely deterministic

I don't completely follow your point about hidden units.

Basically yes compare to Sklearn's perceptron which can also do multiclass by using one-vs-all type scoring. Definitely first start with the binary case and make sure you get comparable performances in several scenarios, then move on to the multiclass one.
Check that your performances are within a factor of 2-3 of the python implementation, ideally they would be faster of course but let's say if it's 5x slower then we wouldn't really want to add that to the registry. So yeah testing+benchmarks if that makes sense.

Thanks!

[davidbp]

Since I haven't registered any package in julia I am not sure I have properly done this.
I can see I have generated a pull request here:

JuliaRegistries/General#6370

Now I need to make the issue to MLJModels

[ablaom]

It looks good to me. Because this is a new package, it may take a few days. I think there is a stand-down period of some days for new packages to allow comment. Future versions are usually auto-merged within half an hour, assuming they meet the auto-merge criterion.

[azev77]

@davidbp are you still interested in including this MLP in MLJ?

[davidbp]

@azev77 it´s been quite a while, I would like to finish the interface now.

[ablaom]

Let me know if I can assist.

[ablaom]

https://alan-turing-institute.github.io/MLJ.jl/dev/quick_start_guide_to_adding_models/

[azev77]

Now that MLJFlux is working, don’t we have access to MLPs via Flux?
@ablaom

[ablaom]

Yep.

[davidbp]

I have updated the tests with an example of a linearly separable case with several classes to verify that the model can separate it perfectly (as expected).
Also notebooks/learning_tests.ipynb shows graphically what we would expect from this model.

There is only something that I do not like, @ablaom maybe you can suggest how to solve this.
I have a simple struct MulticlassPerceptronCore with a fit and predict methods. Since I import methods with the same name in the "Core version" and the "MLJ version" they conflict and I have to use MLJbase.predict and MulticlassPerceptron.predict.

This can be seen in the test cases:

@testset "MulticlassPerceptronCore (tests model implementation)" begin
    @testset "n=100, p=2, n_classes=2, sparse=false, f_average_weights=true" begin
        Random.seed!(6161)
        n, p, n_classes, sparse, f_average_weights = 100, 2, 2, false, true
        X, y = MulticlassPerceptron.make_blobs(n; centers=n_classes, random_seed=4, return_centers=false)
        X = copy(X')

        perceptron_core = MulticlassPerceptronCore(Float64, n_classes, p, sparse)
        fit!(perceptron_core, X, y; n_epochs=100, f_average_weights=true)
        ŷ = MulticlassPerceptron.predict(perceptron_core, X)
        @test length(ŷ)==length(y)
        @test mean(ŷ .== y)>0.95
    end
end

If I write the same code as before using simply predict it does not work.

[ablaom]

Yes, if you are importing both predicts into the same namespace, then you need to qualify them. I don't see that this a big deal. In practice, if your model is registered in MLJ then, in a normal MLJ workflow, the user will never do using MulticlassPerceptron and hence create the conflict of MulticlassPerceptron.predict with MLJ.predict = MLJ.MLJModelInterface.predict. In the standard workflow, only the type MulticlassPerceptronClassifier gets pulled into the user's namespace, for that is all they will need.

FYI MLJ finds this type from the load_path trait (which looks good to me) and you can even drop export MulticlassPerceptronClassifier if you want.

If the conflict really bothers you, you can overload MLJModelInterface.predict in your core code instead of overloading StatsBase.predict, which is what you are doing currently. So change

MulticlassPerceptron.jl/src/MulticlassPerceptronCore.jl

Line 7 in ab8f85c

import StatsBase: fit!, predict

to

import StatsBase: fit!
import MLJModelInterface.predict

The reason MLJModelInterface does not overload StatsBase.predict is that it aims to be an ultra-lightweight package, so we don't want StatsBase as a dependency.

By the way, you ought to be able to remove MLJBase as a dependency in your code (which will speed up loading considerably, I expect) but replacing MLJBase references with MLJModelInterface references. You will still need MLJBase as a test dependency, however https://alan-turing-institute.github.io/MLJ.jl/dev/quick_start_guide_to_adding_models/#Important-1

Does this help?

[davidbp]

It helps a lot, thank you.

I just removed the explicit MLJBase.predict and MulticlassPerceptron.predict from the tests and with your suggested change works as expected (and all tests pass).

Neverhteless, I think I still need MLJBase in interface.jl for the following function:

function MLJModelInterface.predict(fitresult::Tuple{MulticlassPerceptronCore, MLJBase.CategoricalDecoder}, Xnew)

    # Function fit!(MulticlassPerceptronCore, X, y) expects size(X) = n_features x n_observations
    if Xnew isa AbstractArray
        Xnew  = MLJBase.matrix(Xnew)
    elseif Tables.istable(Xnew)
        Xnew  = MLJBase.matrix(Xnew, transpose=true)
    end

    result, decode = fitresult
    prediction     = predict(result, Xnew)
    return decode(prediction)
end

Since Categoricaldecoder does not seem to be in MLJModelInterface.

[ablaom]

That's odd. MLJModelInterface.decoder is defined. It is used in this XGBoost interface: https://github.com/alan-turing-institute/MLJXGBoostInterface.jl/blob/2e31da281e74c9532d5befac3e75dbd97cf9f3b2/src/MLJXGBoostInterface.jl#L642 which does not have MLJBase as dependency. (MMI is an alias for MLJModelInterface).

Perhaps you can provide more detail about the error you are getting.

[davidbp]

If I substitute all references in interface.jl of MLJBase for MLJModelInterfaceI get

ERROR: LoadError: LoadError: UndefVarError: CategoricalDecoder not defined

since MLJModelInterface.CategoricalDecoder is not found. This breaks

function MLJModelInterface.predict(fitresult::Tuple{MulticlassPerceptronCore, MLJModelInterface.CategoricalDecoder}, Xnew)

since it cannot be compiled because it depends on MLJModelInterface.CategoricalDecoder.

Note that

julia> typeof( MLJModelInterface.decoder(y[1]))
MLJBase.CategoricalDecoder{Int64,UInt32}

So it is true that MLJModelInterface.decoder exists, but MLJModelInterface.CategoricalDecoder does not exist.