Skip to content

Conversation

rushabh-v
Copy link
Contributor

Fixes #16186

Copy link
Member

@rth rth left a comment

Choose a reason for hiding this comment

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

Thanks @rushabh-v, LGTM!

I think we can probably accept this without a regression test, since the fix is trivial and I don't see an obvious way to test it.

@glemaitre
Copy link
Member

I would still add an entry in what's new thought mentioning that we will check for inferior or equal instead of strictly inferior.

@rushabh-v
Copy link
Contributor Author

I have added the whatsnew

@cmarmo cmarmo removed the Sprint label Jan 29, 2020
rushabh-v and others added 2 commits January 29, 2020 19:52
Co-Authored-By: Guillaume Lemaitre <g.lemaitre58@gmail.com>
Co-Authored-By: Guillaume Lemaitre <g.lemaitre58@gmail.com>
@@ -197,6 +197,11 @@ Changelog

- |Enhancement| improve error message in :func:`utils.validation.column_or_1d`.
:pr:`15926` by :user:`Loïc Estève <lesteve>`.

- |Fix| `optimize._newton_cg` will now avoid an unnecessary iteration by checking for
Copy link
Member

Choose a reason for hiding this comment

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

We also need to focus on public API in what's new. This is private. What public API did this affect?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It affects LogisticRegression when arg solver=newton-cg. I am adding it in whatsnew

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done. Tell me if any change is required...

@jnothman jnothman merged commit 3743a55 into scikit-learn:master Jan 31, 2020
thomasjpfan pushed a commit to thomasjpfan/scikit-learn that referenced this pull request Feb 22, 2020
panpiort8 pushed a commit to panpiort8/scikit-learn that referenced this pull request Mar 3, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Possible unnecessary iterations in Newton-CG algoithm
6 participants