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

Reshape op #1327

Merged
merged 6 commits into from
Oct 3, 2019
Merged

Reshape op #1327

merged 6 commits into from
Oct 3, 2019

Conversation

mzient
Copy link
Contributor

@mzient mzient commented Oct 1, 2019

Why we need this PR?

It adds new feature requested by the community:

  • Reshape operator with ability to reinterpret input tensor(s) without copying

What happened in this PR?

  • Explain solution of the problem, new feature added.
    • Added the operator which uses ShareData to pass same memory to the output, while reinterpreting the shape and/or layout
  • What was changed, added, removed?
    • Added the operator and tests
    • Removed excessive checks for buffer size
  • What is most important part that reviewers should focus on?
    • reshape.cc
  • Was this PR tested? How?
    • python test
  • Were docs and examples updated, if necessary?
    • added auto-generated docs for the operator

JIRA TASK: [DALI-XXXX]

Signed-off-by: Michal Zientkiewicz <michalz@nvidia.com>
Add tests.

Signed-off-by: Michal Zientkiewicz <michalz@nvidia.com>
Signed-off-by: Michal Zientkiewicz <michalz@nvidia.com>
@mzient
Copy link
Contributor Author

mzient commented Oct 1, 2019

!build

@dali-automaton
Copy link
Collaborator

CI MESSAGE: [925438]: BUILD STARTED

@dali-automaton
Copy link
Collaborator

CI MESSAGE: [925438]: BUILD PASSED

dali/pipeline/operators/util/reshape.cc Show resolved Hide resolved
dali/pipeline/operators/util/reshape.cc Outdated Show resolved Hide resolved
dali/pipeline/operators/util/reshape.cc Show resolved Hide resolved
dali/pipeline/operators/util/reshape.cc Outdated Show resolved Hide resolved
dali/pipeline/operators/util/reshape.cc Outdated Show resolved Hide resolved
dali/pipeline/operators/util/reshape.cc Outdated Show resolved Hide resolved
dali/pipeline/operators/util/reshape.h Outdated Show resolved Hide resolved
dali/pipeline/operators/util/reshape.h Show resolved Hide resolved
dali/test/python/test_operator_reshape.py Outdated Show resolved Hide resolved
dali/test/python/test_operator_reshape.py Show resolved Hide resolved
Add comparison to TensorLayout in pybind.
Expose TensorMeta from TensorList and TensorVector.
Propagate TensorMeta when sharing tensor data from TensorList.

Signed-off-by: Michal Zientkiewicz <michalz@nvidia.com>
Signed-off-by: Michal Zientkiewicz <michalz@nvidia.com>
@mzient
Copy link
Contributor Author

mzient commented Oct 2, 2019

!build

@dali-automaton
Copy link
Collaborator

CI MESSAGE: [926752]: BUILD STARTED

Copy link
Contributor

@jantonguirao jantonguirao left a comment

Choose a reason for hiding this comment

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

Looks good, except for the couple of comments that are not yet resolved

Signed-off-by: Michal Zientkiewicz <michalz@nvidia.com>
@mzient
Copy link
Contributor Author

mzient commented Oct 2, 2019

!build

@dali-automaton
Copy link
Collaborator

CI MESSAGE: [926780]: BUILD STARTED

@dali-automaton
Copy link
Collaborator

CI MESSAGE: [926752]: BUILD PASSED

@dali-automaton
Copy link
Collaborator

CI MESSAGE: [926780]: BUILD PASSED

@mzient mzient merged commit 42dadc8 into NVIDIA:master Oct 3, 2019
00liujj pushed a commit to 00liujj/DALI that referenced this pull request Oct 10, 2019
* Reshape operator which reinterprets data without copying.
* Fix output layout in Reshape.
* Expose Tensor and TensorList layout in Python
* Add comparison to TensorLayout in pybind.
* Expose TensorMeta from TensorList and TensorVector.
* Propagate TensorMeta when sharing tensor data from TensorList.
* Add ShareData to TensorVector.

Signed-off-by: Michal Zientkiewicz <michalz@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