This repository has been archived by the owner on Nov 22, 2022. It is now read-only.
-
Notifications
You must be signed in to change notification settings - Fork 800
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
facebook-github-bot
added
the
CLA Signed
Do not delete this pull request or issue due to inactivity.
label
Mar 15, 2019
kmalik22
force-pushed
the
export-D14473020
branch
from
March 15, 2019 00:51
9e8fdc6
to
7bb03a4
Compare
kmalik22
pushed a commit
to kmalik22/pytext
that referenced
this pull request
Mar 15, 2019
Summary: Pull Request resolved: facebookresearch#394 DocClassificationTask supports dense features, but exporting a DocClassification model with dense features to caffe2 doesn't work. Also, exporting any model that uses both text features (word/char) and dense features together doesn't work. This diff fixes export for dense features. There are two things that needed fixing: - export puts model inputs in a list like this: `[features, feature lengths]`. However, dense features don't go through the representation layer - so they need to be hardcoded to be at the end of model inputs (ugh). The order of features needs to be `[features, feature lengths, dense features]` - dummy_model_input for all other fields has two entries. dummy_model_input for dense_features has one entry This is a blocking feature for [on-device M-suggestions](https://fb.workplace.com/groups/323789728341681/permalink/326686971385290/). A public test for that is scheduled next week. Personalization models in M-suggestions need dense features. Differential Revision: D14473020 fbshipit-source-id: 6c97c115d4140f72d0e1a97c309d95b1c8794b31
kmalik22
force-pushed
the
export-D14473020
branch
from
March 15, 2019 17:57
7bb03a4
to
e83afb7
Compare
kmalik22
pushed a commit
to kmalik22/pytext
that referenced
this pull request
Mar 15, 2019
Summary: Pull Request resolved: facebookresearch#394 DocClassificationTask supports dense features, but exporting a DocClassification model with dense features to caffe2 doesn't work. Also, exporting any model that uses both text features (word/char) and dense features together doesn't work. This diff fixes export for dense features. There are two things that needed fixing: - export puts model inputs in a list like this: `[features, feature lengths]`. However, dense features don't go through the representation layer - so they need to be hardcoded to be at the end of model inputs (ugh). The order of features needs to be `[features, feature lengths, dense features]` - dummy_model_input for all other fields has two entries. dummy_model_input for dense_features has one entry This is a blocking feature for [on-device M-suggestions](https://fb.workplace.com/groups/323789728341681/permalink/326686971385290/). A public test for that is scheduled next week. Personalization models in M-suggestions need dense features. Differential Revision: D14473020 fbshipit-source-id: afe1034f8152a7a07423902533026d83d9f57620
kmalik22
force-pushed
the
export-D14473020
branch
from
March 20, 2019 19:15
e83afb7
to
9450635
Compare
kmalik22
pushed a commit
to kmalik22/pytext
that referenced
this pull request
Mar 20, 2019
…bookresearch#394) Summary: Pull Request resolved: facebookresearch#394 DocClassificationTask supports dense features, but exporting a DocClassification model with dense features to caffe2 doesn't work. Also, exporting any model that uses both text features (word/char) and dense features together doesn't work. This diff fixes export for dense features. There are two things that needed fixing: - exporter puts model inputs in a list like this: `[features, feature lengths]`. However, dense features don't go through the representation layer - so they need to be hardcoded to be at the end of model inputs (ugh). The order of features needs to be `[features, feature lengths, dense features]` - dummy_model_input for all other fields has two entries. dummy_model_input for dense_features has one entry In the long run, exporter should be tightly coupled with the model itself - we should enforce that model inputs have the same order while training and exporting. But that requires a lot of refactoring. This is a blocking feature for [on-device M-suggestions](https://fb.workplace.com/groups/323789728341681/permalink/326686971385290/). A public test for that is scheduled next week. Personalization models in M-suggestions need dense features. Reviewed By: snisarg, gardenia22 Differential Revision: D14473020 fbshipit-source-id: b5ac168b23562a2d7eed0c97cca6a1dd3667750a
…bookresearch#394) Summary: Pull Request resolved: facebookresearch#394 DocClassificationTask supports dense features, but exporting a DocClassification model with dense features to caffe2 doesn't work. Also, exporting any model that uses both text features (word/char) and dense features together doesn't work. This diff fixes export for dense features. There are two things that needed fixing: - exporter puts model inputs in a list like this: `[features, feature lengths]`. However, dense features don't go through the representation layer - so they need to be hardcoded to be at the end of model inputs (ugh). The order of features needs to be `[features, feature lengths, dense features]` - dummy_model_input for all other fields has two entries. dummy_model_input for dense_features has one entry In the long run, exporter should be tightly coupled with the model itself - we should enforce that model inputs have the same order while training and exporting. But that requires a lot of refactoring. This is a blocking feature for [on-device M-suggestions](https://fb.workplace.com/groups/323789728341681/permalink/326686971385290/). A public test for that is scheduled next week. Personalization models in M-suggestions need dense features. Reviewed By: snisarg, gardenia22 Differential Revision: D14473020 fbshipit-source-id: 8d26ff3ffd28087d6172d66f39d1d3f5f8a0ee9b
kmalik22
force-pushed
the
export-D14473020
branch
from
March 20, 2019 19:23
9450635
to
db704e5
Compare
This pull request has been merged in 1465244. |
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
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.
Summary:
DocClassificationTask supports dense features, but exporting a DocClassification model with dense features to caffe2 doesn't work. Also, exporting any model that uses both text features (word/char) and dense features together doesn't work. This diff fixes export for dense features.
There are two things that needed fixing:
[features, feature lengths]
. However, dense features don't go through the representation layer - so they need to be hardcoded to be at the end of model inputs (ugh). The order of features needs to be[features, feature lengths, dense features]
This is a blocking feature for on-device M-suggestions. A public test for that is scheduled next week.
Personalization models in M-suggestions need dense features.
Differential Revision: D14473020