[TensorV2] Multiply strided inputs and output #2434
Conversation
djeong20
requested review from
myungjoo,
jijoongmoon,
again4you,
jaeyun-jung,
leemgs,
wooksong,
helloahn,
kparichay,
gichan-jang,
anyj0527,
zhoonit,
lhs8928,
songgot,
jihochu,
DonghakPark,
SeoHyungjun,
baek2sm,
skykongkong8,
EunjuYang and
a team
as code owners
|
📝 TAOS-CI Version: 1.5.20200925. Thank you for submitting PR #2434. Please a submit 1commit/1PR (one commit per one PR) policy to get comments quickly from reviewers. Your PR must pass all verificiation processes of cibot before starting a review process from reviewers. If you are new member to join this project, please read manuals in documentation folder and wiki page. In order to monitor a progress status of your PR in more detail, visit http://ci.nnstreamer.ai/. |
nntrainer/tensor/float_tensor.cpp
Outdated
| return ((float *)getData())[i]; | ||
| } | ||
|
|
||
| const float FloatTensor::getValue(unsigned int batch, unsigned int c, |
There was a problem hiding this comment.
b,c,h,w or batch, channel, height, width ?
There was a problem hiding this comment.
b,c,h,w seems more reasonable.
nntrainer/tensor/float_tensor.h
Outdated
| * @brief return value at specific location | ||
| * @param[in] idx location | ||
| */ | ||
| const float getValue(unsigned int i) const noexcept; |
This commit adds functionality to retrieving and modifying tensor elements in specific locations. This will provide convenience in implementing the math operation of tensor. **Self-evaluation:** 1. Build test: [X]Passed [ ]Failed [ ]Skipped 2. Run test: [X]Passed [ ]Failed [ ]Skipped Signed-off-by: Donghyeon Jeong <[email protected]>
This patch adds the implementation of multiplication operation that supports different strided inputs and output. **Self-evaluation:** 1. Build test: [X]Passed [ ]Failed [ ]Skipped 2. Run test: [X]Passed [ ]Failed [ ]Skipped Signed-off-by: Donghyeon Jeong <[email protected]>
djeong20
force-pushed
the
impl/tensor_v2/multiply_strided
branch
from
d4ab173
to
c436e3c
Compare
| * @param[in] h height location | ||
| * @param[in] w width location | ||
| * @param[in] value value to be stored | ||
| * @param[in] beta scalar to multiply output with and add |
There was a problem hiding this comment.
Just a suggestion:
How about describing params like beta something like:
x = x * beta + value for better intuition? Would it make doc more nasty?
| for (unsigned int h = 0; h < height(); ++h) { | ||
| for (unsigned int w = 0; w < width(); ++w) { | ||
| output.addValue( | ||
| b, c, h, w, getValue(b, c, h, w) * m.getValue<_FP16>(b, c, h, w), |
There was a problem hiding this comment.
do we need _FP16 in m.getValue? I thought we do not need to specify the type in TensorV2 level.
There was a problem hiding this comment.
We do. m is a TensorV2, not a HalfTensor, which means getValue is a Tensor::getValue that requires template argument.
| for (unsigned int h = 0; h < height(); ++h) { | ||
| _FP16 *out_data = output.getAddress<_FP16>(b, c, h, 0); | ||
| const _FP16 *m_data = m.getAddress<_FP16>(b, c, h, 0); | ||
| const _FP16 *in_data = (_FP16 *)getAddress(getIndex(b, c, h, 0)); |
There was a problem hiding this comment.
Do we need the type casting for getAddress? This is HalfTensor->getAddress
There was a problem hiding this comment.
Yes. Since getAddress is an override method from TensorBase, it returns a void pointer, not _FP16 pointer.
There was a problem hiding this comment.
Do we need overwrite getAddress function from tensor base? How about add getAddress function without overwriting?
There was a problem hiding this comment.
We discussed offline and we will remove getAddress function in tensor_base.h later.
Commits to be reviewed in this PR
[TensorV2] Multiply strided inputs and output
This patch adds the implementation of multiplication operation that supports different strided inputs and output.
Self-evaluation:
Signed-off-by:Donghyeon Jeong [email protected]
[TensorV2] Additional functionality to access tensor elements
This commit adds functionality to retrieving and modifying tensor elements in specific locations.
This will provide convenience in implementing the math operation of tensor.
Self-evaluation:
Signed-off-by: Donghyeon Jeong [email protected]