Repository navigation
[Torchvision] Torchvision per sample random in batches - #6548
mdabek-nvidia wants to merge 6 commits into
Conversation
Signed-off-by: Marek Dabek <mdabek@nvidia.com>
Signed-off-by: Marek Dabek <mdabek@nvidia.com>
TorchVision samples a single parameter set per call and applies it across all leading dimensions (..., C, H, W). Compose drew random values per sample instead, so images in a batch and frames of a video were transformed differently. PipelineCHW now flattens the leading dimensions into frames of FCHW sequences: - GPU: the whole call is processed as a single sample, which is also faster than per-sample processing (1.2-1.9x in RRC+flip+normalize). - CPU: frames are split into up to min(batch_size, num_threads) sequences to keep thread parallelism. Random values drawn for the first sequence are broadcast with fn.permute_batch through the new share_random_value() helper. RandomResizedCrop on CPU generates the crop window once with fn.random_crop_generator and resizes with an ROI. Normalize uses a 4D mean/std for FCHW input. batch_size no longer limits the leading dimensions of the input. Signed-off-by: Marek Dabek <mdabek@nvidia.com> Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: Marek Dabek <mdabek@nvidia.com>
|
Check out this pull request on See visual diffs & provide feedback on Jupyter Notebooks. Powered by ReviewNB |
|
!build |
|
Please review #6544 first |
[Medium impact] The PR appears safe to merge; the previous test-setup finding is fixed.
|
|
CI MESSAGE: [76073775]: BUILD STARTED |
Signed-off-by: Marek Dabek <mdabek@nvidia.com>
Signed-off-by: Marek Dabek <mdabek@nvidia.com>
|
CI MESSAGE: [76073775]: BUILD PASSED |
| # DALI does not support strided data (e.g. a permuted tensor from pil_to_tensor). | ||
| frames = data_input.reshape(-1, *data_input.shape[-3:]).contiguous() | ||
| num_frames = frames.shape[0] | ||
| num_samples = max( |
There was a problem hiding this comment.
[Minor / efficiency] Requiring num_frames % n == 0 means the CPU parallelism silently collapses whenever the frame count has no small divisor: 7, 11, 13 or any other prime frames gives n == 1, so the call runs on a single thread, and 10 frames with 4 threads gives 2 samples. DALI batches can hold samples of different lengths, so there is no need for an even split - e.g. split into n = min(num_frames, self._max_samples_per_call) chunks of size ceil(num_frames / n) / floor(num_frames / n) (torch.tensor_split) and feed the chunks as a list of variable-length samples. Random-value sharing doesn't depend on equal sample sizes, since all samples take the value of sample 0.
The 7 shape in test_compose_random_params_shared_across_leading_dims currently only exercises the n == 1 fallback, not splitting with a remainder.
| # equal to the input size avoids differences between DALI and torchvision resampling | ||
| lambda device, controlled: RandomResizedCrop( | ||
| 32 if controlled else 24, | ||
| scale=(1.0, 1.0) if controlled else (0.08, 1.0), |
There was a problem hiding this comment.
[Minor / tests] The controlled RandomResizedCrop case uses a square input with scale=(1, 1) / ratio=(1, 1), so the crop is always the whole image. That can't catch a (y, x) vs (x, y) mix-up between fn.random_crop_generator's anchor/shape and fn.resize's roi_start/roi_end in the new shared-value path, and nothing compares the new path against fn.random_resized_crop for a non-trivial window. Consider a non-square input (e.g. 24x40) with a narrow scale, and check that the output matches a resize of the crop window recovered from the (deterministic) generator output, or at least that the non-shared and shared paths agree for a single-frame call with the same seed.
| "DALI output type: <class 'PIL.Image.Image'> size: (40, 40)\n", | ||
| "TV output type: <class 'PIL.Image.Image'> size: (40, 40)\n", | ||
| "Outputs are equal: True\n" | ||
| "Max absolute diff: 4\n" |
There was a problem hiding this comment.
[Question] This cell used to print Outputs are equal: True (atol=1) and now prints Max absolute diff: 4; the CPU-vs-GPU cell below goes from 0.0 to 1. Is that only because pil_img is now a different image (original_images[0].resize((64, 64))), or did this PR change resize numerics (e.g. the .contiguous() / FCHW path)? Documentation that advertises a diff of 4 for a plain Resize on PIL input will raise eyebrows; if it's just the new image, I'd keep the original atol=1 assertion-style output, and if it isn't, it's a regression to understand before merge.
Category:
Bug fix
Description:
This PR modified how random operations are applied in batches. In case of NCHW tensors random per sample should be applied, instead of current random per batch approach. It changes behavior of operators that uses randomness, e.g. RandomCrop
Additional information:
Affected modules and functionalities:
Key points relevant for the review:
Tests:
Checklist
Documentation
DALI team only
Requirements
REQ IDs: N/A
JIRA TASK: DALI-4892