Update asserts in python tests #3336

awolant · 2021-09-13T12:41:30Z

Signed-off-by: Albert Wolant awolant@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

Additional information

Affected modules and functionalities:
Python tests
Key points relevant for the review:
Are the assertions correct

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: Albert Wolant <awolant@nvidia.com>

mzient · 2021-09-14T06:47:30Z

dali/test/python/test_operator_readers_coco.py

-@raises(RuntimeError)
+@raises(
+    RuntimeError,
+    glob="Assert on \"HasArgument(name)\" failed: Argument \"preprocessed_annotations_dir\" is not supported by operator \"readers__COCO\".")


Don't include assertion condition in the message.

I remember that on one meeting there was a conclusion that putting entire error messages verbatim is excessive. Here, I'd suggest putting a wildcard in place of __, which we hope to replace with proper module name at some point.

Nitpick: use single quotes, so you don't have to escape the double quotes - it keeps the pattern more readable.

Nitpick: glob can be specified with a positional argument - keeps the line shorter

Suggested change

glob="Assert on \"HasArgument(name)\" failed: Argument \"preprocessed_annotations_dir\" is not supported by operator \"readers__COCO\".")

'Argument "preprocessed_annotations_dir" is not supported by operator *readers*COCO')

mzient · 2021-09-14T06:48:37Z

dali/test/python/test_operator_readers_coco.py

@@ -170,4 +172,4 @@ def test_coco_reader_alias():
    for polygon_masks, pixelwise_masks in [(None, None), (True, None), (None, True)]:
        new_pipe = coco_pipe(fn.readers.coco, file_root, train_annotations, polygon_masks, pixelwise_masks)
        legacy_pipe = coco_pipe(fn.coco_reader, file_root, train_annotations, polygon_masks, pixelwise_masks)
-        compare_pipelines(new_pipe, legacy_pipe, batch_size_alias_test, 50)
+        compare_pipelines(new_pipe, legacy_pipe, batch_size_alias_test, 5)


Misclick

Signed-off-by: Albert Wolant <awolant@nvidia.com>

awolant · 2021-09-14T14:23:49Z

!build

dali-automaton · 2021-09-14T14:25:59Z

CI MESSAGE: [2978636]: BUILD STARTED

dali-automaton · 2021-09-14T22:29:21Z

CI MESSAGE: [2978636]: BUILD PASSED

stiepan · 2021-09-15T12:35:32Z

dali/test/python/test_operator_sequence_rearrange.py

-                    ([np.int32([0]), np.int32([])], True), ([np.int32([6, 7]), np.int32([0])], True),
-                    ([np.int32([-1]), np.int32([0])], True), ([np.int32([[1], [2]]), np.int32([[1], [2]])], True)]:
-                yield check_fail_sequence_rearrange, 2, shape, new_order, per_sample, dev
+        for args, error_msg in zip(orders, error_msgs):


That might be overdoing things but I thought about adding assert on equal lengths of both lists before looping over zip(orders, error_msgs) just in case somebody adds a case to orders in the future and misses the fact that the change should be reflected in error_msgs as zip just happily ignores rest of the longer list.

mzient · 2021-09-15T13:33:13Z

dali/test/python/test_dali_cpu_only.py

+    assert_raises(
+        RuntimeError,
+        pipe.build,
+        glob="Cannot make a gpu output for __ExternalSource_0 operator, device_id should not be equal CPU_ONLY_DEVICE_ID.")


This message doesn't sport the most beautiful English. Indeed, I don't even know what it means and what could be understood by the user.
There are other problems with this message - for once, the name used in the message is not the name of the operator/producer, but the name of the output (a DataNode).
Here's a possible alternative, but I'm not very attached to it (I think it's better, but it probably could be improved further).

Suggested change

glob="Cannot make a gpu output for __ExternalSource_0 operator, device_id should not be equal CPU_ONLY_DEVICE_ID.")

glob='Cannot move the data node \"__ExternalSource_0\" to the GPU in a CPU-only pipeline.'

'The `device_id` parameter is set tot `CPU_ONLY_DEVICE_ID`. Set `device_id` to a valid GPU identifier to enable GPU features in the pipeline. ")

