MultiEmbed #18
Closed
MultiEmbed #18
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
adrianeboyd
reviewed
Aug 8, 2022
|
|
||
|
|
||
| def _get_examples(): | ||
| nlp = spacy.blank("en") |
There was a problem hiding this comment.
nlp should be passed in so you're using the same vocab for the examples as the model.
svlandeg
reviewed
Aug 18, 2022
Comment on lines
+7
to
+8
| from typing import Optional, Callable, List, Dict, Any | ||
| from typing import Union, Sequence, Tuple |
There was a problem hiding this comment.
Can you move the from typing imports to the first line(s) of the file?
| OutT = Ints2d | ||
|
|
||
|
|
||
| @thinc.registry.layers("remap_ids.v2") |
There was a problem hiding this comment.
Hm, it feels strange to add to the Thinc registry from within spacy-experimental. I suppose we need this for config files though. Then maybe it should be spacy_experimental.remap_ids for now?
There was a problem hiding this comment.
This whole PR should just wait until the next thinc release.
There was a problem hiding this comment.
Thinc 8.1.1 is now available, Ákos :-)
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.
Embedding component that is the deterministic version of
MultiHashEmbedi.e.: each token gets mapped to an index unless they are not in the vocabulary in which case they get mapped to a learned unknown vector.The mechanism to initialize
MultiEmbedis a bit strange. TheModelgets created first with dummyEmbedlayers. Then wheninitgets calledMultiEmbedexpects themodel.attrs["tables"]to be already set, which provides the mapping from token attributes to indices. During initialization the dummyEmbedlayers get replaced by ones that adjust their sizes to the number of symbols in thetables.A helper callback is provided in
set_attr.pythat should be placed in theinitialize.before_initsection in theconfig. It can be used to set thetablesforMultiEmbed.Currently the
token_map.pyis a script that has the structure of the usualspacy initscrips.