Skip to content
This repository has been archived by the owner on Nov 22, 2022. It is now read-only.

Fix model export for dense features #394

Closed
wants to merge 1 commit into from

Conversation

kmalik22
Copy link

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:

  • 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. A public test for that is scheduled next week.
Personalization models in M-suggestions need dense features.

Differential Revision: D14473020

@facebook-github-bot facebook-github-bot added the CLA Signed Do not delete this pull request or issue due to inactivity. label Mar 15, 2019
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 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 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
@facebook-github-bot
Copy link
Contributor

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.
Labels
CLA Signed Do not delete this pull request or issue due to inactivity. Merged
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants