Add Expand dims operator #2800

majra20 · 2021-03-16T17:04:56Z

Why we need this PR?

It adds new feature with new operator

What happened in this PR?

What solution was applied:
- Added ExpandDims operator
Affected modules and functionalities:
- ExpandDims operator which allows to squeeze dimensions of size one
Key points relevant for the review:
- ExpandDims operator code, does it do everything we wanted
Validation and testing:
- Python tests: test_operator_expand_dims.py
Documentation (including examples):
NA

JIRA TASK: DALI-1851

mzient · 2021-03-17T08:27:14Z

dali/operators/generic/expand_dims.cc

+    std::vector<int>(), true)
+  .AddOptionalArg("new_axis_names", R"code(Names of new dimensions in data layout.
+
+Size of ``new_axis_names`` should be equal to ``axe``s size. 


The length of new_axis_names must match the length of axes.

mzient · 2021-03-17T08:45:05Z

dali/operators/generic/expand_dims.cc

+    }
+    out_layout += in_layout.empty() ? '?' : in_layout[d];


If the input doesn't have a well defined layout, the output layout should also be empty.

The logic should be something like:

if (1) input has layout if (2) new_axis_names_ match the length of axes: output layout is a product of inserting new_axis_names at `axes` else either (3) reset layout to empty or (4) insert `?` else (5) output layout is empty

(1) - We should assume that 0D input with empty layout has proper layout
(2) - This differs from new_axis_names.empty() in case when axes are also empty
(3) - This is consistent with (5) for 0D and >0D
(4) - This is inconsistent

Elaborating on inconsistency between (4) and (5):
A 0D has both specified and empty layout (layout is not a nullable property).
Now, if we insert ?, a 0D tensor will never be expanded to one with empty layout, even though it's very likely that the user wants it - instead, the tensor will get a (possibly invalid) layout with multiple ? inserted.

Examples of the proposed logic:

ndim = 2
layout = HW
axes = [2]
axis_names = C
output layout HWC

ndim = 2
layout
axes = [2]
axis_names = 'C'
output: error - specifying axis names requires an input with a proper layuout

ndim = 2
layout = HW
axes = []
new_axis_names = ""
output layout = HW

ndim = 0
layout = ""
axes = [0, 1]
new_axis_names = "HW"
output layout = HW

ndim = 0
layout = ""
axes = [0, 1]
new_axis_names = ""
output layout = ""

ndim = 2
layout = "HW"
axes = [0]
new_axis_names = ""
output layout = ""

jantonguirao · 2021-03-17T10:09:20Z

dali/operators/generic/expand_dims.cc

+  .DocStr(R"code(Insert new dimension[s] of extent 1 and inserts new entries in "
+    "the layout (new_axis_names) at these indices in the layout.)code")


We probably want a shorter first description for the table of operators.
I'd say

Suggested change

.DocStr(R"code(Insert new dimension[s] of extent 1 and inserts new entries in "

"the layout (new_axis_names) at these indices in the layout.)code")

.DocStr(R"code(Insert new dimension(s) with extent 1 to the data shape.

The new dimensions are inserted at the positions specified by ``axes``.

If ``new_axis_names`` is provided, the new dimension names will be inserted in the data layout, at the positions specified by ``axes``. If ``new_axis_names`` is not provided, the output data layout will be empty.")code")

jantonguirao · 2021-03-17T10:13:57Z

dali/operators/generic/expand_dims.cc

