Skip to content

FIX LogisticRegression warm start with newton-cholesky solver - #31866

Merged
OmarManzoor merged 4 commits into
scikit-learn:mainfrom
lorentzenchr:fix_multinomial_newton_cholesky
Aug 11, 2025
Merged

OmarManzoor merged 4 commits into
scikit-learn:mainfrom
lorentzenchr:fix_multinomial_newton_cholesky

Conversation

@lorentzenchr

Copy link
Copy Markdown
Member

Reference Issues/PRs

Fixes ##31859.

What does this implement/fix? Explain your changes.

LogisticRegression(warm_start=True, solver="newton-cholesky") for multi-class problems has a bug in that the already provided (warm started) coefficients are partially overwritten by zero instead of "unsymmetrized".

Any other comments?

A corresponding test is added.

@github-actions

github-actions Bot commented Aug 1, 2025 •

Copy link
Copy Markdown

✔️ Linting Passed

All linting checks passed. Your pull request is in excellent shape! ☀️

Generated for commit: 904de3e. Link to the linter CI: here

@lorentzenchr

Copy link
Copy Markdown
Member Author

@david-cortes If you could review this one that would be great!

@ogrisel

ogrisel commented Aug 7, 2025 •

Copy link
Copy Markdown
Member

This PR needs to be synced with main to get the required Linux pymin_conda_forge_arm green.

@david-cortes

Copy link
Copy Markdown
Contributor

Thanks for looking into it. Can confirm that the earlier problem with the intercepts is now solved.

However it seems to be applying the warm-start logic in cases where it shouldn't - not sure if connected to the changes here though:

import numpy as np
from sklearn.datasets import load_iris
from sklearn.linear_model import LogisticRegression
X, y = load_iris(return_X_y=True)
y_bin = (y == 0).astype(np.float64)

model1 = LogisticRegression(
    solver="newton-cholesky",
    max_iter=1
).fit(X, y_bin)
model2 = LogisticRegression(
    solver="newton-cholesky",
    max_iter=1,
    warm_start=True
).fit(X, y).fit(X, y_bin)

# Coefficients are not equal
np.testing.assert_almost_equal(
    model1.coef_,
    model2.coef_
)

@lorentzenchr

Copy link
Copy Markdown
Member Author

@david-cortes The assert failure of your snippet goes away if you set max_iter=2 in model1 and if you always use y_bin instead of y (model2.fit).

@lorentzenchr lorentzenchr added the Waiting for Second Reviewer First reviewer is done, need a second one! label Aug 11, 2025

@OmarManzoor OmarManzoor left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM. Thank you @lorentzenchr

@OmarManzoor
OmarManzoor merged commit 217fe94 into scikit-learn:main Aug 11, 2025
@lorentzenchr
lorentzenchr deleted the fix_multinomial_newton_cholesky branch August 11, 2025 08:26
@david-cortes

Copy link
Copy Markdown
Contributor

@david-cortes The assert failure of your snippet goes away if you set max_iter=2 in model1 and if you always use y_bin instead of y (model2.fit).

@lorentzenchr Yes, but I don't think it should warm-start with coefficients that do not correspond to the same type of model being fitted.

lucyleeow pushed a commit to lucyleeow/scikit-learn that referenced this pull request Aug 22, 2025
jeremiedbb pushed a commit to jeremiedbb/scikit-learn that referenced this pull request Sep 3, 2025
@jeremiedbb jeremiedbb mentioned this pull request Sep 3, 2025
13 tasks done
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

module:linear_model Waiting for Second Reviewer First reviewer is done, need a second one!

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Intercepts of Newton-Cholesky logistic regression get corrupted when warm starting

4 participants