Skip to content

DOC Improve the mathematical description of Logistic Regression#22382

Merged
glemaitre merged 44 commits intoscikit-learn:mainfrom
Micky774:logistic_math_docs
Jun 9, 2022
Merged

DOC Improve the mathematical description of Logistic Regression#22382
glemaitre merged 44 commits intoscikit-learn:mainfrom
Micky774:logistic_math_docs

Conversation

@Micky774
Copy link
Copy Markdown
Contributor

@Micky774 Micky774 commented Feb 4, 2022 •
edited
Loading

Reference Issues/PRs

Addresses #21985

What does this implement/fix? Explain your changes.

Restructures module docs for logistic regression to try to make the explanation clearer.

Any other comments?

@Micky774 Micky774 marked this pull request as draft February 4, 2022 21:38
@Micky774 Micky774 marked this pull request as ready for review February 5, 2022 21:56
@Micky774 Micky774 mentioned this pull request Feb 5, 2022
Closed
@Micky774 Micky774 changed the title WIP DOC Improve the mathematical description of Logistic Regression DOC Improve the mathematical description of Logistic Regression Feb 6, 2022
lorentzenchr
lorentzenchr reviewed Feb 9, 2022
View reviewed changes
Copy link
Copy Markdown
Member

@lorentzenchr lorentzenchr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for working on this. Here are a few opinionated review comments.
On top, could you also replace
"Logistic regression, despite its name, is a linear model for classification" -> "Logistic regression, despite its name, is a generalized linear model for classification"

lorentzenchr
lorentzenchr reviewed Feb 21, 2022
View reviewed changes
Micky774 and others added 4 commits February 22, 2022 12:56
@Micky774 @lorentzenchr
Apply suggestions from code review
c424da2 
Co-authored-by: Christian Lorentzen <lorentzen.ch@gmail.com>
@Micky774 @lorentzenchr
Update doc/modules/linear_model.rst
701c811 
Co-authored-by: Christian Lorentzen <lorentzen.ch@gmail.com>
@Micky774
Merge branch 'main' into logistic_math_docs
c4a1a17
@Micky774
Merge branch 'logistic_math_docs' of https://github.com/Micky774/scik…
740df44 
…it-learn into logistic_math_docs
@Micky774
Copy link
Copy Markdown
Contributor Author

Micky774 commented Feb 25, 2022

@arthurmensch -- I see your name from git blame on the template file _sag_fast.pyx.tp -- I wanted to ask how the l_1 norm was actually implemented -- i.e. l_1 of the flattened vector of weights, or l_21, the l_1 of the l_2 of rows. I can only see that the scalar controlling the ratio of l_1 penalty in sag is beta, but am unclear as to which norm it actually corresponds to.

@glemaitre
Copy link
Copy Markdown
Member

glemaitre commented Mar 9, 2022

@ogrisel you probably want to have a look at the discussion.

thomasjpfan
thomasjpfan reviewed Apr 28, 2022
View reviewed changes
Micky774 and others added 2 commits April 29, 2022 18:09
@ArturoAmorQ ArturoAmorQ mentioned this pull request May 25, 2022
Merged
@ArturoAmorQ
Copy link
Copy Markdown
Member

ArturoAmorQ commented May 27, 2022

Thanks for the PR @Micky774.

I made a review on my repo. Feel free to cherry-pick it from this commit.

The same idea of using a table should then be done for the Multinomial Case for consistency.

@Micky774
Copy link
Copy Markdown
Contributor Author

Micky774 commented May 27, 2022

Thank you @ArturoAmorQ , the tabular presentation is a great addition!

agramfort
agramfort reviewed May 30, 2022
View reviewed changes
lorentzenchr
lorentzenchr approved these changes May 30, 2022
View reviewed changes
Copy link
Copy Markdown
Member

@lorentzenchr lorentzenchr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Some final nitpicks.

@lorentzenchr
Copy link
Copy Markdown
Member

lorentzenchr commented May 30, 2022

@agramfort Thanks for clarifying the issue about the regularization term.

@lorentzenchr lorentzenchr linked an issue May 31, 2022 that may be closed by this pull request
Improve the mathematical description of Logistic Regression #21985
Closed
glemaitre
glemaitre approved these changes Jun 9, 2022
View reviewed changes
Copy link
Copy Markdown
Member

@glemaitre glemaitre left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Otherwise LGTM

@glemaitre glemaitre merged commit c8e0559 into scikit-learn:main Jun 9, 2022
@glemaitre
Copy link
Copy Markdown
Member

glemaitre commented Jun 9, 2022

Thanks @Micky774

@Micky774 Micky774 deleted the logistic_math_docs branch June 9, 2022 15:44
ogrisel pushed a commit to ogrisel/scikit-learn that referenced this pull request Jul 11, 2022
@Micky774 @lorentzenchr
@thomasjpfan @lesteve @glemaitre @ogrisel
DOC Improve the mathematical description of Logistic Regression (scik…
da635f6 
…it-learn#22382)

Co-authored-by: Christian Lorentzen <lorentzen.ch@gmail.com>
Co-authored-by: Thomas J. Fan <thomasjpfan@gmail.com>
Co-authored-by: ArturoAmorQ <arturo.amor-quiroz@polytechnique.edu>
Co-authored-by: Loïc Estève <loic.esteve@ymail.com>
Co-authored-by: Guillaume Lemaitre <g.lemaitre58@gmail.com>
glemaitre added a commit to glemaitre/scikit-learn that referenced this pull request Aug 4, 2022
@Micky774 @lorentzenchr
@thomasjpfan @lesteve @glemaitre
DOC Improve the mathematical description of Logistic Regression (scik…
a89190e 
…it-learn#22382)

Co-authored-by: Christian Lorentzen <lorentzen.ch@gmail.com>
Co-authored-by: Thomas J. Fan <thomasjpfan@gmail.com>
Co-authored-by: ArturoAmorQ <arturo.amor-quiroz@polytechnique.edu>
Co-authored-by: Loïc Estève <loic.esteve@ymail.com>
Co-authored-by: Guillaume Lemaitre <g.lemaitre58@gmail.com>
glemaitre added a commit that referenced this pull request Aug 5, 2022
@Micky774 @lorentzenchr
@thomasjpfan @lesteve @glemaitre
DOC Improve the mathematical description of Logistic Regression (#22382)
4d5e04c 
Co-authored-by: Christian Lorentzen <lorentzen.ch@gmail.com>
Co-authored-by: Thomas J. Fan <thomasjpfan@gmail.com>
Co-authored-by: ArturoAmorQ <arturo.amor-quiroz@polytechnique.edu>
Co-authored-by: Loïc Estève <loic.esteve@ymail.com>
Co-authored-by: Guillaume Lemaitre <g.lemaitre58@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@ogrisel ogrisel ogrisel left review comments

@agramfort agramfort agramfort left review comments

@thomasjpfan thomasjpfan thomasjpfan left review comments

@glemaitre glemaitre glemaitre approved these changes

@lorentzenchr lorentzenchr lorentzenchr approved these changes

Assignees

No one assigned

Labels

Documentation

Projects

No open projects
Review sprint - Mai 2022
Status: Done

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Improve the mathematical description of Logistic Regression

8 participants

@Micky774 @glemaitre @ArturoAmorQ @lorentzenchr @ogrisel @agramfort @thomasjpfan @lesteve