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

[MRG] Doc2Vec inference, notebook cleanup #2103

Merged
merged 8 commits into from
Jul 5, 2018

Conversation

gojomo
Copy link
Collaborator

@gojomo gojomo commented Jun 25, 2018

Inference improvements: default parameters to analogous model parameters

Notebook updates: test operation, correct errors, provide updated executions

@gojomo gojomo changed the title [WIP] Doc2Vec inference, notebook cleanup [ready] Doc2Vec inference, notebook cleanup Jul 4, 2018
@gojomo gojomo requested review from menshikh-iv and piskvorky July 4, 2018 01:44
@gojomo
Copy link
Collaborator Author

gojomo commented Jul 4, 2018

infer_vector() now defaults to using the analogous alpha/min_alpha/epochs from model initialization unless alternate values specified. Old steps name for epochs still supported for backward-compatibility.

doc2vec-lee slight changes. No longer uses odd 55 epochs - the #2061 fix made the inference-self-checks more stable with fewer passes.

doc2vec-IMDB full executability restored & verified. Errors in leading text corrected, other explanations tweaked. ~2h text preprocessing introduced by naive bash-to-python translation now optimized to ~1m. Fancy optimizations & parallel-model-operations that could confuse beginners removed. Re-inference evaluations moved to optional bottom.

@piskvorky piskvorky changed the title [ready] Doc2Vec inference, notebook cleanup [MRG] Doc2Vec inference, notebook cleanup Jul 5, 2018
@piskvorky
Copy link
Owner

@gojomo can you fix the CI errors, so we can merge?

@menshikh-iv menshikh-iv merged commit 875b028 into piskvorky:develop Jul 5, 2018
@gojomo
Copy link
Collaborator Author

gojomo commented Jul 5, 2018

As noted in discussion elsewhere, the CI issues were unrelated to any changes in this PR.

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