Skip to content
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.

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

Fix scipy logsumexp deprecation warning #1703

Merged

Conversation

dreamgonfly
Copy link
Contributor

Problem
I noticed that when training LDA model with the latest version of scipy I get a deprecation warning.

>>> ldamodel = LdaModel(corpus, num_topics=4, id2word=dictionary, passes=20, alpha=auto’, eval_every=5)
gensim/models/ldamodel.py:776: DeprecationWarning: `logsumexp` is deprecated!

Reason
I dug into the problem and found that scipy 1.0.0 has been released recently.
https://scipy.github.io/devdocs/release.1.0.0.html

In the release they deprecated a number of functions from scipy.misc, and logsumexp is one of them. The link below is the corresponding commit.
scipy/scipy#7246

Solution
I fixed the warning by making logsumexp to be imported from scipy.special, as they recommend.
Also I checked that, except in LdaModel, logsumexp is not used in any other places in gensim .

Copy link
Contributor

@menshikh-iv menshikh-iv left a comment

Choose a reason for hiding this comment

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

Please fix problem with imports

@@ -48,7 +48,7 @@
from gensim.models.callbacks import Callback

# log(sum(exp(x))) that tries to avoid overflow
from scipy.misc import logsumexp
from scipy.special import logsumexp
Copy link
Contributor

Choose a reason for hiding this comment

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

For 1.0.0 it works, but as you see in https://github.com/RaRe-Technologies/gensim/blob/develop/setup.py#L290 our minimal version is 0.18.1, in this version, this import will be failed.

Import must work for any scipy >=0.18.1

@dreamgonfly
Copy link
Contributor Author

dreamgonfly commented Nov 9, 2017

Thanks for the comment! I fixed the problem with imports.

@menshikh-iv
Copy link
Contributor

Thank you @dreamgonfly, congratz with first contribution 🥇

@menshikh-iv menshikh-iv merged commit 89834a5 into piskvorky:develop Nov 9, 2017
@uduwage
Copy link

uduwage commented Nov 23, 2017

This fix probably not available on the latest pip or conda installation? or is it?

@menshikh-iv
Copy link
Contributor

menshikh-iv commented Nov 24, 2017

@uduwage fix will be available in next release (~first week of December). Alternative way - you can install develop branch from git.

VaiyeBe pushed a commit to VaiyeBe/gensim that referenced this pull request Nov 26, 2017
* Fix scipy logsumexp deprecation warning

* Fix import error
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.

3 participants