Skip to content

[Torchvision API] crop#6353

Open
mdabek-nvidia wants to merge 11 commits into
NVIDIA:mainfrom
mdabek-nvidia:torchvision_crop
Open

[Torchvision API] crop#6353
mdabek-nvidia wants to merge 11 commits into
NVIDIA:mainfrom
mdabek-nvidia:torchvision_crop

Conversation

@mdabek-nvidia
Copy link
Copy Markdown
Collaborator

Category:

New feature

Description:

Torchvision API operators RandomCrop and functional crop.
Unit tests checking conformance with Torchvision.
Known limitations: tensor and PIL.Image based paddings are not supported

Additional information:

Affected modules and functionalities:

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: Marek Dabek <mdabek@nvidia.com>
Signed-off-by: Marek Dabek <mdabek@nvidia.com>
Signed-off-by: Marek Dabek <mdabek@nvidia.com>
Signed-off-by: Marek Dabek <mdabek@nvidia.com>
Signed-off-by: Marek Dabek <mdabek@nvidia.com>
@mdabek-nvidia
Copy link
Copy Markdown
Collaborator Author

@greptileai please review

@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps Bot commented May 18, 2026

Greptile Summary

This PR adds torchvision-compatible RandomCrop and functional crop operators to DALI's experimental torchvision API. Padding and pad_if_needed are implemented by computing a virtual padded-image space and letting DALI's fn.slice with out_of_bounds_policy handle the boundary fill, avoiding a separate padding pass.

  • randomcrop.py: _randint clamps negative max_value to 0 before passing to fn.random.uniform, preventing an inverted-range DALI crash when the crop size exceeds the padded image. _ValidateFill now correctly rejects dict fills (previously listed as supported but not implemented).
  • crop.py: Functional crop dispatches PIL rounding vs. integer-only validation based on the tensor layout suffix, mirroring torchvision's behavior, then calls ndd.slice directly.
  • Tests: Cover tensor/PIL/batched inputs, multiple padding modes, asymmetric padding with pad_if_needed, and error-rejection paths. Skipped dict-fill tests mark a known limitation acknowledged in the PR description.

Confidence Score: 5/5

Safe to merge; the new operators are well-tested against torchvision references and known limitations are clearly documented.

Both operators are exercised against torchvision reference outputs with deterministic inputs. Previous reviewer concerns (negative max_value in _randint, false dict-fill support, misleading range_start expression, asymmetric padding semantics) are all addressed in this revision. The only remaining item is four TODO comments in the test file that lack tracked issue references.

test_tv_randomcrop.py — four TODO comments for dict-fill support need a tracking issue before merge.

Important Files Changed

Filename Overview
dali/python/nvidia/dali/experimental/torchvision/v2/randomcrop.py New RandomCrop operator. _randint correctly clamps max_value to 0 before feeding fn.random.uniform, preventing inverted-range crashes. Symmetric pad_if_needed expansion is correct and tested. Validation classes are clean.
dali/python/nvidia/dali/experimental/torchvision/v2/functional/crop.py New functional crop operator that delegates size validation to RandomCrop.verify_args and calls ndd.slice with out_of_bounds_policy='pad'. PIL rounding logic mirrors torchvision's behavior. No issues found.
dali/test/python/torchvision/test_tv_randomcrop.py Comprehensive test suite covering shapes, padding variants, PIL/tensor inputs, and edge cases. Contains four TODO comments for dict-fill support without tracked issue references, violating the no-todo-on-merge rule.
dali/test/python/torchvision/test_tv_crop.py Tests functional crop against torchvision reference for tensor, PIL, batched-tensor, dtype preservation, and error-rejection cases. Good coverage across CPU/GPU and multiple dtypes.
dali/python/nvidia/dali/experimental/torchvision/init.py Adds RandomCrop import and all entry. Alphabetically ordered correctly.
dali/python/nvidia/dali/experimental/torchvision/v2/functional/init.py Adds crop import and all entry. Alphabetically ordered correctly.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[User input\nTensor / PIL Image] --> B{adjust_input\ndecorator}
    B --> C[DALI Tensor/Batch\nwith layout]
    C --> D{crop / RandomCrop}
    subgraph crop [functional.crop]
        D1{PIL layout?\nlayout ends HWC} -->|Yes| D2[_round_pil_box\nfloat to int rounding]
        D1 -->|No| D3[_validate_integer_param\nreject floats]
        D2 & D3 --> D4[RandomCrop.verify_args\nsize validation]
        D4 --> D5[ndd.slice\nout_of_bounds_policy=pad]
    end
    subgraph rcrop [RandomCrop._kernel]
        E1[Compute padded_h/w\nfrom explicit padding] --> E2{pad_if_needed?}
        E2 -->|Yes| E3[dali.math.max\nexpand both sides]
        E2 -->|No| E4[max_top/left =\npadded - crop]
        E3 --> E4
        E4 --> E5[_randint\nclamp to 0, fn.random.uniform]
        E5 --> E6[fn.slice\nanchor = offset - pad\nout_of_bounds fill]
    end
    D --> D1
    C --> E1
    E6 --> F[Output Tensor / PIL]
    D5 --> F
