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

inline logsumexp() #1744

Closed
wants to merge 3 commits into from
Closed

inline logsumexp() #1744

wants to merge 3 commits into from

Conversation

arlenk
Copy link
Contributor

@arlenk arlenk commented Nov 29, 2017

scipy's logsumexp performs some robustness checks that are likely not necessary for ldamodel. Inlining a barebones version of logsumexp speeds up ldamodel by 20-40%

float64 added 3 commits November 28, 2017 21:54
logsumexp accounts for 50% of the run time of ldamodel.  Much
of this time is spent by "robustness" checks performed by
scipy's logsumexp (eg, _asarray_validated, checks for NaNs, etc.).

Removing these checks greatly improves the overall performance
of ldamodel.  Eg, run time when fitting a lda model on the
endron dataset (from UCI) decreases from 20-40%.
logsumexp accounts for 50% of the run time of ldamodel.  Much
of this time is spent by "robustness" checks performed by
scipy's logsumexp (eg, _asarray_validated, checks for NaNs, etc.).

Removing these checks greatly improves the overall performance
of ldamodel.  Eg, run time when fitting a lda model on the
enron dataset (from UCI) decreases from 20-40%.
@arlenk arlenk changed the title Performance logsumexp inline logsumexp Nov 29, 2017
@arlenk arlenk changed the title inline logsumexp inline logsumexp() Nov 29, 2017
@arlenk arlenk closed this Nov 29, 2017
@piskvorky
Copy link
Owner

Oh, wow! That's interesting -- why is this closed?

@menshikh-iv
Copy link
Contributor

@piskvorky this is re-created as #1745

@piskvorky
Copy link
Owner

piskvorky commented Nov 29, 2017

Ah, ok. Thanks.

Please cross-link such PRs, otherwise it's not obvious to people what's happening (including myself).

@arlenk
Copy link
Contributor Author

arlenk commented Nov 30, 2017

apologies, still getting the hang of the github interface and created this pull request prematurely.

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.

3 participants