Add GPU filter operator (2D, 3D) #4525

stiepan · 2022-12-19T10:28:11Z

Signed-off-by: Kamil Tokarski [email protected]

Category:

New feature (non-breaking change which adds functionality)

Description:

Adds GPU filter operator (2D and 3D convolutions with use-provided filters).

The relevant kernel part was merged here: #4298

Additional information:

A basic usage is just providing a batch of samples and batch of filters as two positional arguments (filters can reside on CPU or GPU).
By default the output has the same shape as the input, this means there is a need to handle somehow positions near the boundry that would cause OOB accesses. How it is done exactly is controlled with two parameters

border - what values to use to "pad" the input, possible modes are reflect 101, reflect 1001, wrap, clamp and constant. If the constant mode is used, user can provide a third argument that contains scalars to be used as a padding.
anchor - enables user to shift filter placement over the image

Alternatively, the output can be smaller so that there are no OOB accesses - this can be controlled with mode parameter which can be "same" (default) or "valid".

Affected modules and functionalities:

Key points relevant for the review:

Tests:

Checklist

Documentation

DALI team only

Requirements

Implements new requirements
Affects existing requirements
N/A

REQ IDs: N/A

JIRA TASK: DALI-2951

stiepan · 2023-01-02T14:31:18Z

!build

dali-automaton · 2023-01-02T14:47:01Z

CI MESSAGE: [6915075]: BUILD STARTED

dali-automaton · 2023-01-02T15:44:11Z

CI MESSAGE: [6915075]: BUILD FAILED

stiepan · 2023-01-02T16:07:44Z

!build

dali-automaton · 2023-01-02T16:29:41Z

CI MESSAGE: [6915466]: BUILD STARTED

dali-automaton · 2023-01-02T18:35:54Z

CI MESSAGE: [6915466]: BUILD PASSED

jantonguirao · 2023-01-03T11:19:05Z

dali/operators/image/convolution/filter.cc

+  the filter and the image.
+
+)code")
+    .NumInput(2, 3)


Suggested change

.NumInput(2, 3)

.NumInput(2, 3)

.InputDox(0, "data", "TensorList", R"code(Batch of input data.)code")

.InputDox(1, "filter", "TensorList", R"code(Batch of filters.)code")

.InputDox(2, ...)

Please add documentation of the inputs, and perhaps remove the sentence in line 25.

Done, I moved whole sections from the main description to relevant parameters' docs.

jantonguirao · 2023-01-03T11:20:07Z

dali/operators/image/convolution/filter.cc

+
+.. note::
+  In fact, the operator computes a correlation, not a convolution,
+  i.e. the order of filter elements is not flipped when computing product of


Suggested change

i.e. the order of filter elements is not flipped when computing product of

i.e. the order of filter elements is not flipped when computing the product of

jantonguirao · 2023-01-03T11:20:18Z

dali/operators/image/convolution/filter.cc

