Skip to content

[Cutmix] Make fn.multi_paste more flexible, fix validation#5331

Merged
stiepan merged 6 commits intoNVIDIA:mainfrom
stiepan:multipaste_cutmix
Mar 1, 2024
Merged

[Cutmix] Make fn.multi_paste more flexible, fix validation#5331
stiepan merged 6 commits intoNVIDIA:mainfrom
stiepan:multipaste_cutmix

Conversation

@stiepan
Copy link
Copy Markdown
Member

@stiepan stiepan commented Feb 19, 2024

Category:

New feature (non-breaking change which adds functionality)
Bug fix (non-breaking change which fixes an issue)

Description:

This PR reworks arguments and their parsing for fn.multi_paste to make it more flexible and easier to use (with cutmix augmentation in mind; namely ability to mix different batches and no need to use image shape explicitly if the inputs are uniformly shaped). It fixes a couple of bugs.

Fixes:

  • There was no validation of in_ids leading to out-of-bound accesses for incorrect input
  • The number of channels was handled incorrectly
    • GPU assumed 3 channels no matter the actual number of channels - leading to incorrect cuda mem accesses.
    • Both backends did not verify if all the pasted regions have the same number of channels (and if the output number of channels matches). The number of output channels was copied from the corresponding input sample. The PR changes that - the number of output channels is inferred from the actually pasted regions and uses the old behaviour only if there are none (to be compatible with the only case it could have worked previously).

New features:

  • The in_anchors, region shapes and out_anchors have now relative counterparts - they can be specified as [0, 1] floats relative to input shape, input shape and output shape respectively.
  • If all the input shapes are uniform and no output size is provided, the output shape is assumed to be the same.
  • To allow mixing images of diffrent batches, the operator can accept multiple inputs - in that case the in_ids must not be specified and the regions are pasted elementwise.

The first two points are aimed to make it easier to use fn.multi_paste without explcitly handling the actual shapes of the samples.
The multi-input mode should make it easier to use the operator in simple applications where the number of paste regions is uniform - with DALI implict batch you can think in terms of samples rather than indicies in the implicit batch. And it enables cases were you need to mix images from different sources.

Other changes:

  • The args are parsed once and saved not to recompute them along the way
  • No intersection check is moved to the CPU impl, it's outputs were ignored by the GPU impl anyway.
  • Added video support.

Additional information:

Affected modules and functionalities:

Key points relevant for the review:

cutmix

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: DALI-3496

@stiepan
Copy link
Copy Markdown
Member Author

stiepan commented Feb 19, 2024

!build

Comment thread dali/test/python/operator_1/test_multipaste.py Fixed
@dali-automaton
Copy link
Copy Markdown
Collaborator

CI MESSAGE: [12935254]: BUILD STARTED

@dali-automaton
Copy link
Copy Markdown
Collaborator

CI MESSAGE: [12935254]: BUILD FAILED

@stiepan stiepan changed the title [Cutmix] Make fn.multipaste more flexible, fix validation [Cutmix] Make fn.multi_paste more flexible, fix validation Feb 20, 2024
… output shape, allow multipule inputs

Signed-off-by: Kamil Tokarski <ktokarski@nvidia.com>
Signed-off-by: Kamil Tokarski <ktokarski@nvidia.com>
@stiepan stiepan marked this pull request as ready for review February 26, 2024 11:09
Signed-off-by: Kamil Tokarski <ktokarski@nvidia.com>
@stiepan
Copy link
Copy Markdown
Member Author

stiepan commented Feb 26, 2024

!build

@dali-automaton
Copy link
Copy Markdown
Collaborator

CI MESSAGE: [13087567]: BUILD STARTED

@dali-automaton
Copy link
Copy Markdown
Collaborator

CI MESSAGE: [13087567]: BUILD PASSED

Comment thread dali/operators/image/paste/multipaste.cc Outdated
Copy link
Copy Markdown
Contributor

@klecki klecki left a comment

Choose a reason for hiding this comment

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

Before looking at tests, posting the comments

Comment thread dali/operators/image/paste/multipaste.cc Outdated
Comment thread dali/operators/image/paste/multipaste.h Outdated
Comment thread dali/operators/image/paste/multipaste.h
Comment thread dali/operators/image/paste/multipaste.h Outdated
Comment on lines +279 to +280
sample_idx, ". It should be a 2D tensor of shape [number of pasted regions]",
"x2 (i.e. ", paste_count, "x", spatial_ndim,
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Nitpick, but I would format the shape as {number of pasted regions, 2} or something like this, the [number]x2 is a bit weird.

Whatever brace/parenthesis we use for shapes...

Copy link
Copy Markdown
Member Author

@stiepan stiepan Feb 27, 2024

Choose a reason for hiding this comment

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

We print the shape without parenthesis, with x as delimiter. :)

I put the inline TensorShape{} for printing the actual shapes consistently and was a bit surprised.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

image

Comment thread dali/operators/image/paste/multipaste.h Outdated
Comment thread dali/operators/image/paste/multipaste.h Outdated
Comment on lines +312 to +314
in_anchors_data_[i].resize(n_paste);
region_shapes_data_[i].resize(n_paste);
out_anchors_data_[i].resize(n_paste);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

It probably doesn't matter much (and for sure we do similarly bad things), but vector of vector sounds like a possibility of a lot of allocations.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I tried replacing it with the TensorListView to have the var sample sizes handled, but if anything, it performed sligthly worse end-to-end.

Comment thread dali/operators/image/paste/multipaste.h Outdated
Comment thread dali/kernels/imgproc/paste/paste_gpu.h Outdated
Comment thread dali/kernels/imgproc/paste/paste_gpu.h Outdated
…arer messages, remove potential oob access, adjusted docs

Signed-off-by: Kamil Tokarski <ktokarski@nvidia.com>
Comment thread dali/operators/image/paste/multipaste.cc Outdated
Comment thread dali/operators/image/paste/multipaste.cc Outdated
Comment thread dali/operators/image/paste/multipaste.cc Outdated
Comment thread dali/operators/image/paste/multipaste.cc Outdated
Comment thread dali/operators/image/paste/multipaste.cu
Comment thread dali/operators/image/paste/multipaste.h Outdated
Comment thread dali/operators/image/paste/multipaste.h Outdated
Comment thread dali/operators/image/paste/multipaste.h
Signed-off-by: Kamil Tokarski <ktokarski@nvidia.com>
@stiepan
Copy link
Copy Markdown
Member Author

stiepan commented Mar 1, 2024

!build

@dali-automaton
Copy link
Copy Markdown
Collaborator

CI MESSAGE: [13201260]: BUILD STARTED

Signed-off-by: Kamil Tokarski <ktokarski@nvidia.com>
@stiepan
Copy link
Copy Markdown
Member Author

stiepan commented Mar 1, 2024

!build

@dali-automaton
Copy link
Copy Markdown
Collaborator

CI MESSAGE: [13202219]: BUILD STARTED

@dali-automaton
Copy link
Copy Markdown
Collaborator

CI MESSAGE: [13202219]: BUILD PASSED

@stiepan stiepan merged commit 8889d96 into NVIDIA:main Mar 1, 2024
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