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.

Sign up for GitHub

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

Add random_seed to LdaMallet #2153

Merged
merged 17 commits into from
Jan 10, 2019
Merged

Add random_seed to LdaMallet #2153

Changes from 1 commit
Commits
Show all changes
17 commits
Select commit Hold shift + click to select a range
75c25ee
added random_seed parameter to ldamallet wrapper
ChrisPalmerNZ Aug 9, 2018
1bfaf48
added load for backwards compatibility
ChrisPalmerNZ Aug 10, 2018
2ca085c
fix random_seed evaluation for PEP8 best practice
ChrisPalmerNZ Aug 10, 2018
645863c
Various PEP8 compliance fixes
ChrisPalmerNZ Aug 11, 2018
d8e0f54
PEP8: Removed white space from blank lines
ChrisPalmerNZ Aug 11, 2018
53182a0
Added tests for random_seed parameter in ldamallet wrapper
ChrisPalmerNZ Sep 26, 2018
6b2d52b
test_random_seed - test all docs in corpus
ChrisPalmerNZ Oct 14, 2018
a5db1df
Added additional blank line after class declaration
ChrisPalmerNZ Oct 14, 2018
3ef309f
PEP8 formatting changes only
ChrisPalmerNZ Oct 14, 2018
a6af553
Merge remote-tracking branch 'upstream/develop' into mallet-random-seed
menshikh-iv Jan 9, 2019
53136a1
fix pep8
menshikh-iv Jan 9, 2019
0b3d7aa
use 0 as default seed (according to Mallet doc) + pin seed for infere…
menshikh-iv Jan 10, 2019
7fa5069
make seed test strict
menshikh-iv Jan 10, 2019
b001cd3
remove useless test
menshikh-iv Jan 10, 2019
f3bd927
remove not used data
menshikh-iv Jan 10, 2019
a1e73b6
fix typo
menshikh-iv Jan 10, 2019
86ede21
fill random_seed=0 in load
menshikh-iv Jan 10, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 9 additions & 1 deletion gensim/models/wrappers/ldamallet.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ class LdaMallet(utils.SaveLoad, basemodel.BaseTopicModel):

"""
def __init__(self, mallet_path, corpus=None, num_topics=100, alpha=50, id2word=None, workers=4, prefix=None,
optimize_interval=0, iterations=1000, topic_threshold=0.0):
optimize_interval=0, iterations=1000, topic_threshold=0.0, random_seed=None):
"""

Parameters
Expand All @@ -100,6 +100,8 @@ def __init__(self, mallet_path, corpus=None, num_topics=100, alpha=50, id2word=N
Number of training iterations.
topic_threshold : float, optional
Threshold of the probability above which we consider a topic.
random_seed: int, optional
Random seed to ensure consistent results, default is None
Copy link
Contributor

Choose a reason for hiding this comment

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

No need to write default value in docstring description + . at the end of sentence

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I spelled out the default so that users know that they need not enter a random_seed parameter at all.

Copy link
Contributor

Choose a reason for hiding this comment

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

Default parameters automatically showed in documentation, for this reason, no need to duplicate it in docstring.


"""
self.mallet_path = mallet_path
Expand All @@ -122,6 +124,7 @@ def __init__(self, mallet_path, corpus=None, num_topics=100, alpha=50, id2word=N
self.workers = workers
self.optimize_interval = optimize_interval
self.iterations = iterations
self.random_seed = random_seed
Copy link
Contributor

Choose a reason for hiding this comment

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

define custom load functions for old mallet models (without this option), see an example https://github.com/RaRe-Technologies/gensim/blob/develop/gensim/models/tfidfmodel.py#L348-L355

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Please check I have coded and placed this correctly... Should I include logging as in the example?

Training with random_seed - what is required there?

if corpus is not None:
self.train(corpus)

Expand Down Expand Up @@ -268,11 +271,16 @@ def train(self, corpus):
cmd = self.mallet_path + ' train-topics --input %s --num-topics %s --alpha %s --optimize-interval %s '\
'--num-threads %s --output-state %s --output-doc-topics %s --output-topic-keys %s '\
'--num-iterations %s --inferencer-filename %s --doc-topics-threshold %s'

if self.random_seed != None:
Copy link
Contributor

Choose a reason for hiding this comment

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

simply if self.random_seed enough

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Reason for None is so that random seed is not invoked unless a value is explicitly passed, which can be zero. If I test just for if self.random_seed and zero is passed, will it enter this if - a zero is a valid random_seed value.

Copy link
Contributor

Choose a reason for hiding this comment

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

so, in this case, should be if self.random_seed is not None (see best practicies)

cmd += ' --random-seed ' + str(self.random_seed)

cmd = cmd % (
self.fcorpusmallet(), self.num_topics, self.alpha, self.optimize_interval,
self.workers, self.fstate(), self.fdoctopics(), self.ftopickeys(), self.iterations,
self.finferencer(), self.topic_threshold
)

Copy link
Contributor

Choose a reason for hiding this comment

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

why this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Actually I just inserted a blank line after the cmd definition just for readability - it is not required

# NOTE "--keep-sequence-bigrams" / "--use-ngrams true" poorer results + runs out of memory
logger.info("training MALLET LDA with %s", cmd)
check_output(args=cmd, shell=True)
Expand Down