Skip to content

Commit

Permalink
Fix malletmodel2ldamodel conversion (#2288)
Browse files Browse the repository at this point in the history
* Fixes #2069: wrong malletmodel2ldamodel

`malletmodel2ldamodel` sets up expElogbeta attribute
but LdaModel.show_topics uses inner not dirichleted state instead.
And moreover LdaState and LdaModel were not synced.

* add test

* fix linter

* replace sklearn with gensim + use larger dataset & num topics (for more strict check)

* remove sklearn import
  • Loading branch information
horpto authored and menshikh-iv committed Jan 8, 2019
1 parent e8933df commit ebc7971
Show file tree
Hide file tree
Showing 2 changed files with 24 additions and 3 deletions.
6 changes: 4 additions & 2 deletions gensim/models/wrappers/ldamallet.py
Original file line number Diff line number Diff line change
Expand Up @@ -588,9 +588,11 @@ def malletmodel2ldamodel(mallet_model, gamma_threshold=0.001, iterations=50):
"""
model_gensim = LdaModel(
id2word=mallet_model.id2word, num_topics=mallet_model.num_topics,
alpha=mallet_model.alpha, iterations=iterations,
alpha=mallet_model.alpha, eta=0,
iterations=iterations,
gamma_threshold=gamma_threshold,
dtype=numpy.float64 # don't loose precision when converting from MALLET
)
model_gensim.expElogbeta[:] = mallet_model.wordtopics
model_gensim.state.sstats[...] = mallet_model.wordtopics
model_gensim.sync_state()
return model_gensim
21 changes: 20 additions & 1 deletion gensim/test/test_ldamallet_wrapper.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,9 +19,11 @@
from gensim.corpora import mmcorpus, Dictionary
from gensim.models.wrappers import ldamallet
from gensim import matutils
from gensim.utils import simple_preprocess
from gensim.models import ldamodel
from gensim.test import basetmtests
from gensim.test.utils import datapath, get_tmpfile, common_texts
import gensim.downloader as api

dictionary = Dictionary(common_texts)
corpus = [dictionary.doc2bow(text) for text in common_texts]
Expand Down Expand Up @@ -90,6 +92,10 @@ def testMallet2Model(self):

tm1 = ldamallet.LdaMallet(self.mallet_path, corpus=corpus, num_topics=2, id2word=dictionary)
tm2 = ldamallet.malletmodel2ldamodel(tm1)

# set num_topics=-1 to exclude random influence
self.assertEqual(tm1.show_topics(-1, 10), tm2.show_topics(-1, 10))

for document in corpus:
element1_1, element1_2 = tm1[document][0]
element2_1, element2_2 = tm2[document][0]
Expand All @@ -101,7 +107,20 @@ def testMallet2Model(self):
self.assertAlmostEqual(element1_2, element2_2, 1)
logging.debug('%d %d', element1_1, element2_1)
logging.debug('%d %d', element1_2, element2_2)
logging.debug('%d %d', tm1[document][1], tm2[document][1])
logging.debug('%s %s', tm1[document][1], tm2[document][1])

def testMallet2ModelOn20NewsGroups(self):
corpus = [simple_preprocess(doc["data"]) for doc in api.load("20-newsgroups")]
dictionary = Dictionary(corpus)

corpus = [dictionary.doc2bow(text) for text in corpus]

lda_mallet_model = ldamallet.LdaMallet(
self.mallet_path, corpus=corpus,
num_topics=20, id2word=dictionary, iterations=500)

lda_gensim_model = ldamallet.malletmodel2ldamodel(lda_mallet_model, iterations=1000)
self.assertEqual(lda_mallet_model.show_topics(20, 50), lda_gensim_model.show_topics(20, 50))

def testPersistence(self):
if not self.mallet_path:
Expand Down

0 comments on commit ebc7971

Please sign in to comment.