Glove model - Draft Pull Request #189
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
MaxHalford
reviewed
Feb 4, 2020
creme/decomposition/glove.py
Outdated
| __all__ = ['Glove'] | ||
|
|
||
|
|
||
| class Glove(base.Transformer, vectorize.VectorizerMixin): |
creme/decomposition/glove.py
Outdated
| self.t = t | ||
| self.window_size = window_size | ||
| self.cooccurrences = defaultdict(float) | ||
| self.weights = defaultdict( |
There was a problem hiding this comment.
Yeah you need to use a function or functools.partial. The issue with lambda is that it means the class can't be pickled, which is important :)
creme/decomposition/glove.py
Outdated
| """ | ||
| # Extracts words of the document as a list of words: | ||
| tokens = self.tokenizer(self.preprocess(self._get_text(tokens))) | ||
| return {token: self.weights[token] for token in tokens} |
There was a problem hiding this comment.
I think we should return the average of the works embeddings. I'm not sure what most people do but I guess averaging is the way to go.
creme/decomposition/glove.py
Outdated
| path (str): path of the weights of the pretrained model. | ||
|
|
||
| """ | ||
| file = open(path,'r') |
There was a problem hiding this comment.
What format is this function assuming the file is in? If the format has a name could you please specify it in the comment? :)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Hi☺️ ,
I know that everyone is waiting for glove but this work still in progress. This draft pull request is linked to the issue #148.
Remarks on the model:
For reasons of simplicity and transparency in the model, I did not convert the tokens into an index. I used the words directly as an identifier for dictionaries.
It's possible to initialize the weights of the words with pre-trained ones thanks to the method
load.I have few questions:
How to avoid Lambda func in
collections.defaultdict? Is this really a problem here?Do you think that the output of the model
transform_one()`` and ``fit_transform_one()is the right one? I mean, a dictionary dictionary:{'weather': {0: 1.281993..., 1: 1.148658...}}Remarks on the code:
I have to benchmark the model to make sure that the results are good and that I have not made any mistakes. I will find computer resources to run it on wikipedia as the authors did.
Feel free to review my code and give me feedback.
Raphaël