Promote scalars to constants in ndd arithmetic operators

[rostan-t]

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 a and b two 64x224x224x3 batches of uint8 images on the GPU, 0.4 * a + 0.6 * b takes 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:

• Existing tests apply: test_arithm_ops.py
• New tests added
  • Python tests
  • GTests
  • Benchmark
  • Other
• N/A

Checklist

Documentation

• Existing documentation applies
• Documentation updated
  • Docstring
  • Doxygen
  • RST
  • Jupyter
  • Other
• N/A

DALI team only

Requirements

• Implements new requirements
• Affects existing requirements
• N/A

REQ IDs: N/A

JIRA TASK: N/A

[greptile-apps]

Confidence Score: 5/5

The arithmetic behavior appears safe to merge, with only the non-blocking diagnostic-context issue from the existing review thread still outstanding.

Summary

• Encodes scalar operands through integer_constants and real_constants.
• Preserves tensor conversion for non-scalar operands and scalar-only math calls.
• Makes native arithmetic constant arguments non-tensor arguments.
• The latest revision adds the offending integer value to overflow diagnostics, but the existing review thread remains only partially addressed because the valid range is still omitted.

Diagram

%%{init: {'theme': 'neutral'}}%%
flowchart LR
  A[Python arithmetic arguments] --> B{Exact scalar type?}
  B -->|bool or int| C[integer_constants]
  B -->|float| D[real_constants]
  B -->|other| E[Tensor or Batch input]
  C --> F[Expression descriptor]
  D --> F
  E --> F
  F --> G[_ArithmeticGenericOp]

Loading

_{Reviews (4) · Last reviewed commit: "Make sure constant integers are fit in i..."}

[rostan-t]

!build

[dali-automaton]

CI MESSAGE: [66962649]: BUILD STARTED

[dali-automaton]

CI MESSAGE: [66962649]: BUILD PASSED

[jantonguirao]

[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.

[rostan-t]

Fixed by e409e87.

[rostan-t]

Note: this was a pre-existing issue since ndd.tensor itself wraps int32 integers. This is also the (documented) behavior of arithmetic operators in pipeline mode.
In dynamic mode, this is now rejected with a clear exception.

[rostan-t]

!build

[dali-automaton]

CI MESSAGE: [67345016]: BUILD STARTED

[dali-automaton]

CI MESSAGE: [67345016]: BUILD PASSED