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

Bug fixes #6

Closed
wants to merge 1 commit into from
Closed

Bug fixes #6

wants to merge 1 commit into from

Conversation

avinashsai
Copy link
Contributor

#1 Use Infer_vectors:
Doc2Vec model should only be trained using training data and infer_vectors should be used for test
data.Hence modified the model to use only training data.
#2 random.shuffle()
This function shuffles the data given to doc2vec model in every epoch and hence the sentiment labels
will not map to their corresponding vectors. Hence modified get_vectors() function to map labels with
their corresponding vectors.
#3 gensim version updates:
changed epochs=d2v.iter to epochs=epochs=d2v.epochs according to latest gensim version

@ibrahimsharaf
Copy link
Owner

Hi @avinashsai, thanks for the PR!

Let's split this PR into 3 separate new PRs, that would be better for readability, reviewing and testing.

  • 1st one for updating requirements.txt file to the latest versions of needed packages, with needed syntax changes?
  • 2nd and 3rd PRs for the first 2 mentioned bugs.

Thanks again for collaborating :)

@ibrahimsharaf
Copy link
Owner

For the first PR (updating requirements), what do you think about using https://pyup.io/ for dependency updates? I think it's quite cool.

@avinashsai
Copy link
Contributor Author

Yeah, it's really good. Probably we can consider using it.

@avinashsai
Copy link
Contributor Author

@ibrahimsharaf What is the next step to proceed further?

@ibrahimsharaf
Copy link
Owner

@avinashsai the first step is to create a new PR for updating the requirements, using pyup bot, and coding the needed syntax changes.

Ayatallah added a commit to Ayatallah/doc2vec that referenced this pull request May 14, 2019
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.

2 participants