Skip to content

Conversation

@mzient
Copy link
Contributor

@mzient mzient commented Dec 5, 2025

Category:

Other (e.g. Documentation, Tests, Configuration)

Description:

This PR removed the old experimental "eager" mode and tests for it.
Some small parts still used by pipeline debug mode were moved to a different file.

Additional information:

Affected modules and functionalities:

DALI eager.

Key points relevant for the review:

Tests:

  • Existing tests apply
  • 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: Michał Zientkiewicz <[email protected]>
import nvidia.dali.types as _types
from nvidia.dali import _conditionals
from nvidia.dali._utils.eager_utils import _Classification, _transform_data_to_tensorlist
from nvidia.dali._debug_utils import _Classification, _transform_data_to_tensorlist

Check notice

Code scanning / CodeQL

Cyclic import Note

Import of module
nvidia.dali._debug_utils
begins an import cycle.
import nvidia.dali.tensors as _tensors
import nvidia.dali.types as _types

from nvidia.dali.external_source import _prep_data_for_feed_input

Check notice

Code scanning / CodeQL

Cyclic import Note

Import of module
nvidia.dali.external_source
begins an import cycle.
Import of module
external_source
begins an import cycle.
"""

def __init__(self, data, type_name, arg_constant_len=-1):
from nvidia.dali._debug_mode import DataNodeDebug

Check notice

Code scanning / CodeQL

Cyclic import Note

Import of module
nvidia.dali._debug_mode
begins an import cycle.
@staticmethod
def _classify_data(data, type_name, arg_constant_len):
"""Returns tuple (is_batch, device, unpacked data)."""
from nvidia.dali._debug_mode import DataNodeDebug

Check notice

Code scanning / CodeQL

Cyclic import Note

Import of module
nvidia.dali._debug_mode
begins an import cycle.
@greptile-apps
Copy link

greptile-apps bot commented Dec 5, 2025

Greptile Overview

Greptile Summary

This PR removes the old experimental eager mode API from DALI, deleting over 3,100 lines of code including Python modules, C++ bindings, and tests. The key changes:

  • Removed modules: nvidia.dali.experimental.eager, nvidia.dali._utils.eager_utils
  • Removed tests: test_eager_coverage.py, test_eager_operators.py (removed from test runner)
  • Removed C++ bindings: EagerOperator Python bindings, TensorList arithmetic operator overloads
  • Preserved functionality: EagerOperator class and header files remain because debug mode (PipelineDebug) still uses them internally
  • Refactoring: Extracted _Classification and _transform_data_to_tensorlist from eager_utils into new _debug_utils.py module for debug mode use

The refactoring is clean with no remaining references to removed code. The new _debug_utils.py module uses lazy imports (imports inside functions) to safely avoid circular dependencies with _debug_mode.py.

Confidence Score: 5/5

  • This PR is safe to merge - it's a clean removal of deprecated experimental code with proper preservation of debug mode functionality
  • The code removal is thorough and systematic with no remaining references to deleted modules. The refactoring properly extracts needed utilities for debug mode into a dedicated module. All tests for eager mode are removed from the test runner. The EagerOperator C++ class is correctly preserved for internal debug mode use.
  • No files require special attention

Important Files Changed

File Analysis

Filename Score Overview
dali/python/backend_impl.cc 5/5 Removes eager operator bindings and arithmetic operator overloads for TensorList - clean removal of obsolete code
dali/python/nvidia/dali/_debug_mode.py 5/5 Updates import to use new _debug_utils module instead of removed eager_utils - minimal change
dali/python/nvidia/dali/_debug_utils.py 5/5 New file extracting _Classification and _transform_data_to_tensorlist from removed eager_utils for debug mode use - uses lazy imports to avoid circular dependencies
dali/python/nvidia/dali/fn/init.py 5/5 Removes call to _wrap_eager_op from operator wrapping logic - clean removal

Sequence Diagram

sequenceDiagram
    participant User as User Code
    participant FnInit as fn/__init__.py
    participant DebugMode as _debug_mode.py
    participant DebugUtils as _debug_utils.py (NEW)
    participant Backend as backend_impl.cc
    participant EagerOp as EagerOperator (kept)
    
    Note over User,Backend: Before: Eager Mode Active
    User->>FnInit: Import operator
    FnInit->>Backend: Expose EagerOperator bindings
    FnInit->>FnInit: Call _wrap_eager_op()
    Backend->>Backend: Register TensorList arithmetic ops
    User->>User: Use experimental.eager API
    
    Note over User,Backend: After: Eager Mode Removed
    User->>FnInit: Import operator
    Note over FnInit: _wrap_eager_op() removed
    Note over Backend: EagerOperator bindings removed
    Note over Backend: TensorList arithmetic ops removed
    
    Note over User,EagerOp: Debug Mode Still Works
    User->>DebugMode: Use debug pipeline
    DebugMode->>DebugUtils: Import _Classification, _transform_data_to_tensorlist
    Note over DebugUtils: Functions extracted from eager_utils
    DebugMode->>EagerOp: Call EagerOperator (internal use)
    Note over EagerOp: Kept for debug mode
Loading

Copy link

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

10 files reviewed, no comments

Edit Code Review Agent Settings | Greptile

@dali-automaton
Copy link
Collaborator

CI MESSAGE: [39676940]: BUILD STARTED

Signed-off-by: Michał Zientkiewicz <[email protected]>
@@ -0,0 +1,147 @@
# Copyright (c) 2022-2025, NVIDIA CORPORATION & AFFILIATES. All rights reserved.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Copied from the removed _eager_utils.py

Copy link

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

10 files reviewed, no comments

Edit Code Review Agent Settings | Greptile

@dali-automaton
Copy link
Collaborator

CI MESSAGE: [39676940]: BUILD PASSED

@mzient mzient merged commit 887ff53 into NVIDIA:main Dec 8, 2025
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants