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

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.

3 participants