Add error message checks #3371

banasraf · 2021-09-24T09:30:47Z

Signed-off-by: Rafal Banas.Rafal97@gmail.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

Adding error message checks + some test fixes

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

Signed-off-by: Rafal <Banas.Rafal97@gmail.com>

banasraf · 2021-09-24T09:34:00Z

dali/test/python/test_dali_tf_dataset_eager.py

-    # source provided, so we don't have valid placeholder
-    yield check_disallowed_es, {'name': 'a', 'source': []}, {'a': in_dataset}
-    # misnamed placeholder
-    yield check_disallowed_es, {'name': 'b'}, {'a': in_dataset}


This one is already covered on L260

JanuszL · 2021-09-24T10:06:53Z

dali/test/python/test_dali_tf_dataset_eager.py


    pipeline = get_image_pipeline(batch_size, num_threads, 'gpu', 0)

    with tf.device('/cpu:0'):
-        dataset = get_dali_dataset_from_pipeline(
-            pipeline, batch_size, num_threads, 'cpu', 0)
+        dataset = get_dali_dataset_from_pipeline(pipeline, 'cpu', 0)


JanuszL · 2021-09-24T10:07:41Z

dali/test/python/test_dali_tf_dataset_eager.py


 @pipeline_def(batch_size=10, num_threads=4, device_id=0)
 def es_pipe(kwargs):
    return fn.external_source(**kwargs)


-@raises(Exception)
-def check_disallowed_es(kwargs, input_datasets):
+# @raises(Exception)


leftover - removed

JanuszL · 2021-09-24T10:09:38Z

dali/test/python/test_dali_tf_dataset_eager.py

@@ -35,7 +37,10 @@ def test_tf_dataset_cpu():
    run_tf_dataset_eager_mode('cpu')

 # Return differently sized images to check if DALIDataset can handle this case gracefully
-@raises(tf.errors.FailedPreconditionError)
+@raises(tf.errors.FailedPreconditionError, glob='*Batch output at index * from DALI pipeline is not uniform - individual '


You can remove leading and trailing * from glob pattern. They are not needed, as glob to regexp converted we use adds them automatically.

You can skip most of the message and only match relevant fragments, making some slack for future adjustment of the error messages.

I removed all the leading and trailing * and shortened the patterns to the essential parts.

JanuszL · 2021-09-24T10:10:55Z

dali/test/python/test_functional_api.py

 def test_set_outputs():
    data = [[[np.random.rand(1, 3, 2)], [np.random.rand(1, 4, 5)]]]
    pipe = Pipeline(batch_size = 1, num_threads = 1, device_id = None)
    pipe.set_outputs(fn.external_source(data, num_outputs = 2, cycle = 'quiet'))
-    assert_raises(TypeError, pipe.build)
+    pipe.build()


I would stick to checking if the build step raises that error, not any other one.

JanuszL · 2021-09-24T10:11:29Z

dali/test/python/test_operator_pad.py

@@ -12,6 +12,7 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.

+from nose.tools.nontrivial import raises


Please remove. That's probably VSCode's doing - it keeps doing it to me all the time :\

JanuszL · 2021-09-24T10:11:58Z

dali/test/python/test_operator_pad.py

@@ -150,7 +151,7 @@ def test_pad_error():
                                align=align, shape_arg=shape_arg, layout=layout)

    pipe.build()
-    assert_raises(RuntimeError, pipe.run)
+    pipe.run()


I would keep this check here.

JanuszL · 2021-09-24T10:12:30Z

dali/test/python/test_video_pipeline.py

 def test_wrong_length_sequence_videopipeline():
    pipe = VideoPipe(batch_size=BATCH_SIZE, data=VIDEO_FILES, sequence_length=100000)
-    assert_raises(RuntimeError, pipe.build)
+    pipe.build()


Can we keep this check here?

JanuszL · 2021-09-24T10:14:19Z

dali/test/python/test_video_pipeline.py

+    list_file.close()
+
+    pipe = VideoPipeList(batch_size=BATCH_SIZE, data=list_file.name)
+    with assert_raises(RuntimeError, glob='*No files were read'):


I would improve video_loader error message in this case as we discussed offline.

As we agreed, the warning about empty range, that is already there, should be enough

JanuszL · 2021-09-24T10:14:39Z

dali/test/python/test_video_pipeline.py

+        yield _test_file_list_invalid_range, r[0], r[1]
+
+
+def test_file_list_empty():


Suggested change

def test_file_list_empty():

def test_file_list_empty_range():

mzient · 2021-09-24T13:08:30Z

dali/test/python/test_dali_tf_dataset_eager.py

+@raises(TypeError, glob='*`input_datasets` must be a dictionary that maps input names (the `name` specified for External '
+                        'Source node in DALI pipeline) to input datasets objects (`tf.data.Dataset`) or '
+                        '`nvidia.dali.plugin.tf.experimental.Input` wrapper objects*')


Likewise, you don't have to put the entire message here - and you can skip leading and trailing *.

I shortened the patterns to the essential part

Signed-off-by: Rafal <Banas.Rafal97@gmail.com>

banasraf · 2021-09-27T09:36:52Z

!build

dali-automaton · 2021-09-27T09:41:30Z

CI MESSAGE: [3063756]: BUILD STARTED

dali-automaton · 2021-09-27T11:07:00Z

CI MESSAGE: [3063756]: BUILD FAILED

dali-automaton · 2021-09-27T11:52:31Z

CI MESSAGE: [3063756]: BUILD PASSED

Add error message checks and fix some TF tests. Signed-off-by: Rafal <Banas.Rafal97@gmail.com>

Add error message checks

63bf744

Signed-off-by: Rafal <Banas.Rafal97@gmail.com>

banasraf commented Sep 24, 2021

View reviewed changes

jantonguirao assigned JanuszL and awolant Sep 24, 2021

JanuszL reviewed Sep 24, 2021

View reviewed changes

mzient reviewed Sep 24, 2021

View reviewed changes

Review fixes. Compresses glob patterns.

13ce924

Signed-off-by: Rafal <Banas.Rafal97@gmail.com>

JanuszL approved these changes Sep 27, 2021

View reviewed changes

awolant approved these changes Sep 27, 2021

View reviewed changes

banasraf merged commit 520e3ab into NVIDIA:main Sep 27, 2021

cyyever pushed a commit to cyyever/DALI that referenced this pull request Oct 17, 2021

