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

Utils and Matutils changes #1062

Merged
merged 3 commits into from
Dec 28, 2016
Merged

Utils and Matutils changes #1062

merged 3 commits into from
Dec 28, 2016

Conversation

bhargavvader
Copy link
Contributor

@bhargavvader bhargavvader commented Dec 27, 2016

  1. Moved dirichlet_expectation to matutils as the same code was being used in both ldamodel and hdpmodel.
  2. Changed all numpy in utils and matutils to np, keeping it uniform across the package.
  3. Fixed hanging indent.
  4. Changed import conventions in hdpmodel to make it consistent with ldamodel

@bhargavvader bhargavvader mentioned this pull request Dec 27, 2016
@@ -608,7 +607,7 @@ def is_corpus(obj):
doc1 = next(iter(obj)) # empty corpus is resolved to False here
if len(doc1) == 0: # sparse documents must have a __len__ function (list, tuple...)
return True, obj # the first document is empty=>assume this is a corpus
id1, val1 = next(iter(doc1)) # if obj is a numpy array, it resolves to False here
id1, val1 = next(iter(doc1)) # if obj is a np array, it resolves to False here
Copy link
Owner

Choose a reason for hiding this comment

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

This replacement (and the one above) look unnecessary, the original was clearer.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Cool, will fix it

@@ -74,15 +64,15 @@ def expect_log_sticks(sticks):

def lda_e_step(doc_word_ids, doc_word_counts, alpha, beta, max_iter=100):
gamma = np.ones(len(alpha))
expElogtheta = np.exp(dirichlet_expectation(gamma))
expElogtheta = np.exp(matutils.dirichlet_expectation(gamma))
Copy link
Contributor

@tmylk tmylk Dec 28, 2016

Choose a reason for hiding this comment

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

better to add from matutils import dirichlet_expectation to avoid this change

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Okay, changed

@tmylk tmylk merged commit 49ede4d into piskvorky:develop Dec 28, 2016
@tmylk
Copy link
Contributor

tmylk commented Dec 28, 2016

Thanks for the improvement!

@@ -829,15 +828,15 @@ def __init__(self, q, corpus, chunksize, maxsize, as_numpy):

def run(self):
if self.as_numpy:
import numpy # don't clutter the global namespace with a dependency on numpy
import np # don't clutter the global namespace with a dependency on numpy
Copy link
Owner

Choose a reason for hiding this comment

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

Looks like the global module namespace already has numpy, so this line is unnecessary.

@@ -801,13 +800,13 @@ def chunkize_serial(iterable, chunksize, as_numpy=False):
[[0, 1, 2], [3, 4, 5], [6, 7, 8], [9]]

"""
import numpy
import numpy as np
Copy link
Owner

Choose a reason for hiding this comment

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

Already imported at module level, this line no longer unnecessary.

jayantj pushed a commit to jayantj/gensim that referenced this pull request Jan 4, 2017
* Utils, Matutils changes

* Changed comments

* changed matutils import
@bhargavvader bhargavvader deleted the utils branch February 23, 2017 10:34
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.

None yet

3 participants