Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Unexpected behaviour with LogisticClassifier #131

Closed
ablaom opened this issue Nov 13, 2022 · 5 comments
Closed

Unexpected behaviour with LogisticClassifier #131

ablaom opened this issue Nov 13, 2022 · 5 comments
Labels
bug Something isn't working

Comments

@ablaom
Copy link
Member

ablaom commented Nov 13, 2022

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

@ablaom ablaom added the bug Something isn't working label Nov 13, 2022
@ablaom
Copy link
Member Author

ablaom commented Nov 13, 2022

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

@jbrea
Copy link
Collaborator

jbrea commented Nov 14, 2022

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

@tlienart
Copy link
Collaborator

tlienart commented Nov 14, 2022

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:

function LogisticRegression(
λ::Real=eps(),

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
Copy link
Collaborator

jbrea commented Nov 14, 2022

Could the following line be the problem?

lambda::Real = 1.0

@tlienart
Copy link
Collaborator

Thanks a lot @jbrea of course it's that line and

lambda::Real = 1.0
for the LogisticClassifier, I didn't think of the interface... btw it's probably better there's no default parameter in the interface 🙄

This was referenced Nov 15, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants