Fill gaps in dynamic mode documentation - Tensor, Batch and Readers.

[mzient]

Category:

Refactoring (Redesign of existing code that doesn't affect functionality)
Other (e.g. Documentation, Tests, Configuration)

Description:

This PR extends the documentation of Dynamic Mode.

It also hides the assign function in Tensor/Batch and makes re-evaluation of batches cheaper by switching nesting order.
The unused data property of the Tensor class has been removed (it's no longer necessary as we have __array__ and __cuda_array_interface__ properties).

Additional information:

Some unrelated typos in comments and documentation were fixed as they were identical to the ones I made in this PR.

Affected modules and functionalities:

Key points relevant for the review:

Tests:

Tensor and Batch 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

[greptile-apps]

Greptile Overview

Greptile Summary

This PR enhances the Dynamic Mode documentation by adding comprehensive docstrings to Tensor, Batch, and Reader classes. The PR also includes minor refactoring improvements.

Key Changes:

• Documentation: Added detailed class-level and method-level docstrings for Tensor and Batch classes, explaining their purpose, properties, and methods. Added docstrings for Reader.run() and Reader.next_epoch() methods.
• API Cleanup: Renamed assign() to _assign() in both Tensor and Batch classes to make it private, as it was an internal implementation detail not meant for public use.
• Performance Optimization: Modified Batch.evaluate() and Tensor.evaluate() to check if self._backend is None before entering the EvalContext context manager, avoiding unnecessary context creation when the batch/tensor is already evaluated.
• Code Cleanup: Removed unused data property from Tensor class.

Issues Found:

• 5 spelling/grammar typos in the new docstrings that should be corrected

Confidence Score: 4/5

• This PR is safe to merge after fixing the typos in docstrings
• The changes are primarily documentation additions with minor refactoring. The logic changes (privatizing assign method and reordering context manager checks) are safe improvements. The only issues are spelling/grammar typos in documentation that should be fixed before merge. Existing tests apply according to PR description.
• All files need minor typo corrections in docstrings before merge

Important Files Changed

File Analysis

Filename	Score	Overview
dali/python/nvidia/dali/experimental/dynamic/_batch.py	4/5	Added comprehensive docstrings for Batch class and methods; renamed assign to _assign (privatized); optimized evaluate() by checking _backend before entering context manager; contains 3 typos in docstrings
dali/python/nvidia/dali/experimental/dynamic/_tensor.py	4/5	Added comprehensive docstrings for Tensor class and properties; renamed assign to _assign (privatized); removed unused data property; contains 2 typos in docstrings
dali/python/nvidia/dali/experimental/dynamic/ops.py	5/5	Added docstrings to Reader.run() and Reader.next_epoch() methods; no logic changes, documentation only

Sequence Diagram

sequenceDiagram
    participant User
    participant Batch
    participant EvalContext
    participant Backend

    Note over Batch,Backend: Before (Original Implementation)
    User->>Batch: evaluate()
    Batch->>EvalContext: with _EvalContext.get()
    activate EvalContext
    EvalContext->>Batch: enter context
    Batch->>Batch: check if self._backend is None
    alt backend is None
        Batch->>Backend: _invocation_result.value(ctx)
        Backend-->>Batch: return backend
    end
    EvalContext->>Batch: exit context
    deactivate EvalContext
    Batch-->>User: return self

    Note over Batch,Backend: After (Optimized Implementation)
    User->>Batch: evaluate()
    Batch->>Batch: check if self._backend is None
    alt backend is None
        Batch->>EvalContext: with _EvalContext.get()
        activate EvalContext
        EvalContext->>Batch: enter context
        Batch->>Backend: _invocation_result.value(ctx)
        Backend-->>Batch: return backend
        EvalContext->>Batch: exit context
        deactivate EvalContext
    end
    Batch-->>User: return self

    Note over Batch: Performance Improvement: Context only created when needed

Loading

_{3 files reviewed, 5 comments}

_{Edit Code Review Agent Settings | Greptile}

[review-notebook-app]

Check out this pull request on 

See visual diffs & provide feedback on Jupyter Notebooks.

---

Powered by ReviewNB

[dali-automaton]

CI MESSAGE: [37814741]: BUILD STARTED

[greptile-apps]

Greptile Overview

Greptile Summary

This PR enhances the documentation of DALI's Dynamic Mode API, focusing on Tensor, Batch, and Reader classes by adding comprehensive docstrings for classes, methods, and properties.

Key Changes:

• Added extensive class-level and method-level documentation for Tensor and Batch objects, explaining lazy evaluation, device handling, and data ownership
• Documented Reader methods (run and next_epoch) with clear usage notes about iterator behavior
• Made assign method private (renamed to _assign) in both Tensor and Batch classes
• Removed unused data property from Tensor class
• Optimized evaluate() methods by checking if self._backend is None before entering EvalContext, reducing overhead for already-evaluated objects
• Fixed multiple typos across C++ headers and documentation ("Rreturns" → "Returns", "specied" → "specified", "consectutive" → "consecutive", etc.)

Documentation Improvements:

• Properties now have docstrings explaining their purpose and return values
• Methods include parameter descriptions and usage examples (e.g., Batch.shape example)
• Added warnings about direct constructor usage, directing users to factory functions
• Clarified batch dimension semantics (N dimension not included in ndim or layout)

Confidence Score: 5/5

• This PR is safe to merge with minimal risk - it primarily adds documentation with minor refactoring
• The changes are primarily documentation additions with minimal code changes. The refactoring (renaming assign to _assign, removing unused data property, and optimizing evaluate() nesting) are safe improvements. The optimization in evaluate() is beneficial as it avoids entering the context manager when unnecessary. All changes maintain backward compatibility for public APIs.
• No files require special attention

Important Files Changed

File Analysis

Filename	Score	Overview
dali/python/nvidia/dali/experimental/dynamic/_batch.py	5/5	Adds comprehensive docstrings for Batch class and methods, renames assign to _assign (private), and optimizes evaluate() by checking _backend before entering context
dali/python/nvidia/dali/experimental/dynamic/_tensor.py	5/5	Adds comprehensive docstrings for Tensor class and properties, renames assign to _assign (private), removes unused data property, and optimizes evaluate() similarly to Batch
dali/python/nvidia/dali/experimental/dynamic/ops.py	5/5	Adds docstrings for Reader methods (run and next_epoch) to improve API documentation

Sequence Diagram

sequenceDiagram
    participant User
    participant Tensor
    participant Batch
    participant EvalContext
    participant Backend
    
    User->>Tensor: Create tensor/batch with data
    Note over Tensor: Stores data or lazy invocation
    
    User->>Tensor: Access property (shape, dtype, etc.)
    alt Property available without evaluation
        Tensor-->>User: Return cached/inferred value
    else Requires evaluation
        Tensor->>Tensor: evaluate()
        Note over Tensor: Check if _backend is None (NEW)
        alt _backend already exists
            Tensor-->>User: Return property from backend
        else _backend is None
            Tensor->>EvalContext: Enter context
            EvalContext->>Backend: Execute lazy operation
            Backend-->>Tensor: Return computed backend
            Tensor-->>User: Return property
        end
    end
    
    Note over Tensor,Batch: assign() renamed to _assign() (private)

Loading

_{6 files reviewed, no comments}

_{Edit Code Review Agent Settings | Greptile}

[dali-automaton]

CI MESSAGE: [37814741]: BUILD FAILED

[rostan-t]

Should the functions that call (or might call) evaluate on a tensor or batch be documented?

[rostan-t]

Suggested change

	* a single buffer and shape, owner by DALI, representing consecutive tensors
	* a single buffer and shape, owned by DALI, representing consecutive tensors

[rostan-t]

This class should override __call__ in order to add docstring.