Linear transformation GPU kernel #1262
Conversation
szalpal
changed the title
|
!build |
|
CI MESSAGE: [902327]: BUILD STARTED |
|
CI MESSAGE: [902327]: BUILD PASSED |
| for (int i = 0; i < M; i++) { | ||
| val += sample.transformation_matrix.at(o, i) * in(x, y, i); | ||
| } | ||
| out(x, y, o) = val; |
There was a problem hiding this comment.
I don't know if you need to reimplement the matrix - vector multiplication.
Maybe something like that:
auto *in_vec = reinterpret_cast<vec<InputType, M>*>(&in(x, y, 0));
auto *out_vec = reinterpret_cast<vec<OutputType, N>*>(&out(x, y, 0));
*out_vec = sample.transformation_matrix * (*in_vec);Or instead of these reinterpret_casts you could just copy memory to temporary vecs.
There was a problem hiding this comment.
- This would only work with dense HWC layout.
- We can reinterpret input, but for the output we need to apply proper conversion anyway.
There was a problem hiding this comment.
Many issues.
- Incomplete functionality (M*x + B - missing B)
- Implementation bugs
- input pointers don't have proper ROI offsets
- number of channels should be fixed, based on matrix shape
- loss of data when accumulating to OutputType (should be float)
- Misleading namespace - "algebra" suggests something more than a pixelwise operation
...and more problems.
|
!build |
|
CI MESSAGE: [905830]: BUILD STARTED |
|
CI MESSAGE: [905830]: BUILD PASSED |
Signed-off-by: Michał Szołucha <mszolucha@nvidia.com>
| ASSERT_EQ(mat.rows * mat.cols * mat.channels(), res.first.num_elements()) | ||
| << "Number of elements doesn't match"; | ||
| auto ptr = reinterpret_cast<typename TypeParam::Out *>(mat.data); | ||
| for (int i = 0; i < res.first.num_elements(); i++) { |
There was a problem hiding this comment.
I really don't like this loop - you're flattening the shape and it's not a good idea when considering ROIs.
Why don't you have a TensorView and use Check function that will print failing coordintates and limit the output size in case of some total disaster?
There was a problem hiding this comment.
I used the OpenCV to calculate proper ROI. AFAIK, there's no easy way to create BB-ed TensorView (which would be good idea, but not for this PR)
There was a problem hiding this comment.
You can create a TensorView back from the cv::Mat and use Check function.
dali/kernels/imgproc/pointwise/linear_transformation_gpu_test.cu
Outdated
Show resolved
Hide resolved
| Out *output_; | ||
| std::vector<In> input_host_; | ||
| std::vector<float> ref_output_; | ||
| std::vector<TensorShape<kNDims>> in_shapes_ = {{4, 3, kNChannelsIn}, {4, 3, kNChannelsIn}}; |
There was a problem hiding this comment.
Why not:
| std::vector<TensorShape<kNDims>> in_shapes_ = {{4, 3, kNChannelsIn}, {4, 3, kNChannelsIn}}; | |
| TensorListShape<kNDims> in_shapes_ = {{{4, 3, kNChannelsIn}, {4, 3, kNChannelsIn}}}; |
Signed-off-by: Michał Szołucha <mszolucha@nvidia.com>
|
!build |
|
|
||
| namespace dali { | ||
| namespace kernels { | ||
| namespace lin_trans { |
There was a problem hiding this comment.
lin_trans doesn't tell me anything. linear_transform at least
|
CI MESSAGE: [925087]: BUILD STARTED |
| } | ||
| }; | ||
|
|
||
| using TestTypes = std::tuple<float>; |
There was a problem hiding this comment.
at least one uint type
|
CI MESSAGE: [925087]: BUILD PASSED |
* Linear transformation GPU kernel Signed-off-by: Michał Szołucha <mszolucha@nvidia.com> Signed-off-by: Jianjun Liu <00liujj@163.com>
Why we need this PR?
What happened in this PR?