MNT replace log_logistic by logaddexp

[lorentzenchr]

Reference Issues/PRs

No

What does this implement/fix? Explain your changes.

This PR replaces log_logistic(x) by the more precise and faster -np.logaddexp(0, -x).
The whole Cython module _logistic_sigmoid is therefore removed.

Any other comments?

import numpy as np
from sklearn.utils.extmath import log_logistic
import mpmath as mp

mp.dps = 100

# First about precision
log_logistic(np.array([36.7368005]))  # -2.22044605e-16
log_logistic(np.array([36.7368006]))  # 0
-np.logaddexp(0, -36.7368005)         # -1.1102231019822803e-16
-np.logaddexp(0, -36.7368006)         # -1.1102229909599743e-16
-mp.log1p(mp.exp(-36.7368005))        # mpf('-1.1102231019822803e-16')
-mp.log1p(mp.exp(-36.7368006))        # mpf('-1.1102229909599743e-16')

# Then about speed
x = np.linspace(-30, 30, 1000)
%timeit log_logistic(x)         # 73.4 µs ± 3.24 µs per loop
%timeit (-np.logaddexp(0, -x))  # 19.7 µs ± 539 ns per loop

[github-actions]

✔️ Linting Passed

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

_{Generated for commit: efd77f3. Link to the linter CI: here}

[lorentzenchr]

I set the label "no changelog needed" as it seems off to add an efficiency entry in whatsnew to the neural net module. One might get the wrong impression.

[glemaitre]

This function was still "kind" of public. It can internally call np.logaddexp now.
Can we deprecate it and asking to use the numpy one and then we remove it in 2 versions.

[lorentzenchr]

But it is not listed in our API docs. So it is not public.
Do you know of any user/3rd party using that function from us?

[lorentzenchr]

https://github.com/search?q=log_logistic+language%3APython&type=code&l=Python&p=1 gives 904 hits, most of which are forks of scikit-learn. For me, that's strong evidence that we can remove. I can add a changelog and say with what function to replace it.

[glemaitre]

But it is not listed in our API docs.

Yes, we have some legacy and bad practices that should be fixed. We should have clear public, developer, and private tools.

So it is not public.

This is tricky because the public/private agreement would be the presence or not of a leading _.

Do you know of any user/3rd party using that function from us?

Apparently some do: https://github.com/search?q=from+sklearn.utils.extmath+import+log_logistic&type=code

[glemaitre]

For instance, this is one third-party library: https://github.com/VowpalWabbit/vowpal_wabbit with 8.3k stars and 2k forks. Removing without notice will make a lot of people angry at once :)

I am still advocating for a deprecation. I know that this is frustrating for such a tools that should have never been public but this is safer.

[lorentzenchr]

About vowpalwabbit, their only use is np.exp(log_logistic(...)) which is just crazy.

[glemaitre]

Keep it with wrapping logaddexp and deprecate it?

Yes.

[lorentzenchr]

Not happy but will do.

[lorentzenchr]

As you see, that's not a 5 minutes thing. It cost me 1/2h.

[jjerphan]

But that prevented potential breakages which my hasty approving review definitely had not envisioned. 🤦

Thank you for the efforts, @lorentzenchr.

[glemaitre]

Thanks @lorentzenchr Merging.