mzient · 2021-09-15T13:37:07Z

dali/test/python/test_operator_sequence_rearrange.py


 def test_wrong_layouts_sequence_rearrange():
    shape = [5, 1]
    new_order = [0, 2, 1, 3, 4]
    per_sample = False
    for dev in ["cpu", "gpu"]:
        for layout in ["HF", "HW"]:
-            yield check_fail_sequence_rearrange, 5, shape, new_order, per_sample, dev, layout
+            yield raises(RuntimeError, glob="Expected sequence as the input, where outermost dimension represents frames dimension `F`")(check_fail_sequence_rearrange), 5, shape, new_order, per_sample, dev, layout


That's also a bit cryptic to an outsider (maybe mentioning layout would help?).

Signed-off-by: Albert Wolant <awolant@nvidia.com>

awolant · 2021-09-16T06:53:15Z

!build

dali-automaton · 2021-09-16T06:56:06Z

CI MESSAGE: [2992705]: BUILD STARTED

dali-automaton · 2021-09-16T08:02:12Z

CI MESSAGE: [2992705]: BUILD PASSED

* Update asserts in python tests Signed-off-by: Albert Wolant <awolant@nvidia.com>

Update readers.coco tests

14f364d

Signed-off-by: Albert Wolant <awolant@nvidia.com>

awolant changed the title ~~Update readers.coco tests~~ Update asserts in python tests Sep 13, 2021

mzient reviewed Sep 14, 2021

View reviewed changes

mzient previously approved these changes Sep 14, 2021

View reviewed changes

mzient self-requested a review September 14, 2021 09:14

mzient marked this pull request as ready for review September 14, 2021 09:15

mzient marked this pull request as draft September 14, 2021 09:16

awolant added 3 commits September 14, 2021 15:24

More fixes

35a1956

Signed-off-by: Albert Wolant <awolant@nvidia.com>

Fix sequence rearrange test

3bdde5f

Signed-off-by: Albert Wolant <awolant@nvidia.com>

Fix date

fefbe3f

Signed-off-by: Albert Wolant <awolant@nvidia.com>

awolant marked this pull request as ready for review September 14, 2021 14:14

Merge remote-tracking branch 'nvidia/main' into fix_asserts

a179eab

klecki assigned mzient and stiepan Sep 15, 2021

stiepan reviewed Sep 15, 2021

View reviewed changes

stiepan approved these changes Sep 15, 2021

View reviewed changes

mzient reviewed Sep 15, 2021

View reviewed changes

Fix review comments

889cf29

Signed-off-by: Albert Wolant <awolant@nvidia.com>

mzient approved these changes Sep 16, 2021

View reviewed changes

awolant merged commit 4814867 into NVIDIA:main Sep 16, 2021

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