Add error message checks (NVIDIA#3371)

3aa7f82

Add error message checks and fix some TF tests. Signed-off-by: Rafal <Banas.Rafal97@gmail.com>

cyyever pushed a commit to cyyever/DALI that referenced this pull request Jan 23, 2022

Add error message checks (NVIDIA#3371)

9c11cee

Add error message checks and fix some TF tests. Signed-off-by: Rafal <Banas.Rafal97@gmail.com>

cyyever pushed a commit to cyyever/DALI that referenced this pull request Jan 23, 2022

Add error message checks (NVIDIA#3371)

ee0a4e6

Add error message checks and fix some TF tests. Signed-off-by: Rafal <Banas.Rafal97@gmail.com>

cyyever pushed a commit to cyyever/DALI that referenced this pull request Jan 23, 2022

Add error message checks (NVIDIA#3371)

24976ab

Add error message checks and fix some TF tests. Signed-off-by: Rafal <Banas.Rafal97@gmail.com>

cyyever pushed a commit to cyyever/DALI that referenced this pull request Jan 23, 2022

Add error message checks (NVIDIA#3371)

cf08acb

Add error message checks and fix some TF tests. Signed-off-by: Rafal <Banas.Rafal97@gmail.com>

cyyever pushed a commit to cyyever/DALI that referenced this pull request Jan 23, 2022

Add error message checks (NVIDIA#3371)

cc3c9b7

Add error message checks and fix some TF tests. Signed-off-by: Rafal <Banas.Rafal97@gmail.com>

cyyever pushed a commit to cyyever/DALI that referenced this pull request Jan 23, 2022

Add error message checks (NVIDIA#3371)

6ed9133

Add error message checks and fix some TF tests. Signed-off-by: Rafal <Banas.Rafal97@gmail.com>

cyyever pushed a commit to cyyever/DALI that referenced this pull request Jan 23, 2022

Add error message checks (NVIDIA#3371)

da02bed

Add error message checks and fix some TF tests. Signed-off-by: Rafal <Banas.Rafal97@gmail.com>

cyyever pushed a commit to cyyever/DALI that referenced this pull request Jan 23, 2022

Add error message checks (NVIDIA#3371)

ac49580

Add error message checks and fix some TF tests. Signed-off-by: Rafal <Banas.Rafal97@gmail.com>

cyyever pushed a commit to cyyever/DALI that referenced this pull request Jan 23, 2022

Add error message checks (NVIDIA#3371)

3b7874a

Add error message checks and fix some TF tests. Signed-off-by: Rafal <Banas.Rafal97@gmail.com>

cyyever pushed a commit to cyyever/DALI that referenced this pull request Jan 23, 2022

Add error message checks (NVIDIA#3371)

f770cdb

Add error message checks and fix some TF tests. Signed-off-by: Rafal <Banas.Rafal97@gmail.com>

cyyever pushed a commit to cyyever/DALI that referenced this pull request Jan 23, 2022

Add error message checks (NVIDIA#3371)

ec86ffc

Add error message checks and fix some TF tests. Signed-off-by: Rafal <Banas.Rafal97@gmail.com>

cyyever pushed a commit to cyyever/DALI that referenced this pull request Jan 23, 2022

Add error message checks (NVIDIA#3371)

9b50687

Add error message checks and fix some TF tests. Signed-off-by: Rafal <Banas.Rafal97@gmail.com>

cyyever pushed a commit to cyyever/DALI that referenced this pull request Jan 23, 2022

Add error message checks (NVIDIA#3371)

d9f5f62

Add error message checks and fix some TF tests. Signed-off-by: Rafal <Banas.Rafal97@gmail.com>

cyyever pushed a commit to cyyever/DALI that referenced this pull request Feb 21, 2022

Add error message checks (NVIDIA#3371)

d52c8bc

Add error message checks and fix some TF tests. Signed-off-by: Rafal <Banas.Rafal97@gmail.com>

cyyever pushed a commit to cyyever/DALI that referenced this pull request May 13, 2022

Add error message checks (NVIDIA#3371)

81a9c88

Add error message checks and fix some TF tests. Signed-off-by: Rafal <Banas.Rafal97@gmail.com>

cyyever pushed a commit to cyyever/DALI that referenced this pull request Jun 7, 2022

Add error message checks (NVIDIA#3371)

5e657c6

Add error message checks and fix some TF tests. Signed-off-by: Rafal <Banas.Rafal97@gmail.com>

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add error message checks #3371

Add error message checks #3371

banasraf commented Sep 24, 2021

banasraf Sep 24, 2021

JanuszL Sep 24, 2021

JanuszL Sep 24, 2021

banasraf Sep 27, 2021

JanuszL Sep 24, 2021

mzient Sep 24, 2021

banasraf Sep 27, 2021

JanuszL Sep 24, 2021

banasraf Sep 27, 2021

JanuszL Sep 24, 2021

mzient Sep 24, 2021

banasraf Sep 27, 2021

JanuszL Sep 24, 2021

banasraf Sep 27, 2021

JanuszL Sep 24, 2021

banasraf Sep 27, 2021

JanuszL Sep 24, 2021

banasraf Sep 27, 2021

JanuszL Sep 24, 2021

banasraf Sep 27, 2021

mzient Sep 24, 2021

banasraf Sep 27, 2021

banasraf commented Sep 27, 2021

dali-automaton commented Sep 27, 2021

dali-automaton commented Sep 27, 2021

dali-automaton commented Sep 27, 2021

		yield _test_file_list_invalid_range, r[0], r[1]


		def test_file_list_empty():

	def test_file_list_empty():
	def test_file_list_empty_range():

Add error message checks #3371

Add error message checks #3371

Conversation

banasraf commented Sep 24, 2021

Description

What happened in this PR

Additional information

Checklist

Tests

Documentation

DALI team only

Requirements

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

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

banasraf commented Sep 27, 2021

dali-automaton commented Sep 27, 2021

dali-automaton commented Sep 27, 2021

dali-automaton commented Sep 27, 2021