-
Notifications
You must be signed in to change notification settings - Fork 618
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[Cutmix] Make fn.multi_paste more flexible, fix validation #5331
Conversation
!build |
CI MESSAGE: [12935254]: BUILD STARTED |
CI MESSAGE: [12935254]: BUILD FAILED |
… output shape, allow multipule inputs Signed-off-by: Kamil Tokarski <ktokarski@nvidia.com>
Signed-off-by: Kamil Tokarski <ktokarski@nvidia.com>
b325604
to
13a4484
Compare
Signed-off-by: Kamil Tokarski <ktokarski@nvidia.com>
!build |
CI MESSAGE: [13087567]: BUILD STARTED |
CI MESSAGE: [13087567]: BUILD PASSED |
There was a problem hiding this 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
sample_idx, ". It should be a 2D tensor of shape [number of pasted regions]", | ||
"x2 (i.e. ", paste_count, "x", spatial_ndim, |
There was a problem hiding this comment.
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...
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
in_anchors_data_[i].resize(n_paste); | ||
region_shapes_data_[i].resize(n_paste); | ||
out_anchors_data_[i].resize(n_paste); |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
…arer messages, remove potential oob access, adjusted docs Signed-off-by: Kamil Tokarski <ktokarski@nvidia.com>
Signed-off-by: Kamil Tokarski <ktokarski@nvidia.com>
!build |
CI MESSAGE: [13201260]: BUILD STARTED |
Signed-off-by: Kamil Tokarski <ktokarski@nvidia.com>
!build |
CI MESSAGE: [13202219]: BUILD STARTED |
CI MESSAGE: [13202219]: BUILD PASSED |
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:
New features:
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:
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-3496