Skip to content

An inconsistency between the document of LogisticRegression and code implementation #29509

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.

Sign up for GitHub

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

Jump to bottom
Closed
ParsifalXu opened this issue Jul 17, 2024 · 3 comments
Closed

An inconsistency between the document of LogisticRegression and code implementation #29509

ParsifalXu opened this issue Jul 17, 2024 · 3 comments
Labels
Documentation Needs Triage Issue requires triage spam spam PR and issues

Comments

@ParsifalXu
Copy link
Contributor

ParsifalXu commented Jul 17, 2024 •
edited
Loading

Describe the issue linked to the documentation

Hi,

I may find a potential condition missing in sklearn.linear_model.LogisticRegression.

As mentioned in the document of parameter dual:

dual: bool, default=False
Dual (constrained) or primal (regularized) formulation. Dual formulation is only implemented for l2 penalty with liblinear solver. Prefer dual=False when n_samples > n_features.

Corresponding part found in the source code:

def _check_solver(solver, penalty, dual):
    if solver not in ["liblinear", "saga"] and penalty not in ("l2", None):
        raise ValueError(
            f"Solver {solver} supports only 'l2' or None penalties, got {penalty} "
            "penalty."
        )
    if solver != "liblinear" and dual:
        raise ValueError(f"Solver {solver} supports only dual=False, got dual={dual}")

    if penalty == "elasticnet" and solver != "saga":
        raise ValueError(
            f"Only 'saga' solver supports elasticnet penalty, got solver={solver}."
        )

    if solver == "liblinear" and penalty is None:
        raise ValueError("penalty=None is not supported for the liblinear solver")

    return solver

Apprarently, the relationship between dual and solver are checked without the condition of l2 penalty.
(solver == 'liblinear' && penalty == 'l1' && dual = True) can still pass the _check_solver function.

Could you check it?

Suggest a potential alternative/fix

Perhaps we can modify the judgment condition.
@ParsifalXu ParsifalXu added Documentation Needs Triage Issue requires triage labels Jul 17, 2024
@adrinjalali adrinjalali mentioned this issue Jul 18, 2024
Closed
@adrinjalali
Copy link
Member

This seems AI generated, very similar to #29440

Other issues by the same author seem also AI generated: #29463, #29464

@ParsifalXu we don't appreciate AI generated content (issue or PR). If you find issues and would like to fix, please open PRs for them. Otherwise these issues so far seem low impact and take more energy from maintainers than they bring value. Not banning you from the org since you opened #29468, but more AI generated content might ban you from the org.

@adrinjalali adrinjalali closed this as not planned Won't fix, can't repro, duplicate, stale Jul 18, 2024
@adrinjalali adrinjalali mentioned this issue Jul 18, 2024
Closed
@adrinjalali adrinjalali added the spam spam PR and issues label Jul 18, 2024
@ParsifalXu
Copy link
Contributor Author

ParsifalXu commented Jul 19, 2024 •
edited
Loading

Hi @adrinjalali ,

I sincerely apologize for any extra work this may have caused. However, I want to clarify that this content is not generated by AI. I am a beginner trying to learn how to use your great tool well. I truly appreciate your efforts in developing this fantastic tool and I am simply trying to do some contributions, even if they are just minor documentation improvements.

To better understand how to submit an issue, I reviewed some previous issues and tried to learn how to describe the questions or issues as clear as possible and hoped that you can make a quick fix or confirm (I can submit PRs too). Maybe the similarity in text might lead you to believe this was AI-generated ( I typed every single word by myself! ). So if possible, please remove the spam label. If you think this issue is a problem, you can just confirm it and I will try to make a PR to improve the document, if not, please just let me know and close the issue.

Once again, I apologize for any inconvenience this has caused you and other contributors. I have always believed that good documentation and good code each other. If there are better ways to provide documentation improvements, please let me know.

@adrinjalali
Copy link
Member

Feel free to open PRs in these cases with proper tests if needed, and fixes to documentation.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Documentation Needs Triage Issue requires triage spam spam PR and issues
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@adrinjalali @ParsifalXu