search

JuliaAI / MLJLinearModels.jl

Generalized Linear Regressions Models (penalized regressions, robust regressions, ...)
MIT License
80 stars 13 forks source link

Unexpected behaviour with LogisticClassifier #131

Closed ablaom closed 1 year ago

ablaom commented 1 year ago

https://discourse.julialang.org/t/unexpected-behavior-in-logisticclassifier-mljlinearmodels/90013

ablaom commented 1 year ago

@tlienart Be great if you can take a look at this.

jbrea commented 1 year ago

In #124 we discussed a new default for the regularization constant of logistic regression. I think this default hasn't changed everywhere...

tlienart commented 1 year ago

In #124 we discussed a new default for the regularization constant of logistic regression. I think this default hasn't changed everywhere...

Sorry guys I don't have much time to look into this in details at the moment but this:

https://github.com/JuliaAI/MLJLinearModels.jl/blob/7e34598fa69099f480409e474857abd289e95427/src/glr/constructors.jl#L141-L142

seems to suggest the lambda should be fine. In discourse it says that if they use lambda = 1e-6 they get expected results (and in the original example they call the default constructor LogisticClassifier() which should use the default parameter eps()). This suggests that maybe they're not using the latest version as by default lambda should be close to 0?

jbrea commented 1 year ago

Could the following line be the problem? https://github.com/JuliaAI/MLJLinearModels.jl/blob/55f22b0492152badf66cfac37410341e7fb9a0b6/src/mlj/classifiers.jl#L72

tlienart commented 1 year ago

Thanks a lot @jbrea of course it's that line and https://github.com/JuliaAI/MLJLinearModels.jl/blob/55f22b0492152badf66cfac37410341e7fb9a0b6/src/mlj/classifiers.jl#L29 for the LogisticClassifier, I didn't think of the interface... btw it's probably better there's no default parameter in the interface 🙄