Loading

Reviews (11): Last reviewed commit: "Review fixes" | Re-trigger Greptile

Comment thread dali/python/nvidia/dali/experimental/torchvision/v2/randomcrop.py Outdated
Comment thread dali/python/nvidia/dali/experimental/torchvision/v2/randomcrop.py
Signed-off-by: Marek Dabek <mdabek@nvidia.com>
Comment thread dali/python/nvidia/dali/experimental/torchvision/v2/randomcrop.py Fixed
Comment thread dali/python/nvidia/dali/experimental/torchvision/v2/randomcrop.py Fixed
Comment thread dali/python/nvidia/dali/experimental/torchvision/v2/randomcrop.py Fixed
@mdabek-nvidia
Copy link
Copy Markdown
Collaborator Author

@greptileai re-review please

Signed-off-by: Marek Dabek <mdabek@nvidia.com>
Comment thread dali/python/nvidia/dali/experimental/torchvision/v2/randomcrop.py Outdated
@mdabek-nvidia mdabek-nvidia marked this pull request as ready for review May 19, 2026 13:37
Signed-off-by: Marek Dabek <mdabek@nvidia.com>
Comment thread dali/python/nvidia/dali/experimental/torchvision/v2/randomcrop.py Outdated
Comment thread dali/python/nvidia/dali/experimental/torchvision/v2/randomcrop.py
Comment thread dali/python/nvidia/dali/experimental/torchvision/v2/randomcrop.py
Comment thread dali/test/python/torchvision/test_tv_randomcrop.py Outdated
@mdabek-nvidia
Copy link
Copy Markdown
Collaborator Author

mdabek-nvidia commented May 22, 2026

@greptileai, re-review

Comment thread dali/python/nvidia/dali/experimental/torchvision/v2/randomcrop.py Fixed
Comment thread dali/python/nvidia/dali/experimental/torchvision/v2/randomcrop.py Fixed
Comment thread dali/python/nvidia/dali/experimental/torchvision/v2/randomcrop.py Outdated
@mdabek-nvidia
Copy link
Copy Markdown
Collaborator Author

@greptileai re-review

Comment thread dali/python/nvidia/dali/experimental/torchvision/v2/randomcrop.py
Signed-off-by: Marek Dabek <mdabek@nvidia.com>
@mdabek-nvidia
Copy link
Copy Markdown
Collaborator Author

@greptileai re-review

@mdabek-nvidia mdabek-nvidia changed the title Torchvision crop [Torchvision API] crop May 25, 2026
@mdabek-nvidia
Copy link
Copy Markdown
Collaborator Author

!build

@dali-automaton
Copy link
Copy Markdown
Collaborator

CI MESSAGE: [52613374]: BUILD STARTED

@dali-automaton
Copy link
Copy Markdown
Collaborator

CI MESSAGE: [52613374]: BUILD FAILED

@dali-automaton
Copy link
Copy Markdown
Collaborator

CI MESSAGE: [52613374]: BUILD PASSED

Comment thread dali/python/nvidia/dali/experimental/torchvision/v2/functional/crop.py Outdated
* Different index support for tensor and PILImages

Signed-off-by: Marek Dabek <mdabek@nvidia.com>
@mdabek-nvidia
Copy link
Copy Markdown
Collaborator Author

!build

@dali-automaton
Copy link
Copy Markdown
Collaborator

CI MESSAGE: [52656583]: BUILD STARTED

@dali-automaton
Copy link
Copy Markdown
Collaborator

CI MESSAGE: [52656583]: BUILD 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.

5 participants