Update asserts in python tests (NVIDIA#3336)

5391028

* Update asserts in python tests Signed-off-by: Albert Wolant <awolant@nvidia.com>

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

Update asserts in python tests (NVIDIA#3336)

9d0c927

* Update asserts in python tests Signed-off-by: Albert Wolant <awolant@nvidia.com>

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

Update asserts in python tests (NVIDIA#3336)

57dda75

* Update asserts in python tests Signed-off-by: Albert Wolant <awolant@nvidia.com>

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

Update asserts in python tests (NVIDIA#3336)

7bf6b48

* Update asserts in python tests Signed-off-by: Albert Wolant <awolant@nvidia.com>

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

Update asserts in python tests (NVIDIA#3336)

37d72f5

* Update asserts in python tests Signed-off-by: Albert Wolant <awolant@nvidia.com>

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

Update asserts in python tests (NVIDIA#3336)

0ff84f8

* Update asserts in python tests Signed-off-by: Albert Wolant <awolant@nvidia.com>

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

Update asserts in python tests (NVIDIA#3336)

dfa9ada

* Update asserts in python tests Signed-off-by: Albert Wolant <awolant@nvidia.com>

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

Update asserts in python tests (NVIDIA#3336)

3d81c60

* Update asserts in python tests Signed-off-by: Albert Wolant <awolant@nvidia.com>

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

Update asserts in python tests (NVIDIA#3336)

e27e5c4

* Update asserts in python tests Signed-off-by: Albert Wolant <awolant@nvidia.com>

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

Update asserts in python tests (NVIDIA#3336)

0a7fbe2

* Update asserts in python tests Signed-off-by: Albert Wolant <awolant@nvidia.com>

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

Update asserts in python tests (NVIDIA#3336)

8f5b1ae

* Update asserts in python tests Signed-off-by: Albert Wolant <awolant@nvidia.com>

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

Update asserts in python tests (NVIDIA#3336)

79c7f26

* Update asserts in python tests Signed-off-by: Albert Wolant <awolant@nvidia.com>

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

Update asserts in python tests (NVIDIA#3336)

6d4c8fd

* Update asserts in python tests Signed-off-by: Albert Wolant <awolant@nvidia.com>

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

Update asserts in python tests (NVIDIA#3336)

2f95cef

* Update asserts in python tests Signed-off-by: Albert Wolant <awolant@nvidia.com>

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

Update asserts in python tests (NVIDIA#3336)

4d05e6c

* Update asserts in python tests Signed-off-by: Albert Wolant <awolant@nvidia.com>

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

Update asserts in python tests (NVIDIA#3336)

e21a64b

* Update asserts in python tests Signed-off-by: Albert Wolant <awolant@nvidia.com>

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

Update asserts in python tests (NVIDIA#3336)

f250f65

* Update asserts in python tests Signed-off-by: Albert Wolant <awolant@nvidia.com>

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update asserts in python tests #3336

Update asserts in python tests #3336

awolant commented Sep 13, 2021 •

edited

Loading

mzient Sep 14, 2021 •

edited

Loading

awolant Sep 14, 2021

mzient Sep 14, 2021

awolant commented Sep 14, 2021

dali-automaton commented Sep 14, 2021

dali-automaton commented Sep 14, 2021

stiepan Sep 15, 2021 •

edited

Loading

awolant Sep 16, 2021

mzient Sep 15, 2021 •

edited

Loading

awolant Sep 16, 2021

mzient Sep 15, 2021

awolant Sep 16, 2021

awolant commented Sep 16, 2021

dali-automaton commented Sep 16, 2021

dali-automaton commented Sep 16, 2021

	glob="Assert on \"HasArgument(name)\" failed: Argument \"preprocessed_annotations_dir\" is not supported by operator \"readers__COCO\".")
	'Argument "preprocessed_annotations_dir" is not supported by operator readersCOCO')

	glob="Cannot make a gpu output for __ExternalSource_0 operator, device_id should not be equal CPU_ONLY_DEVICE_ID.")
	glob='Cannot move the data node \"__ExternalSource_0\" to the GPU in a CPU-only pipeline.'
	'The `device_id` parameter is set tot `CPU_ONLY_DEVICE_ID`. Set `device_id` to a valid GPU identifier to enable GPU features in the pipeline. ")

Update asserts in python tests #3336

Update asserts in python tests #3336

Conversation

awolant commented Sep 13, 2021 • edited Loading

Description

What happened in this PR

Additional information

Checklist

Tests

Documentation

DALI team only

Requirements

mzient Sep 14, 2021 • edited Loading

Choose a reason for hiding this comment

awolant Sep 14, 2021

Choose a reason for hiding this comment

mzient Sep 14, 2021

Choose a reason for hiding this comment

awolant commented Sep 14, 2021

dali-automaton commented Sep 14, 2021

dali-automaton commented Sep 14, 2021

stiepan Sep 15, 2021 • edited Loading

Choose a reason for hiding this comment

awolant Sep 16, 2021

Choose a reason for hiding this comment

mzient Sep 15, 2021 • edited Loading

Choose a reason for hiding this comment

awolant Sep 16, 2021

Choose a reason for hiding this comment

mzient Sep 15, 2021

Choose a reason for hiding this comment

awolant Sep 16, 2021

Choose a reason for hiding this comment

awolant commented Sep 16, 2021

dali-automaton commented Sep 16, 2021

dali-automaton commented Sep 16, 2021

awolant commented Sep 13, 2021 •

edited

Loading

mzient Sep 14, 2021 •

edited

Loading

stiepan Sep 15, 2021 •

edited

Loading

mzient Sep 15, 2021 •

edited

Loading