Promote scalars to constants in ndd arithmetic operators - #6475
Conversation
Signed-off-by: Rostan Tabet <rtabet@nvidia.com>
Signed-off-by: Rostan Tabet <rtabet@nvidia.com>
|
!build |
|
CI MESSAGE: [66962649]: BUILD STARTED |
|
CI MESSAGE: [66962649]: BUILD PASSED |
| integers.append(int(arg)) | ||
| elif type_ is int: | ||
| desc.append(f"${len(integers)}:int32") | ||
| integers.append(arg) |
There was a problem hiding this comment.
[Critical] Python int is unbounded, but this gets packed into integer_constants, declared as std::vector<int32_t> in arithmetic.cc. Values outside the int32 range are silently wrapped, not rejected or promoted to int64.
Verified against the actual op (pre-existing _ArithmeticGenericOp, unaffected by this PR's C++ change): fn._arithmetic_generic_op(data, expression_desc="add(&0 $0:int32)", integer_constants=[5_000_000_000]) on a [1, 2, 3] input returns [705032705, 705032706, 705032707] — 5_000_000_000 mod 2**32, no error, no warning.
Before this PR, an int scalar went through as_tensor(arg), and np.array(5_000_000_000).dtype is int64 — so this is a real precision/correctness regression for any int64-range scalar (timestamps, large counts, hashes, etc.), not just a style change. Worth at least clamping/validating the range and raising, or falling back to a tensor input when the value doesn't fit in int32.
Category:
Bug fix (non-breaking change which fixes an issue)
Description:
Currently, integers and floats are not promoted to constants in ndd arithmetic operators. This PR fixes it.
With
aandbtwo 64x224x224x3 batches of uint8 images on the GPU,0.4 * a + 0.6 * btakes around 640 us to run on main and below 230 us on this branch.One important detail is that all bool/int/floats are currently promoted to constants, forcing the creation of a new operator instance each time. PR #6476 addresses this issue.
Additional information:
Affected modules and functionalities:
Airthmetic ops
Key points relevant for the review:
Tests:
test_arithm_ops.pyChecklist
Documentation
DALI team only
Requirements
REQ IDs: N/A
JIRA TASK: N/A