+    .NumOutput(1)
+    .AllowSequences()
+    .AddOptionalArg("anchor",
+                    R"code(Specifies position of the filter over the input.


Suggested change

R"code(Specifies position of the filter over the input.

R"code(Specifies the position of the filter over the input.

jantonguirao · 2023-01-03T11:22:53Z

dali/operators/image/convolution/filter.cu

+
+namespace filter {
+
+extern template std::unique_ptr<OpImplBase<GPUBackend>>


nitpick: use a macro to reduce verbosity?

jantonguirao · 2023-01-03T11:24:44Z

dali/test/python/test_dali_variable_batch_size.py

+        fill_value_bacthes = [list(fvs) for _, _, fvs in batches]
+
+        @pipeline_def
+        def piepline():


Suggested change

def piepline():

def pipeline():

jantonguirao · 2023-01-03T11:33:54Z

dali/operators/image/convolution/filter.h

+                                                     const filter::InputDesc& input_desc);
+
+  bool SetupImpl(std::vector<OutputDesc>& output_desc, const Workspace& ws) override {
+    if (!impl_) {


how about checking that the current impl_ instance matches the previous input and filter type?

It makes sense on the one hand, on the other isn't it something that should the exectur do?

stiepan · 2023-01-03T17:26:19Z

!build

dali-automaton · 2023-01-03T17:56:23Z

CI MESSAGE: [6922910]: BUILD FAILED

stiepan · 2023-01-03T18:04:28Z

!build

dali-automaton · 2023-01-03T18:09:45Z

CI MESSAGE: [6923202]: BUILD FAILED

Signed-off-by: Kamil Tokarski <[email protected]>

Add variable batch size test Add volumetric tests Signed-off-by: Kamil Tokarski <[email protected]>

Signed-off-by: Kamil Tokarski <[email protected]>

stiepan · 2023-01-03T18:24:28Z

I had to rebase over the operator tests split.

stiepan · 2023-01-03T18:24:35Z

!build

dali-automaton · 2023-01-03T18:30:27Z

CI MESSAGE: [6923309]: BUILD STARTED

dali-automaton · 2023-01-03T20:59:43Z

CI MESSAGE: [6923309]: BUILD PASSED

banasraf · 2023-01-04T13:05:20Z

dali/operators/image/convolution/filter.cc

+    .AddOptionalArg("border",
+                    R"code(Controls how to handle out-of-bound filter positions over the sample.
+
+Supported values are: ``"reflect_101"``, ``"reflect_1001"``, ``"wrap"``,


In the tests I've seen just "101" and "1001" values and not "reflect_*". Is it something different?

It's the same, just out of convenience.

The boundary.h

if (type == "reflect_1001" || type == "reflect1001" || type == "1001")

Ok, understood

* Add GPU filter operator (2D, 3D) Signed-off-by: Kamil Tokarski <[email protected]>

stiepan force-pushed the filter_op branch from 5e88a67 to b6fdf54 Compare January 2, 2023 10:25

stiepan marked this pull request as ready for review January 2, 2023 10:31

jantonguirao assigned banasraf and awolant Jan 2, 2023

jantonguirao self-assigned this Jan 3, 2023

jantonguirao approved these changes Jan 3, 2023

View reviewed changes

stiepan added 12 commits January 3, 2023 19:14

Add GPU filter operator (2D, 3D)

3850d61

Signed-off-by: Kamil Tokarski <[email protected]>

Docs nitpicks, basic tests

fe81f91

Signed-off-by: Kamil Tokarski <[email protected]>

Random samples tests

6435b6d

Signed-off-by: Kamil Tokarski <[email protected]>

Valid mode test cases

0ef9079

Signed-off-by: Kamil Tokarski <[email protected]>

Update the notice

5c9a38c

Signed-off-by: Kamil Tokarski <[email protected]>

Add float16 tests

63e5469

Signed-off-by: Kamil Tokarski <[email protected]>

No cpu only test for gpu op

287e8a2

Signed-off-by: Kamil Tokarski <[email protected]>

Remove unnecessary includes, doc nitpicks

ea82921

Add variable batch size test Add volumetric tests Signed-off-by: Kamil Tokarski <[email protected]>

Remove accidental specialization

2632e9d

Signed-off-by: Kamil Tokarski <[email protected]>

Adjust filter docs

eccdd13

Signed-off-by: Kamil Tokarski <[email protected]>

Use macro when enumerating all the specializations of filter impl

84a39ca

Signed-off-by: Kamil Tokarski <[email protected]>

Move tests to one of the split operator groups

aa0e438

Signed-off-by: Kamil Tokarski <[email protected]>

stiepan force-pushed the filter_op branch from db7c300 to aa0e438 Compare January 3, 2023 18:23

awolant approved these changes Jan 4, 2023

View reviewed changes

banasraf reviewed Jan 4, 2023

View reviewed changes

banasraf approved these changes Jan 4, 2023

View reviewed changes

stiepan merged commit 8db68c1 into NVIDIA:main Jan 4, 2023

stiepan mentioned this pull request Jan 5, 2023

Is sobel filtering implemented in DALI? #2963

Closed

aderylo pushed a commit to zpp-dali-2022/DALI that referenced this pull request Mar 17, 2023

Add GPU filter operator (2D, 3D) (NVIDIA#4525)

abda284

* Add GPU filter operator (2D, 3D) Signed-off-by: Kamil Tokarski <[email protected]>

JanuszL mentioned this pull request Sep 6, 2023

Roadmap 2023 #4578

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add GPU filter operator (2D, 3D) #4525

Add GPU filter operator (2D, 3D) #4525

stiepan commented Dec 19, 2022 •

edited

Loading

stiepan commented Jan 2, 2023

dali-automaton commented Jan 2, 2023

dali-automaton commented Jan 2, 2023

stiepan commented Jan 2, 2023

dali-automaton commented Jan 2, 2023

dali-automaton commented Jan 2, 2023

jantonguirao Jan 3, 2023

stiepan Jan 3, 2023

jantonguirao Jan 3, 2023

stiepan Jan 3, 2023

jantonguirao Jan 3, 2023

stiepan Jan 3, 2023

jantonguirao Jan 3, 2023

stiepan Jan 3, 2023

jantonguirao Jan 3, 2023

stiepan Jan 3, 2023

jantonguirao Jan 3, 2023

stiepan Jan 3, 2023

stiepan commented Jan 3, 2023

dali-automaton commented Jan 3, 2023

stiepan commented Jan 3, 2023

dali-automaton commented Jan 3, 2023

stiepan commented Jan 3, 2023

stiepan commented Jan 3, 2023

dali-automaton commented Jan 3, 2023

dali-automaton commented Jan 3, 2023

banasraf Jan 4, 2023

stiepan Jan 4, 2023

banasraf Jan 4, 2023

-    .NumInput(2, 3)
+    .NumInput(2, 3)
+    .InputDox(0, "data", "TensorList", R"code(Batch of input data.)code")
+    .InputDox(1, "filter", "TensorList", R"code(Batch of filters.)code")
+    .InputDox(2, ...)

	i.e. the order of filter elements is not flipped when computing product of
	i.e. the order of filter elements is not flipped when computing the product of

	R"code(Specifies position of the filter over the input.
	R"code(Specifies the position of the filter over the input.


		namespace filter {

		extern template std::unique_ptr<OpImplBase<GPUBackend>>

Add GPU filter operator (2D, 3D) #4525

Add GPU filter operator (2D, 3D) #4525

Conversation

stiepan commented Dec 19, 2022 • edited Loading

Category:

Description:

Additional information:

Affected modules and functionalities:

Key points relevant for the review:

Tests:

Checklist

Documentation

DALI team only

Requirements

stiepan commented Jan 2, 2023

dali-automaton commented Jan 2, 2023

dali-automaton commented Jan 2, 2023

stiepan commented Jan 2, 2023

dali-automaton commented Jan 2, 2023

dali-automaton commented Jan 2, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

stiepan commented Jan 3, 2023

dali-automaton commented Jan 3, 2023

stiepan commented Jan 3, 2023

dali-automaton commented Jan 3, 2023

stiepan commented Jan 3, 2023

stiepan commented Jan 3, 2023

dali-automaton commented Jan 3, 2023

dali-automaton commented Jan 3, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

stiepan commented Dec 19, 2022 •

edited

Loading