Skip to content

Add PackSegmentsFix operator in caffe2 #9048

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.

Sign up for GitHub

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

Jump to bottom
Closed
nateanl wants to merge 1 commit into from
Closed

Add PackSegmentsFix operator in caffe2 #9048

nateanl wants to merge 1 commit into from

Conversation

@nateanl
Copy link
Member

@nateanl nateanl commented Jun 29, 2018

Summary:
This operator is similar to PackSegmentsOp but with an additional option dim.
Input: a UNKNOWN_DIM * D tensor and a lengths tensor. The shape of lengths is UNKNOWN_DIM
The output will be UNKNOWN_DIM * dim * D

Differential Revision: D8702782

@nateanl nateanl force-pushed the export-D8702782 branch from 56171b9 to 9e63645 Compare July 2, 2018 16:04
@nateanl nateanl force-pushed the export-D8702782 branch from 9e63645 to 2d4491c Compare July 2, 2018 18:19
facebook-github-bot
facebook-github-bot reviewed Jul 2, 2018
View reviewed changes
Copy link
Contributor

@facebook-github-bot facebook-github-bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@nateanl has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

@bddppq bddppq self-requested a review July 5, 2018 17:16
bddppq
bddppq reviewed Jul 5, 2018
View reviewed changes
Copy link
Contributor

@bddppq bddppq left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IIUC what you wanna achieve is to use an argument (currently named dim, I suggest to rename it to max_length) to override the max_length dimension in the output? If so you can simply add this argument to the existing PackSegmentsOp operator.

@nateanl nateanl force-pushed the export-D8702782 branch from 2d4491c to 80df043 Compare July 5, 2018 20:48
bddppq
bddppq reviewed Jul 5, 2018
View reviewed changes
caffe2/operators/pack_segments.cc Outdated

This comment was marked as off-topic.

Sign in to view

This comment was marked as off-topic.

Sign in to view

caffe2/operators/pack_segments.h Outdated

This comment was marked as off-topic.

Sign in to view

caffe2/operators/pack_segments.cu Outdated

This comment was marked as off-topic.

Sign in to view

This comment was marked as off-topic.

Sign in to view

bddppq
bddppq reviewed Jul 5, 2018
View reviewed changes
caffe2/python/operator_test/pack_ops_test.py Outdated

This comment was marked as off-topic.

Sign in to view

This comment was marked as off-topic.

Sign in to view

This comment was marked as off-topic.

Sign in to view

@nateanl nateanl force-pushed the export-D8702782 branch from 80df043 to ba2de01 Compare July 5, 2018 21:27
@bddppq
Copy link
Contributor

bddppq commented Jul 5, 2018

@pytorchbot retest this please

bddppq
bddppq approved these changes Jul 5, 2018
View reviewed changes
Copy link
Contributor

@bddppq bddppq left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

caffe2/python/operator_test/pack_ops_test.py Outdated

This comment was marked as off-topic.

Sign in to view

bddppq
bddppq reviewed Jul 5, 2018
View reviewed changes
caffe2/python/operator_test/pack_ops_test.py Outdated

This comment was marked as off-topic.

Sign in to view

This comment was marked as off-topic.

Sign in to view

@nateanl @facebook-github-bot
Modify the original PackSegments operator by adding "max_length" argu…
c6fc7e3 
…ment (pytorch#9048)

Summary:
Closes pytorch#9048

max_length argument helps fix the shape of the output to be N * max_length * D, where N is the batch_size, D is the feature_dim.

Differential Revision: D8702782

fbshipit-source-id: 599dd40821a5ca119acb4d61162f194f222cfbfb
@nateanl nateanl force-pushed the export-D8702782 branch from ba2de01 to c6fc7e3 Compare July 6, 2018 13:57
goodlux pushed a commit to goodlux/pytorch that referenced this pull request Aug 15, 2018
@nateanl
Modify the original PackSegments operator by adding "max_length" argu…
a70ab52 
…ment (pytorch#9048)

Summary:
Closes pytorch#9048

max_length argument helps fix the shape of the output to be N * max_length * D, where N is the batch_size, D is the feature_dim.

Reviewed By: bddppq

Differential Revision: D8702782

fbshipit-source-id: e30555608fee1c4a61cc95922f4a71c7f54903af
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@facebook-github-bot facebook-github-bot facebook-github-bot left review comments

+1 more reviewer

@bddppq bddppq bddppq approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@nateanl @bddppq @facebook-github-bot