+  .PassThrough({{0, 0}})
+  .AllowSequences()
+  .SupportVolumetric()
+  .AddOptionalArg<int>("axes", R"code(Indices where to put new dimensions of size 1.)code",


Suggested change

.AddOptionalArg<int>("axes", R"code(Indices where to put new dimensions of size 1.)code",

.AddOptionalArg<int>("axes", R"code(Indices representing the positions in the data shape where a new dimension with extent 1 will be inserted.

The indices should be in the range ``[0, ndim]``, where ``ndim`` is the number of dimensions in the input. Providing the index ``ndim`` results in a new dimension appended at the end of the shape.)code",

jantonguirao · 2021-03-17T10:14:38Z

dali/operators/generic/expand_dims.cc

+  .SupportVolumetric()
+  .AddOptionalArg<int>("axes", R"code(Indices where to put new dimensions of size 1.)code",
+    std::vector<int>(), true)
+  .AddOptionalArg("new_axis_names", R"code(Names of new dimensions in data layout.


Suggested change

.AddOptionalArg("new_axis_names", R"code(Names of new dimensions in data layout.

.AddOptionalArg("new_axis_names", R"code(Names of the new dimensions in the data layout.

jantonguirao · 2021-03-17T10:16:18Z

dali/operators/generic/expand_dims.cc

+  new_axis_names_ = spec.GetArgument<TensorLayout>("new_axis_names");
+  if (!new_axis_names_.empty()) {
+    DALI_ENFORCE(new_axis_names_.size() == axes_.size(), make_string("Specified ", axes_.size(),
+      " new dimensions, but layout specify ", new_axis_names_.size(), " new names"));


Suggested change

" new dimensions, but layout specify ", new_axis_names_.size(), " new names"));

" new dimensions, but layout contains only ", new_axis_names_.size(), " new dimension names"));

mzient · 2021-03-17T11:41:38Z

dali/operators/generic/expand_dims.cc

+  .PassThrough({{0, 0}})
+  .AllowSequences()
+  .SupportVolumetric()
+  .AddOptionalArg<int>("axes", R"code(Indices where to put new dimensions of size 1.)code",


Suggested change

.AddOptionalArg<int>("axes", R"code(Indices where to put new dimensions of size 1.)code",

.AddOptionalArg<int>("axes", R"code(Indices at which the new dimensions are inserted.

.)code",

Signed-off-by: Rafal Maj <rmaj@nvidia.com>

dali/operators/generic/expand_dims.cc

Signed-off-by: Rafal Maj <rmaj@nvidia.com>

mzient · 2021-03-17T12:04:43Z

dali/operators/generic/expand_dims.cc

+  .AddOptionalArg("new_axis_names", R"code(Names of the new dimensions in the data layout.
+
+The length of ``new_axis_names`` must match the length of ``axes``.
+If argument won't be provided new dimensions will have layout '?')code", TensorLayout(""));


Suggested change

If argument won't be provided new dimensions will have layout '?')code", TensorLayout(""));

If argument isn't be provided new dimensions will have layout '?')code", TensorLayout(""));

mzient · 2021-03-17T12:07:11Z

dali/operators/generic/expand_dims.cc

+  .AddOptionalArg("new_axis_names", R"code(Names of the new dimensions in the data layout.
+
+The length of ``new_axis_names`` must match the length of ``axes``.
+If argument won't be provided layout will be cleared.)code", TensorLayout(""));


Suggested change

If argument won't be provided layout will be cleared.)code", TensorLayout(""));

If argument is not provided, output layout will be cleared.)code", TensorLayout(""));

or

Suggested change

If argument won't be provided layout will be cleared.)code", TensorLayout(""));

If argument is not provided, the layout will be cleared.)code", TensorLayout(""));

mzient · 2021-03-17T12:07:53Z

dali/operators/generic/expand_dims.cc

+ExpandDims<Backend>::ExpandDims(const OpSpec &spec)
+    : Reshape<Backend>(spec, typename Reshape<Backend>::BypassInit()) {
+  axes_ = spec.GetRepeatedArgument<int>("axes");
+  DALI_ENFORCE(spec.HasArgument("axes"), make_string("``axes`` argument should be provided."));


This is not necessary. Just make the argument mandatory in the schema.

mzient · 2021-03-17T12:09:35Z

dali/operators/generic/expand_dims.cc

+  .AddOptionalArg<int>("axes", R"code(Indices at which the new dimensions are inserted.)code",
+    std::vector<int>(), true)


Suggested change

.AddOptionalArg<int>("axes", R"code(Indices at which the new dimensions are inserted.)code",

std::vector<int>(), true)

.AddArg("axes", R"code(Indices at which the new dimensions are inserted.)code",

DALI_INT_VEC, true)

mzient · 2021-03-17T12:11:50Z

dali/operators/generic/expand_dims.cc

+  }
+  std::sort(axes_.begin(), axes_.end());
+  DALI_ENFORCE(std::adjacent_find(axes_.begin(), axes_.end()) == axes_.end(),
+    make_string("Specified at least twice same index to add new dimension."));


Suggested change

make_string("Specified at least twice same index to add new dimension."));

make_string("Duplicate axis index found."));

Signed-off-by: Rafal Maj <rmaj@nvidia.com>

mzient · 2021-03-17T17:23:54Z

dali/operators/generic/expand_dims.cc

+  auto in_layout = in.GetLayout();
+  if (in_layout.empty() && ndim) {
+    DALI_ENFORCE(!use_new_axis_names_arg_,
+      make_string("Specifying ``new_axis_names`` requires an input with a proper layuout."));


Suggested change

make_string("Specifying ``new_axis_names`` requires an input with a proper layuout."));

make_string("Specifying ``new_axis_names`` requires an input with a proper layout."));

Signed-off-by: Rafal Maj <rmaj@nvidia.com>

majra20 · 2021-03-18T07:59:52Z

!build

dali-automaton · 2021-03-18T08:06:03Z

CI MESSAGE: [2179748]: BUILD STARTED

dali-automaton · 2021-03-18T09:37:40Z

CI MESSAGE: [2179748]: BUILD PASSED

majra20 assigned jantonguirao and mzient Mar 16, 2021

mzient reviewed Mar 17, 2021

View reviewed changes

jantonguirao reviewed Mar 17, 2021

View reviewed changes

mzient reviewed Mar 17, 2021

View reviewed changes

Rafal Maj added 24 commits March 17, 2021 12:58

src_dims arg added to reshape

5bd8317

Signed-off-by: Rafal Maj <rmaj@nvidia.com>

Code review adjustments

50d6bea

Signed-off-by: Rafal Maj <rmaj@nvidia.com>

Add use_src_dims_ flag

e6a8b16

Signed-off-by: Rafal Maj <rmaj@nvidia.com>

src_dims arg added to reshape

0cd0f1a

Signed-off-by: Rafal Maj <rmaj@nvidia.com>

Code review adjustments

a0b3322

Signed-off-by: Rafal Maj <rmaj@nvidia.com>

squeeze operator

f8d2bfb

Signed-off-by: Rafal Maj <rmaj@nvidia.com>

Rebase

587bc2d

Signed-off-by: Rafal Maj <rmaj@nvidia.com>

Schema desc, 0d tensor handling

220c6db

Signed-off-by: Rafal Maj <rmaj@nvidia.com>

rebase

703f9bc

Signed-off-by: Rafal Maj <rmaj@nvidia.com>

Change of copyright year

9a26503

Signed-off-by: Rafal Maj <rmaj@nvidia.com>

Bypass init in reshape constructor

a88256a

Signed-off-by: Rafal Maj <rmaj@nvidia.com>

Fix build

76f9f63

Signed-off-by: Rafal Maj <rmaj@nvidia.com>

Add degenerate test cases

c1d58e0

Signed-off-by: Rafal Maj <rmaj@nvidia.com>

Add use_src_dims_ flag

abc9a0d

Signed-off-by: Rafal Maj <rmaj@nvidia.com>

squeeze operator

99da3f7

Signed-off-by: Rafal Maj <rmaj@nvidia.com>

Schema desc, 0d tensor handling

51b13aa

Signed-off-by: Rafal Maj <rmaj@nvidia.com>

Expand dims operator

0a38f1d

Signed-off-by: Rafal Maj <rmaj@nvidia.com>

Rebase

1d18b26

Signed-off-by: Rafal Maj <rmaj@nvidia.com>

Add use_src_dims_ flag handling

4cad892

Signed-off-by: Rafal Maj <rmaj@nvidia.com>

Add documentation

c5d5d12

Signed-off-by: Rafal Maj <rmaj@nvidia.com>

Skip reshape constructor

91a087d

Signed-off-by: Rafal Maj <rmaj@nvidia.com>

Remove unused arg

948a6e5

Signed-off-by: Rafal Maj <rmaj@nvidia.com>

Style changes

987f06f

Signed-off-by: Rafal Maj <rmaj@nvidia.com>

Fix merge

20fd0bd

Signed-off-by: Rafal Maj <rmaj@nvidia.com>

Reimplementing adding new dims

3157599

Signed-off-by: Rafal Maj <rmaj@nvidia.com>

majra20 force-pushed the expand_dims branch from e1fc19e to 3157599 Compare March 17, 2021 12:00

mzient reviewed Mar 17, 2021

View reviewed changes

dali/operators/generic/expand_dims.cc Show resolved Hide resolved

Changing docs

328b6f0

Signed-off-by: Rafal Maj <rmaj@nvidia.com>

mzient reviewed Mar 17, 2021

View reviewed changes

Handling sort correctly

19d4dd1

Signed-off-by: Rafal Maj <rmaj@nvidia.com>

mzient reviewed Mar 17, 2021

View reviewed changes

jantonguirao approved these changes Mar 17, 2021

View reviewed changes

Remove redundant check, typo

45ba4eb

Signed-off-by: Rafal Maj <rmaj@nvidia.com>

mzient approved these changes Mar 18, 2021

View reviewed changes

majra20 merged commit e442122 into NVIDIA:master Mar 18, 2021

majra20 deleted the expand_dims branch March 18, 2021 09:55

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add Expand dims operator #2800

Add Expand dims operator #2800

majra20 commented Mar 16, 2021

mzient Mar 17, 2021

mzient Mar 17, 2021

jantonguirao Mar 17, 2021

jantonguirao Mar 17, 2021

jantonguirao Mar 17, 2021

jantonguirao Mar 17, 2021

mzient Mar 17, 2021

mzient Mar 17, 2021

mzient Mar 17, 2021

mzient Mar 17, 2021

mzient Mar 17, 2021

mzient Mar 17, 2021

mzient Mar 17, 2021

majra20 commented Mar 18, 2021

dali-automaton commented Mar 18, 2021

dali-automaton commented Mar 18, 2021

		.DocStr(R"code(Insert new dimension[s] of extent 1 and inserts new entries in "
		"the layout (new_axis_names) at these indices in the layout.)code")

-  .DocStr(R"code(Insert new dimension[s] of extent 1 and inserts new entries in "
-    "the layout (new_axis_names) at these indices in the layout.)code")
+  .DocStr(R"code(Insert new dimension(s) with extent 1 to the data shape.
+The new dimensions are inserted at the positions specified by ``axes``.
+If ``new_axis_names`` is provided, the new dimension names will be inserted in the data layout, at the positions specified by ``axes``. If ``new_axis_names`` is not provided, the output data layout will be empty.")code")

-  .AddOptionalArg<int>("axes", R"code(Indices where to put new dimensions of size 1.)code",
+  .AddOptionalArg<int>("axes", R"code(Indices representing the positions in the data shape where a new dimension with extent 1 will be inserted.
+The indices should be in the range ``[0, ndim]``, where ``ndim`` is the number of dimensions in the input. Providing the index ``ndim`` results in a new dimension appended at the end of the shape.)code",

	.AddOptionalArg("new_axis_names", R"code(Names of new dimensions in data layout.
	.AddOptionalArg("new_axis_names", R"code(Names of the new dimensions in the data layout.

	" new dimensions, but layout specify ", new_axis_names_.size(), " new names"));
	" new dimensions, but layout contains only ", new_axis_names_.size(), " new dimension names"));

	.AddOptionalArg<int>("axes", R"code(Indices where to put new dimensions of size 1.)code",
	.AddOptionalArg<int>("axes", R"code(Indices at which the new dimensions are inserted.
	.)code",

	If argument won't be provided new dimensions will have layout '?')code", TensorLayout(""));
	If argument isn't be provided new dimensions will have layout '?')code", TensorLayout(""));

	If argument won't be provided layout will be cleared.)code", TensorLayout(""));
	If argument is not provided, output layout will be cleared.)code", TensorLayout(""));

	If argument won't be provided layout will be cleared.)code", TensorLayout(""));
	If argument is not provided, the layout will be cleared.)code", TensorLayout(""));

		.AddOptionalArg<int>("axes", R"code(Indices at which the new dimensions are inserted.)code",
		std::vector<int>(), true)

	make_string("Specified at least twice same index to add new dimension."));
	make_string("Duplicate axis index found."));

	make_string("Specifying ``new_axis_names`` requires an input with a proper layuout."));
	make_string("Specifying ``new_axis_names`` requires an input with a proper layout."));

Add Expand dims operator #2800

Add Expand dims operator #2800

Conversation

majra20 commented Mar 16, 2021

Why we need this PR?

What happened in this PR?

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

Choose a reason for hiding this comment

majra20 commented Mar 18, 2021

dali-automaton commented Mar 18, 2021

dali-automaton commented Mar 18, 2021