Add input type validation to feed_ndarray in MXNet and PyTorch #3308

JanuszL · 2021-09-02T17:55:50Z

adds type validation between DALI Tensor/TensorList and MXNet/PyTorch tensor
inside feed_ndarray just in case anyone wants to use feed_ndarray directly
it doesn't cover PaddlePaddle as feed_ndarray accepts a raw pointer to
PaddlePaddle tensor and there is no API to check the type
updates usage of raises, assert_raises in test_fw_iterators.py to use
implementation from nose_utils

Signed-off-by: Janusz Lisiecki jlisiecki@nvidia.com

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 type validation between DALI Tensor/TensorList and MXNet/PyTorch tensor
inside feed_ndarray just in case anyone wants to use feed_ndarray directly
it doesn't cover PaddlePaddle as feed_ndarray accepts a raw pointer to
PaddlePaddle tensor and there is no API to check the type
updates usage of raises, assert_raises in test_fw_iterators.py to use
implementation from nose_utils

Additional information

Affected modules and functionalities:
- test_fw_iterators.py
- plugin/mxnet.py
- plugin/pytorch.py

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

- adds type validation between DALI Tensor/TensorList and MXNet/PyTorch tensor inside feed_ndarray just in case anyone wants to use feed_ndarray directly - it doesn't cover PaddlePaddle as feed_ndarray accepts a raw pointer to PaddlePaddle tensor and there is no API to check the type - updates usage of raises, assert_raises in test_fw_iterators.py to use implementation from nose_utils Signed-off-by: Janusz Lisiecki <jlisiecki@nvidia.com>

JanuszL · 2021-09-02T17:56:18Z

!build

dali-automaton · 2021-09-02T18:01:21Z

CI MESSAGE: [2901568]: BUILD STARTED

dali-automaton · 2021-09-02T19:09:48Z

CI MESSAGE: [2901568]: BUILD PASSED

JanuszL · 2021-09-03T11:28:55Z

dali/python/nvidia/dali/plugin/mxnet.py

