This repository has been archived by the owner on Nov 17, 2023. It is now read-only.
Remove inplace support for ToTensor operator #14083
Merged
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
|
@mxnet-label-bot add [pr-awaiting-review, Operator] |
sandeep-krishnamurthy
changed the title
stu1130
approved these changes
Feb 7, 2019
zhreshold
reviewed
Feb 7, 2019
| struct totensor_forward { | ||
| template<typename DType> | ||
| MSHADOW_XINLINE static void Map(uint32_t c, float* out_data, const DType* in_data, | ||
| const int length, const int channel, const int step, | ||
| const float normalize_factor = 255.0f) { | ||
| #pragma omp parallel for | ||
| for (int i = 0; i < length; ++i) { | ||
| KERNEL_ASSIGN(out_data[step + c*length + i], req, | ||
| (in_data[step + i*channel + c]) / normalize_factor); | ||
| out_data[step + c*length + i] = |
There was a problem hiding this comment.
you should take care of the req rather than disable the check, this is dangerous for any behavior our of your usecase right now
There was a problem hiding this comment.
Ah. You are right. Thanks. I removed the support for in place during operator registration.
Pasting below your notes for any future reference regarding this PR. Thanks.
think about the case from HWC to NCHW
values get overwrite and can never be recovered
to support inplace, you have to have a temp buffer
.set_attr<nnvm::FInplaceOption>("FInplaceOption",
[](const NodeAttrs& attrs) {
return std::vector<std::pair<int, int> >{{0, 0}};
})
disable it
if you don’t want to implement inplace op
sandeep-krishnamurthy
force-pushed
the
fix_totensor
branch
from
14bedee
to
60fead4
Compare
sandeep-krishnamurthy
changed the title
sandeep-krishnamurthy
changed the title
sandeep-krishnamurthy
changed the title
zhreshold
approved these changes
Feb 7, 2019
stephenrawls
pushed a commit
to stephenrawls/incubator-mxnet
that referenced
this pull request
Feb 16, 2019
* Remove stale check for op req type * Do not register to tensor operator with in place option.
vdantu
pushed a commit
to vdantu/incubator-mxnet
that referenced
this pull request
Mar 31, 2019
* Remove stale check for op req type * Do not register to tensor operator with in place option.
haohuanw
pushed a commit
to haohuanw/incubator-mxnet
that referenced
this pull request
Jun 23, 2019
* Remove stale check for op req type * Do not register to tensor operator with in place option.
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.
Description
Remove inplace support for ToTensor operator.
Checklist
Essentials
Please feel free to remove inapplicable items for your PR.
Comments
@stu1130