-
Notifications
You must be signed in to change notification settings - Fork 615
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
Remove access to underlying contiguous TL buffer in Flip op #3280
Conversation
Signed-off-by: Krzysztof Lecki <[email protected]>
!build |
CI MESSAGE: [2833186]: BUILD STARTED |
CI MESSAGE: [2833186]: BUILD PASSED |
dali/operators/generic/flip.cu
Outdated
input.type().id(), DType, auto in_shape = TransformShapes(input.shape(), input.GetLayout()); | ||
auto in_view = reshape<flip_ndim>(view<const DType>(input), in_shape); | ||
kernels::KernelContext ctx; ctx.gpu.stream = stream; kernels::FlipGPU<DType> kernel; | ||
auto reqs = kernel.Setup(ctx, in_view); | ||
kernels::OutListGPU<DType, flip_ndim> out_view(output.mutable_data<DType>(), | ||
reqs.output_shapes[0].to_static<flip_ndim>()); | ||
kernel.Run(ctx, out_view, in_view, depthwise, vertical, horizontal); | ||
) | ||
auto out_view = | ||
reshape<flip_ndim>(view<DType>(output), reqs.output_shapes[0].to_static<flip_ndim>()); | ||
kernel.Run(ctx, out_view, in_view, depthwise, vertical, horizontal);) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is it justified to have multiple statements per line? Generally we'd rather avoid such...
I'd be in favour of extracting these to a separate function:
template<typename DType>
void f(...) {
auto in_shape = TransformShapes(input.shape(), input.GetLayout());
auto in_view = reshape<flip_ndim>(view<const DType>(input), in_shape);
kernels::KernelContext ctx;
ctx.gpu.stream = stream;
kernels::FlipGPU<DType> kernel;
auto reqs = kernel.Setup(ctx, in_view);
auto out_view =
reshape<flip_ndim>(view<DType>(output), reqs.output_shapes[0].to_static<flip_ndim>());
kernel.Run(ctx, out_view, in_view, depthwise, vertical, horizontal);)
}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have no idea why it happened here, probably due to the old DALI_TYPE_SWITCH the formatter got confused.
I will fix it, I don't think we need additional function though.
Signed-off-by: Krzysztof Lecki <[email protected]>
!build |
CI MESSAGE: [2848854]: BUILD STARTED |
CI MESSAGE: [2848854]: BUILD PASSED |
Description
What happened in this PR
Remove only the mutable_data and data usage
in GPU variant.
Tested with enforced shapes.
This op still uses per-sample approach in both
CPU and GPU variants, this is left unchanged.
Additional information
Affected modules and functionalities:
Flip
Key points relevant for the review:
Checklist
Tests
Documentation
DALI team only
Requirements
REQ IDs: N/A
JIRA TASK: N/A