@@ -52,6 +52,15 @@ def feed_ndarray(dali_tensor, arr, cuda_stream = None):
                    In most cases, using the default internal user stream or stream 0
                    is expected.
    """
+    if isinstance(dali_tensor, (TensorListCPU, TensorListGPU)):


We cannot do such thing for PaddlePaddle as feed_ndarray there accepts raw pointer. We cannot accept tensor itself there as we cannot extract pointer itself to it without providing placement and type, so we would need to extend feed_ndarray signature and move allocation of data there as well (setting shape and placement does that). See the API.

stiepan · 2021-09-03T15:05:29Z

dali/test/python/test_fw_iterators.py

+    pipe.build()
+    out = pipe.run()[0]
+    torch_tensor = torch.empty((1), dtype=torch.int8, device = 'cpu')
+    assert_raises(AssertionError, feed_ndarray, out, torch_tensor, glob="Type of DALI Tensor/TensorList doesn't match Torch tensor type:*")


Just a nitpick, error message checking looks for any occurrence of the pattern, so stars are not necessary at the beginning and end.

JanuszL · 2021-09-03T15:39:50Z

!build

dali-automaton · 2021-09-03T15:46:03Z

CI MESSAGE: [2909038]: BUILD STARTED

dali-automaton · 2021-09-03T17:06:22Z

CI MESSAGE: [2909038]: BUILD FAILED

dali-automaton · 2021-09-03T17:36:21Z

CI MESSAGE: [2909038]: BUILD PASSED

mzient · 2021-09-06T12:39:14Z

dali/test/python/test_fw_iterators.py

@@ -344,7 +345,8 @@ def check_mxnet_iterator_pass_reader_name(shards_num, pipes_number, batch_size,

    if batch_size > data_set_size // shards_num and last_batch_policy == LastBatchPolicy.DROP:
        assert_raises(AssertionError, MXNetIterator, pipes, [
-                      ("ids", MXNetIterator.DATA_TAG)], reader_name="Reader", last_batch_policy=last_batch_policy)
+                      ("ids", MXNetIterator.DATA_TAG)], reader_name="Reader", last_batch_policy=last_batch_policy,
+                      glob="It seems that there is no data in the pipeline. This may happen if `last_batch_policy` is set to PARTIAL and the requested batch size is greater than the shard size.")


I don't think we should put verbatim copies of the entire message here - just enough to make sure it's the error we expect - like:

Suggested change

glob="It seems that there is no data in the pipeline. This may happen if `last_batch_policy` is set to PARTIAL and the requested batch size is greater than the shard size.")

glob="It seems that there is no data in the pipeline*last_batch_policy*")

mzient · 2021-09-06T12:42:40Z

dali/python/nvidia/dali/plugin/mxnet.py

+    assert dali_type == arr.dtype, ("Type of DALI Tensor/TensorList"
+            " doesn't match MXNet tensor type: {} vs {}".format(dali_type, np.dtype(arr.dtype)))


Suggested change

assert dali_type == arr.dtype, ("Type of DALI Tensor/TensorList"

" doesn't match MXNet tensor type: {} vs {}".format(dali_type, np.dtype(arr.dtype)))

assert dali_type == arr.dtype, ("The element type of DALI Tensor/TensorList"

" doesn't match the element type of the target MXNet NDArray: {} vs {}".format(dali_type, np.dtype(arr.dtype)))

mzient · 2021-09-06T12:43:25Z

dali/python/nvidia/dali/plugin/pytorch.py

+    assert to_torch_type[dali_type] == arr.dtype, ("Type of DALI Tensor/TensorList"
+            " doesn't match Torch tensor type: {} vs {}".format(to_torch_type[dali_type], arr.dtype))


Suggested change

assert to_torch_type[dali_type] == arr.dtype, ("Type of DALI Tensor/TensorList"

" doesn't match Torch tensor type: {} vs {}".format(to_torch_type[dali_type], arr.dtype))

assert to_torch_type[dali_type] == arr.dtype, ("The element type of DALI Tensor/TensorList"

" doesn't match the element type of the target PyTorch Tensor: {} vs {}".format(to_torch_type[dali_type], arr.dtype))

mzient · 2021-09-06T12:44:23Z

dali/test/python/test_fw_iterators.py

+    pipe.build()
+    out = pipe.run()[0]
+    mxnet_tensor = mxnet.nd.empty([1], None, np.int8)
+    assert_raises(AssertionError, feed_ndarray, out, mxnet_tensor, glob="Type of DALI Tensor/TensorList doesn't match MXNet tensor type:")


Update the pattern here if you update the message as suggested.

Signed-off-by: Janusz Lisiecki <jlisiecki@nvidia.com>

JanuszL · 2021-09-06T17:16:04Z

!build

dali-automaton · 2021-09-06T17:20:54Z

CI MESSAGE: [2925593]: BUILD STARTED

dali-automaton · 2021-09-06T21:38:47Z

CI MESSAGE: [2925593]: BUILD PASSED

klecki assigned mzient and stiepan Sep 3, 2021

JanuszL commented Sep 3, 2021

View reviewed changes

stiepan reviewed Sep 3, 2021

View reviewed changes

stiepan approved these changes Sep 3, 2021

View reviewed changes

JanuszL marked this pull request as draft September 4, 2021 07:27

JanuszL marked this pull request as ready for review September 6, 2021 09:45

mzient reviewed Sep 6, 2021

View reviewed changes

Review fix

2cacd47

Signed-off-by: Janusz Lisiecki <jlisiecki@nvidia.com>

JanuszL force-pushed the feed_ndarray_validation branch from 8a5dfef to 2cacd47 Compare September 6, 2021 17:15

mzient approved these changes Sep 6, 2021

View reviewed changes

JanuszL merged commit a49640d into NVIDIA:main Sep 6, 2021

JanuszL deleted the feed_ndarray_validation branch September 6, 2021 21:50

JanuszL restored the feed_ndarray_validation branch September 6, 2021 21:50

JanuszL deleted the feed_ndarray_validation branch September 6, 2021 21:50

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add input type validation to feed_ndarray in MXNet and PyTorch #3308

Add input type validation to feed_ndarray in MXNet and PyTorch #3308

JanuszL commented Sep 2, 2021 •

edited

Loading

JanuszL commented Sep 2, 2021

dali-automaton commented Sep 2, 2021

dali-automaton commented Sep 2, 2021

JanuszL Sep 3, 2021

stiepan Sep 3, 2021

JanuszL Sep 3, 2021

JanuszL commented Sep 3, 2021

dali-automaton commented Sep 3, 2021

dali-automaton commented Sep 3, 2021

dali-automaton commented Sep 3, 2021

mzient Sep 6, 2021

JanuszL Sep 6, 2021

mzient Sep 6, 2021 •

edited

Loading

JanuszL Sep 6, 2021

mzient Sep 6, 2021

JanuszL Sep 6, 2021

mzient Sep 6, 2021

JanuszL Sep 6, 2021

JanuszL commented Sep 6, 2021

dali-automaton commented Sep 6, 2021

dali-automaton commented Sep 6, 2021

	glob="It seems that there is no data in the pipeline. This may happen if `last_batch_policy` is set to PARTIAL and the requested batch size is greater than the shard size.")
	glob="It seems that there is no data in the pipelinelast_batch_policy")

		assert dali_type == arr.dtype, ("Type of DALI Tensor/TensorList"
		" doesn't match MXNet tensor type: {} vs {}".format(dali_type, np.dtype(arr.dtype)))

		assert to_torch_type[dali_type] == arr.dtype, ("Type of DALI Tensor/TensorList"
		" doesn't match Torch tensor type: {} vs {}".format(to_torch_type[dali_type], arr.dtype))

Add input type validation to feed_ndarray in MXNet and PyTorch #3308

Add input type validation to feed_ndarray in MXNet and PyTorch #3308

Conversation

JanuszL commented Sep 2, 2021 • edited Loading

Description

What happened in this PR

Additional information

Checklist

Tests

Documentation

DALI team only

Requirements

JanuszL commented Sep 2, 2021

dali-automaton commented Sep 2, 2021

dali-automaton commented Sep 2, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

JanuszL commented Sep 3, 2021

dali-automaton commented Sep 3, 2021

dali-automaton commented Sep 3, 2021

dali-automaton commented Sep 3, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mzient Sep 6, 2021 • edited Loading

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

JanuszL commented Sep 6, 2021

dali-automaton commented Sep 6, 2021

dali-automaton commented Sep 6, 2021

JanuszL commented Sep 2, 2021 •

edited

Loading

mzient Sep 6, 2021 •

edited

Loading