-
Notifications
You must be signed in to change notification settings - Fork 655
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
Delete DataManager #691
Delete DataManager #691
Conversation
Also done: - Create a forwardInternal with labels and the implementations in the higher level blocks SequentialBlock and ParallelBlock - Removed duplicate testSeqToSeq test Change-Id: I16f5343886a62538f3a247df062d28a765805cb8
LGTM when I see the title. Look into the code now |
.addSingleton( | ||
a -> { | ||
try { | ||
return embedding.embedText(a); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The problem to put it here is: I would like to freeze this block and not use for training. If remove DataManager, I will not be able to freeze the block
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This would be frozen because the ModelZooTextEmbedding freezes (it uses a separate predictor). In a different case, you should be able to just call embedding.freeze() to freeze just the embedding.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you verify if PyTorch engine will work in this case? I am not sure if the training work are correctly handled
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There is no word embedding for pt in the model zoo so I can't test it. But, is there a reason that PT may not work?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It should work by default. Just concerned if the NDArray inside will be impacted. In the DistiledBert training for Amazon Dataset, we throw the embedding layer into a lambda function to avoid parameter collections
Also done:
blocks SequentialBlock and ParallelBlock