Skip to content
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

Fix Transpose operator when data shape has irrelevant dimensions (i.e. dimension of size 1) #1244

Merged
merged 2 commits into from
Sep 12, 2019

Conversation

jantonguirao
Copy link
Contributor

@jantonguirao jantonguirao commented Sep 10, 2019

Signed-off-by: Joaquin Anton janton@nvidia.com

Why we need this PR?

Fixes a bug in Transpose operator when the shape of the input contains a 1 (e.g. [800, 600, 1], [1, 500, 400, 3])

What happened in this PR?

Add changes to detect a special case and remove any dimension that is equal to 1 before invoking cuTT. The permutation order is also adjusted to reflect the flattened shape.

JIRA TASK: N/A

@jantonguirao jantonguirao force-pushed the fix_transpose_op branch 3 times, most recently from 6cce67d to f229c16 Compare September 10, 2019 15:46
@jantonguirao jantonguirao changed the title Fix Transpose operator when last dimension of the input is 1 (e.g. C==1) Fix Transpose operator when data shape has irrelevant dimensions (i.e. dimension of size 1) Sep 10, 2019
@jantonguirao
Copy link
Contributor Author

!build

@dali-automaton
Copy link
Collaborator

CI MESSAGE: [893930]: BUILD STARTED

@dali-automaton
Copy link
Collaborator

CI MESSAGE: [893930]: BUILD FAILED

Copy link
Contributor

@Kh4L Kh4L left a comment

Choose a reason for hiding this comment

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

Please factor out the logic and add it also to the non-batched kernel

@jantonguirao
Copy link
Contributor Author

!build

@dali-automaton
Copy link
Collaborator

CI MESSAGE: [894085]: BUILD STARTED

@dali-automaton
Copy link
Collaborator

CI MESSAGE: [894085]: BUILD PASSED

@jantonguirao
Copy link
Contributor Author

!build

@dali-automaton
Copy link
Collaborator

CI MESSAGE: [895257]: BUILD STARTED

…400, 1), (1, 200, 400, 3))

Signed-off-by: Joaquin Anton <janton@nvidia.com>
@jantonguirao
Copy link
Contributor Author

!build

@dali-automaton
Copy link
Collaborator

CI MESSAGE: [895306]: BUILD STARTED

@dali-automaton
Copy link
Collaborator

CI MESSAGE: [895306]: BUILD PASSED

Copy link
Contributor

@Kh4L Kh4L left a comment

Choose a reason for hiding this comment

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

LGTM 👍

for (int i = 0; i < rank; ++i) {
new_dims[i] = dims[rank - 1 - i];
new_perm[i] = rank - 1 - perm[rank - 1 - i];
dims[i] = dims_copy[rank - 1 - i];
Copy link
Contributor

Choose a reason for hiding this comment

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

This can be done in place.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

will do

@jantonguirao
Copy link
Contributor Author

!build

@dali-automaton
Copy link
Collaborator

CI MESSAGE: [895771]: BUILD STARTED

}
return {std::move(new_dims), std::move(new_perm)};

std::unordered_map<int, int> idx_map;
Copy link
Contributor

@mzient mzient Sep 11, 2019

Choose a reason for hiding this comment

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

Possible improvement: this could be just a vector.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

Signed-off-by: Joaquin Anton <janton@nvidia.com>
@jantonguirao
Copy link
Contributor Author

!build

@dali-automaton
Copy link
Collaborator

CI MESSAGE: [895806]: BUILD STARTED

@dali-automaton
Copy link
Collaborator

CI MESSAGE: [895806]: BUILD PASSED

@jantonguirao jantonguirao merged commit 2cacada into NVIDIA:master Sep 12, 2019
00liujj pushed a commit to 00liujj/DALI that referenced this pull request Oct 10, 2019
…IA#1244)

* Fix Transpose operator to accept data shapes including 1 (e.g. (200, 400, 1), (1, 200, 400, 3))

Signed-off-by: Joaquin Anton <janton@nvidia.com>
Signed-off-by: Jianjun Liu <00liujj@163.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants