Debug mode direct ExternalSource #3605

ksztenderski · 2022-01-03T17:29:53Z

Description

Bug fix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)
Breaking change (fix or feature that would cause existing functionality to not work as expected)
Refactoring (Redesign of existing code that doesn't affect functionality)
Other (e.g. Documentation, Tests, Configuration)

What happened in this PR

Adds debug version of ExternalSource operator removing additional callback to the backend. Before that, in the debug mode for external source we created a separate pipeline like for any other operator, which seems like an overkill considering that all of the ExternalSource implementation (relevant for debug mode) is in Python.

Adds TensorList constructor from list of Tensors.

Additional information

Affected modules and functionalities:

Key points relevant for the review:

Checklist

Tests

Documentation

DALI team only

Requirements

Implements new requirements
Affects existing requirements
N/A

REQ IDs: N/A

JIRA TASK: N/A

Add debug mode version of ExternalSource operator removing creation of seperate pipeline for it. Add TensorList constructor from list of Tensors. Add seperation of preperation of data for feed_input from actual feed_input. Add tests of ExternalSource in debug mode. Signed-off-by: ksztenderski <ksztenderski@nvidia.com>

Signed-off-by: ksztenderski <ksztenderski@nvidia.com>

dali/python/nvidia/dali/pipeline.py

dali/python/backend_impl.cc

ksztenderski · 2022-01-03T17:50:14Z

!build

dali-automaton · 2022-01-03T17:56:47Z

CI MESSAGE: [3690021]: BUILD STARTED

dali-automaton · 2022-01-03T18:04:27Z

CI MESSAGE: [3690021]: BUILD FAILED

Signed-off-by: ksztenderski <ksztenderski@nvidia.com>

ksztenderski · 2022-01-04T09:45:54Z

!build

dali-automaton · 2022-01-04T09:51:10Z

CI MESSAGE: [3693373]: BUILD STARTED

Signed-off-by: ksztenderski <ksztenderski@nvidia.com>

dali-automaton · 2022-01-04T11:25:48Z

CI MESSAGE: [3693373]: BUILD PASSED

klecki

Comments only to TensorList constructor, I didn't read the rest yet.

klecki · 2022-01-10T17:13:24Z

dali/python/backend_impl.cc

+    auto &t = list_of_tensors[i].cast<Tensor<Backend>&>();
+    tv[i] = std::move(t);
+  }
+  tl->Copy(tv, 0);


I think we probably should have a sync. We might do a Copy on some other stream (we have this stream pool that creates stream per-device), and sync on that stream, as syncing stream 0 might be a bit over the top.

dali/python/backend_impl.cc

klecki · 2022-01-10T17:21:11Z

dali/test/python/test_backend_impl.py

@@ -142,3 +142,18 @@ def check_squeeze(shape, dim, in_layout, expected_out_layout):
             (0, (1, 5, 1), "ABC", "BC"),
             (None, (3, 5, 1), "ABC", "AB")]:
        yield check_squeeze, shape, dim, in_layout, expected_out_layout
+
+
+def test_tensorlist_constructor_from_list_of_tensors():


It might be nice to test non-uniformly shaped TensorList as well (where each tensor has different dimensions).

Add test for TL with different tensors of different shapes Signed-off-by: ksztenderski <ksztenderski@nvidia.com>

…derski/DALI into debug_mode_direct_external_source Signed-off-by: ksztenderski <ksztenderski@nvidia.com>

ksztenderski · 2022-01-11T14:11:08Z

!build

dali-automaton · 2022-01-11T14:16:18Z

CI MESSAGE: [3735215]: BUILD STARTED

dali-automaton · 2022-01-11T15:31:25Z

CI MESSAGE: [3735215]: BUILD PASSED

klecki · 2022-01-12T17:20:07Z

dali/python/nvidia/dali/pipeline.py

@@ -55,6 +57,55 @@ def _get_default_stream_for_array(array):
    else:
        return None

+
+def _prep_data_for_feed_input(data, batch_size, layout):


Just a nitpick, we can probably move it to external_source.py

klecki · 2022-01-12T17:32:59Z

dali/python/backend_impl.cc

+  TensorVector<Backend> tv(list_of_tensors.size());
+  for (size_t i = 0; i < list_of_tensors.size(); ++i) {
+    auto &t = list_of_tensors[i].cast<Tensor<Backend>&>();
+    tv[i] = std::move(t);


Doesn't this move the data out of the list? Won't this be destructive function?

changed to ShareData

dali/python/backend_impl.cc

klecki · 2022-01-12T17:54:37Z

dali/python/nvidia/dali/_debug_mode.py

+            if self._num_outputs is not None:
+                for idx in range(self._num_outputs):
+                    if self._batch:
+                        raw_data = callback_out[idx]


Is this correct? We just save last output as raw_data here?

It was a mistake, fixed it.

klecki · 2022-01-12T17:55:47Z

dali/python/nvidia/dali/_debug_mode.py

+                    if self._batch:
+                        raw_data = callback_out[idx]
+                    else:
+                        raw_data = [callback_out[i][idx] for i in range(self._batch_size)]


dali/python/nvidia/dali/_debug_mode.py

Use ShareData in TL constructor from list of tensors instead of move Signed-off-by: ksztenderski <ksztenderski@nvidia.com>

ksztenderski · 2022-01-13T14:11:02Z

!build

dali-automaton · 2022-01-13T14:16:48Z

CI MESSAGE: [3750054]: BUILD STARTED

dali-automaton · 2022-01-13T15:45:22Z

CI MESSAGE: [3750054]: BUILD FAILED

dali-automaton · 2022-01-13T15:53:29Z

CI MESSAGE: [3750054]: BUILD PASSED

Add direct external_source in debug mode Adds debug version of ExternalSource operator removing additional callback to the backend. Before that, in the debug mode for external source we created a separate pipeline like for any other operator, which seems like an overkill considering that all of the ExternalSource implementation (relevant for the debug mode) is in Python. Adds TensorList constructor from list of Tensors. Signed-off-by: ksztenderski <ksztenderski@nvidia.com>

ksztenderski added 2 commits January 3, 2022 17:34

Test for TensorList constructor from list of Tensors

4f1b4e9

Signed-off-by: ksztenderski <ksztenderski@nvidia.com>

ksztenderski assigned klecki Jan 3, 2022

ksztenderski commented Jan 3, 2022

View reviewed changes

dali/python/nvidia/dali/pipeline.py Outdated Show resolved Hide resolved

ksztenderski commented Jan 3, 2022

View reviewed changes

dali/python/nvidia/dali/pipeline.py Show resolved Hide resolved

ksztenderski commented Jan 3, 2022

View reviewed changes

dali/python/backend_impl.cc Show resolved Hide resolved

Reformat code

f53023b

Signed-off-by: ksztenderski <ksztenderski@nvidia.com>

Update copyright year

20c47ff

Signed-off-by: ksztenderski <ksztenderski@nvidia.com>

jantonguirao assigned awolant Jan 4, 2022

klecki reviewed Jan 10, 2022

View reviewed changes

ksztenderski and others added 3 commits January 11, 2022 10:12

Merge branch 'main' into debug_mode_direct_external_source

b96b11d

Use userStream in TL constructor from list of tensors

d01346e

Add test for TL with different tensors of different shapes Signed-off-by: ksztenderski <ksztenderski@nvidia.com>

Merge branch 'debug_mode_direct_external_source' of github.com:kszten…

7f6a09b

…derski/DALI into debug_mode_direct_external_source Signed-off-by: ksztenderski <ksztenderski@nvidia.com>

awolant approved these changes Jan 11, 2022

View reviewed changes

klecki reviewed Jan 12, 2022

View reviewed changes

Fix multiple outputs ES in debug mode

8c429b9

Use ShareData in TL constructor from list of tensors instead of move Signed-off-by: ksztenderski <ksztenderski@nvidia.com>

klecki approved these changes Jan 13, 2022

View reviewed changes

ksztenderski merged commit 2d40b1a into NVIDIA:main Jan 13, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Debug mode direct ExternalSource #3605

Debug mode direct ExternalSource #3605

ksztenderski commented Jan 3, 2022 •

edited

Loading

ksztenderski commented Jan 3, 2022

dali-automaton commented Jan 3, 2022

dali-automaton commented Jan 3, 2022

ksztenderski commented Jan 4, 2022

dali-automaton commented Jan 4, 2022

dali-automaton commented Jan 4, 2022

klecki left a comment

klecki Jan 10, 2022

ksztenderski Jan 11, 2022

klecki Jan 10, 2022

ksztenderski Jan 11, 2022

ksztenderski commented Jan 11, 2022

dali-automaton commented Jan 11, 2022

dali-automaton commented Jan 11, 2022

klecki Jan 12, 2022

ksztenderski Jan 13, 2022

klecki Jan 12, 2022

ksztenderski Jan 13, 2022

klecki Jan 12, 2022

ksztenderski Jan 13, 2022

klecki Jan 12, 2022

ksztenderski Jan 13, 2022

ksztenderski commented Jan 13, 2022

dali-automaton commented Jan 13, 2022

dali-automaton commented Jan 13, 2022

dali-automaton commented Jan 13, 2022

Debug mode direct ExternalSource #3605

Debug mode direct ExternalSource #3605

Conversation

ksztenderski commented Jan 3, 2022 • edited Loading

Description

What happened in this PR

Additional information

Checklist

Tests

Documentation

DALI team only

Requirements

ksztenderski commented Jan 3, 2022

dali-automaton commented Jan 3, 2022

dali-automaton commented Jan 3, 2022

ksztenderski commented Jan 4, 2022

dali-automaton commented Jan 4, 2022

dali-automaton commented Jan 4, 2022

klecki left a 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

ksztenderski commented Jan 11, 2022

dali-automaton commented Jan 11, 2022

dali-automaton commented Jan 11, 2022

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

ksztenderski commented Jan 13, 2022

dali-automaton commented Jan 13, 2022

dali-automaton commented Jan 13, 2022

dali-automaton commented Jan 13, 2022

ksztenderski commented Jan 3, 2022 •

edited

Loading