Skip to content

Extend static analysis for arithmetic operators - #6476

Open
rostan-t wants to merge 6 commits into
NVIDIA:mainfrom
rostan-t:ndd-arithm-ops-static-analysis
Open

Extend static analysis for arithmetic operators#6476
rostan-t wants to merge 6 commits into
NVIDIA:mainfrom
rostan-t:ndd-arithm-ops-static-analysis

Conversation

@rostan-t

@rostan-t rostan-t commented Sep 9, 2026

Copy link
Copy Markdown
Collaborator

Category:

New feature (non-breaking change which adds functionality)

Description:

This extends dynamic mode static analysis to support arithmetic operators.

PR #6475 promotes bools, ints, and floats to constants in arithmetic ops. With this PR, we can limit it to only those that are provably constant.

To prevent a regression on Python 3.10 when a call is on the same line as an arithmetic operation, we also add disambiguation on the bytecode since exact span checking is absent.

Additional information:

Affected modules and functionalities:

Dynamic mode.

Key points relevant for the review:

Commits a677329 and 0374268 belong to #6475.

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

Signed-off-by: Rostan Tabet <rtabet@nvidia.com>
Signed-off-by: Rostan Tabet <rtabet@nvidia.com>
Signed-off-by: Rostan Tabet <rtabet@nvidia.com>
Signed-off-by: Rostan Tabet <rtabet@nvidia.com>
Signed-off-by: Rostan Tabet <rtabet@nvidia.com>
@rostan-t rostan-t added the dynamic mode Related to dynamic mode label Sep 9, 2026
@rostan-t

rostan-t commented Sep 9, 2026

Copy link
Copy Markdown
Collaborator Author

!build

@greptile-apps

greptile-apps Bot commented Sep 9, 2026

Copy link
Copy Markdown
Contributor

RetriggerView in GreptileConfidence Score: 5/5

The PR appears safe to merge; the only outstanding concern from the previous review was fixed by strict operand-classification iteration.

Summary

  • Resolves the previous operand-truncation concern by enforcing equal-length iteration with zip(..., strict=True).
  • Adds arithmetic-dunder transparency for Tensor and Batch.
  • Generalizes call-site source analysis to calls, binary operations, and augmented assignments, including Python 3.10 bytecode disambiguation.
  • Passes classified integer and real constants through the arithmetic operator schema.

Diagram

%%{init: {'theme': 'neutral'}}%%
flowchart LR
    A[Python arithmetic expression] --> B[Transparent arithmetic dunder]
    B --> C[Resolve source site]
    C --> D[Classify operands]
    D -->|Provably constant| E[Integer or real expression constant]
    D -->|Dynamic| F[Tensor or Batch operator input]
    E --> G[Arithmetic expression operator]
    F --> G
Loading

Comment thread dali/python/nvidia/dali/experimental/dynamic/_op_builder.py
Comment thread dali/python/nvidia/dali/experimental/dynamic/_arithmetic.py Outdated
Comment thread dali/python/nvidia/dali/experimental/dynamic/math.py
@dali-automaton

Copy link
Copy Markdown
Collaborator

CI MESSAGE: [66962570]: BUILD STARTED

Signed-off-by: Rostan Tabet <rtabet@nvidia.com>
@rostan-t
rostan-t force-pushed the ndd-arithm-ops-static-analysis branch from 94b76d7 to e28c90a Compare September 9, 2026 13:11
@rostan-t

rostan-t commented Sep 9, 2026

Copy link
Copy Markdown
Collaborator Author

!build

@dali-automaton

Copy link
Copy Markdown
Collaborator

CI MESSAGE: [66975775]: BUILD STARTED

@mzient mzient self-assigned this Sep 9, 2026
@dali-automaton

Copy link
Copy Markdown
Collaborator

CI MESSAGE: [66975775]: BUILD PASSED

@dali-automaton

Copy link
Copy Markdown
Collaborator

CI MESSAGE: [66962570]: BUILD PASSED


# only reachable from math functions called with only scalars, e.g. ndd.math.max(2, 3)
if not tensor_args:
args = [to_input(arg) for arg in args]

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Minor] This unconditionally converts every scalar argument to a Tensor when there are no Tensor/Batch operands at all — which, per the comment above, is exactly the ndd.math.max(2, 3)-style call this PR's constant-promotion feature is meant to help. After this loop runs, every arg has become a Tensor, so the any(type(arg) in (bool, int, float) ...) check below is always False here and none of these arguments can be promoted to integer_constants/real_constants — the whole point of this PR is bypassed for the pure-scalar call path.

The reason for this branch is presumably .NumInput(1, 64) on the underlying op (verified in arithmetic.cc:146) requiring at least one real tensor input, which tensor_args can't guarantee when it's empty. But that only requires one of the arguments to stay a Tensor, not all of them — e.g. converting just args[0] before running constant classification on the rest would satisfy NumInput(1, 64) while still letting the remaining scalars take the $N path.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

dynamic mode Related to dynamic mode

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants