Fix abs diff computation in check_batch test utility #4957
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
stiepan
changed the title
stiepan
force-pushed
the
fix_check_batch_abs
branch
from
e4c30b8
to
77cad6e
Compare
|
!build |
|
CI MESSAGE: [9150585]: BUILD STARTED |
|
!build |
|
CI MESSAGE: [9153258]: BUILD STARTED |
|
CI MESSAGE: [9153258]: BUILD FAILED |
|
CI MESSAGE: [9153258]: BUILD PASSED |
stiepan
commented
Aug 7, 2023
|
|
||
|
|
||
| def _check_absdiff(): | ||
| for i in range(-128, 127): |
There was a problem hiding this comment.
In principle, overflow on signed int is UB (that we rely on anyway without this PR). The following one-time check aims to verify it works as expected.
There was a problem hiding this comment.
Maybe we should explain it in a comment in the code rather than in the PR?
awolant
approved these changes
Aug 8, 2023
klecki
reviewed
Aug 18, 2023
dali/test/python/test_utils.py
Outdated
| @@ -128,6 +129,51 @@ def get_gpu_num(): | |||
| return len(out_list) | |||
|
|
|||
|
|
|||
| def _get_absdiff(left, right): | |||
|
|
|||
| def make_signed(dtype): | |||
klecki
approved these changes
Aug 18, 2023
|
|
||
|
|
||
| def _check_absdiff(): | ||
| for i in range(-128, 127): |
There was a problem hiding this comment.
Maybe we should explain it in a comment in the code rather than in the PR?
Signed-off-by: Kamil Tokarski <ktokarski@nvidia.com>
Signed-off-by: Kamil Tokarski <ktokarski@nvidia.com>
Signed-off-by: Kamil Tokarski <ktokarski@nvidia.com>
stiepan
force-pushed
the
fix_check_batch_abs
branch
from
d652935
to
ce41e78
Compare
|
!build |
|
CI MESSAGE: [9451724]: BUILD STARTED |
|
CI MESSAGE: [9451724]: BUILD PASSED |
JanuszL
pushed a commit
to JanuszL/DALI
that referenced
this pull request
Oct 13, 2023
* Fix check_batch abs diff computation * Add one-time sanity check --------- Signed-off-by: Kamil Tokarski <ktokarski@nvidia.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Category:
Other (e.g. Documentation, Tests, Configuration)
Description:
The
check_batchutility reports wrong abs differences for unsigned types if the actual difference is bigger than the half of the range of the type. (Take uint8 and 1 vs 254 - the abs diff is 253 while the chekc_batch would report 3).Additional information:
Affected modules and functionalities:
Key points relevant for the review:
Tests:
Checklist
Documentation
DALI team only
Requirements
REQ IDs: N/A
JIRA TASK: N/A