performance improvements

[tiemvanderdeure]

recode has a specialized dispatch on categorical arrays, so calling unwrap on them has huge overhead

[tiemvanderdeure]

Before and after profviews:
Before:

After:

I'm running this and am getting a 10 times speedup

N = 1000
X = (x1 = rand(Float32, N), x2 = randn(Float32, N), x3 = categorical(rand('a':'c', N)))
y = categorical(bitrand(N))

model = MLJFlux.NeuralNetworkBinaryClassifier(epochs = 10, builder=MLJFlux.MLP(; hidden=(5,4)), batch_size = 100)
mach = machine(model, X, y)
fit!(mach)

@profview for i in 1:100 predict(mach, X) end

[tiemvanderdeure]

Added a commit to change matrices to Float32 before passing them to the chain - otherwise this is done inside the chain anyway, but this throws a warning which has a small overhead, even though it is not printed for some reason. I think we just forgot to do this in #272

[tiemvanderdeure]

The test failure is unrelated, I think?

[ablaom]

Great work @tiemvanderdeure identifying the performance bottleneck, @tiemvanderdeure. I just want to confirm that we are not returning to the problem reported here. Can you please check?

[ablaom]

Yes, it appears MLJFlux is failing on julia 1.11

[tiemvanderdeure]

I just want to confirm that we are not returning to the problem reported here.

I did reintroduce that problem (although I don't think it affects the matrix in the end). I fixed it by calling unwrap after recoding.

The real problem here is that recode is type unstable in CategoricalArrays for normal Array types.

[tiemvanderdeure]

Okay just made a PR to CategoricalArrays.jl that also fixes this JuliaData/CategoricalArrays.jl#407

[ablaom]

@tiemvanderdeure Do you want to wait to see if we get prompt action at CategoricalArrays? Or do think your current solution is a perfectly valid solution here?

[tiemvanderdeure]

Or do think your current solution is a perfectly valid solution here?

Haven't benchmarked this but I think first recoding and then unwrapping might be better even after that PR is merged. One would expect that recoding a categorical array is faster than recoding a normal array. So I think we can merge this as is.