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

LdaMulticore parameter alpha: 'auto' not supported but documentation says it is #2223

Closed
johann-petrak opened this issue Oct 11, 2018 · 5 comments
Labels
bug Issue described a bug difficulty easy Easy issue: required small fix documentation Current issue related to documentation good first issue Issue for new contributors (not required gensim understanding + very simple) Hacktoberfest Issues marked for hacktoberfest

Comments

@johann-petrak
Copy link
Contributor

When using the value 'auto' for parameter "alpha" for LdaMulticore, the following exception is raised:

NotImplementedError: auto-tuning alpha not implemented in multicore LDA; use plain LdaModel.

(ldamulticore.py, line 146)

However the documentation string of the __init__ states that "auto" is supported.

This may be a copy/paste error, but is there any chance to see auto get supported for the Multicore version as well?

Versions

Linux-4.15.0-35-generic-x86_64-with-debian-buster-sid
Python 3.6.3 |Anaconda custom (64-bit)| (default, Oct 13 2017, 12:02:49) 
[GCC 7.2.0]
NumPy 1.14.2
SciPy 1.0.0
gensim 3.4.0
FAST_VERSION 1

However, the exception code is still present in the latest master as well, I think.

@menshikh-iv
Copy link
Contributor

Hello @johann-petrak, thanks for the report: that's really copy-paste issue.

is there any chance to see auto get supported for the Multicore version as well

Most likely no, sorry

@menshikh-iv menshikh-iv added bug Issue described a bug documentation Current issue related to documentation difficulty easy Easy issue: required small fix good first issue Issue for new contributors (not required gensim understanding + very simple) Hacktoberfest Issues marked for hacktoberfest labels Oct 12, 2018
@johann-petrak
Copy link
Contributor Author

Hello @johann-petrak, thanks for the report: that's really copy-paste issue.

is there any chance to see auto get supported for the Multicore version as well

Most likely no, sorry

Could you explain why? The Mallet implementation does this while being mult-threaded (I understand how multi-processing is different from multi-threaded). I would just like to understand what the limitation is.
Estimating alpha from the data is something one often wants to do, but having to resort to a single thread is a high cost for that.

@johann-petrak
Copy link
Contributor Author

johann-petrak commented Oct 12, 2018

Created pull request #2225 for this

@johann-petrak
Copy link
Contributor Author

Also just noticed that 'auto' is not available for LdaModel if it is used in distributed mode, this should also get added to the documentation.

@menshikh-iv
Copy link
Contributor

@johann-petrak I think better to ask @piskvorky or @ziky90 about it.

menshikh-iv pushed a commit that referenced this issue Oct 15, 2018
* Remove the 'auto' parameter value.
The 'auto' parameter value for parameter alpha is not supported
in LdaMulticore, so remove it from the documentation.

* Note that alpha='auto' not available in distributed mode.
Fixes #2223

* remove trailing whitespace
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Issue described a bug difficulty easy Easy issue: required small fix documentation Current issue related to documentation good first issue Issue for new contributors (not required gensim understanding + very simple) Hacktoberfest Issues marked for hacktoberfest
Projects
None yet
Development

No branches or pull requests

2 participants