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 infinite diff in LdaModel.do_mstep #2344

Merged
merged 2 commits into from
Jan 28, 2019
Merged

Conversation

horpto
Copy link
Contributor

@horpto horpto commented Jan 20, 2019

Fix partially #416
Fix partially #2051

This PR solves infinite diff in output. It works for me. This caused by a big negative values in self.state.sstate and narrow dtype of values. I try to solve only this bug. self.expElogbeta can contain zeros anyway. That can be solved by pointing a more wide dtype, for example np.float64. I don't increase dtype of self.expElogbeta as I know there is a drawback between memory consumption/file sizes/sent bytes and precision. User should be smart enough and choose wisely himself.

gensim/models/ldamodel.py Show resolved Hide resolved
gensim/models/ldamodel.py Show resolved Hide resolved
gensim/models/ldamodel.py Show resolved Hide resolved
@menshikh-iv menshikh-iv changed the title Fix #416, #2051: Infinite diff in LdaModel.do_mstep Fix infinite diff in LdaModel.do_mstep Jan 23, 2019
@menshikh-iv
Copy link
Contributor

menshikh-iv commented Jan 23, 2019

ping @johann-petrak @stevemarin @TC-Rudel

Guys, this change should fix diff=inf/nan issue (from #416), can anyone check is current PR fixed your problems:

  1. clone horpto fork: git clone git@github.com:horpto/gensim.git && cd gensim
  2. install gensim from inf-mstep branch: git checkout inf-mstep && pip install -e . && python setup.py build_ext --inplace
  3. check that diff=nan doesn't appear for you any more?

@menshikh-iv
Copy link
Contributor

Thanks @horpto

@menshikh-iv menshikh-iv merged commit 179a2c1 into piskvorky:develop Jan 28, 2019
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.

2 participants