-
Notifications
You must be signed in to change notification settings - Fork 621
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
Add experimental CV-CUDA resize #5637
Add experimental CV-CUDA resize #5637
Conversation
!build |
CI MESSAGE: [18508010]: BUILD STARTED |
CI MESSAGE: [18508010]: BUILD FAILED |
20da032
to
be71021
Compare
!build |
CI MESSAGE: [18511395]: BUILD STARTED |
be71021
to
571b143
Compare
!build |
CI MESSAGE: [18511556]: BUILD STARTED |
CI MESSAGE: [18511556]: BUILD FAILED |
571b143
to
1a6852b
Compare
!build |
CI MESSAGE: [18512606]: BUILD STARTED |
CI MESSAGE: [18512606]: BUILD FAILED |
1a6852b
to
826106d
Compare
!build |
CI MESSAGE: [18515034]: BUILD STARTED |
CI MESSAGE: [18515034]: BUILD FAILED |
826106d
to
889c104
Compare
!build |
CI MESSAGE: [18537144]: BUILD STARTED |
Signed-off-by: Rafal Banas <[email protected]>
889c104
to
c40fd3b
Compare
!build |
CI MESSAGE: [18539715]: BUILD STARTED |
CI MESSAGE: [18537144]: BUILD FAILED |
CI MESSAGE: [18539715]: BUILD PASSED |
if (HasEmptySamples(in_shape_)) { | ||
curr_minibatch_size_ = 1; | ||
} |
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.
Why? Can't we just skip them when assembling the input/output TensorBatch?
We could do any of the following - and perhaps more:
- store indices for each minibatch explicitlly instead of just storing the range - probably the easiest to implement;
- have an global index array with only non-empty samples (each entry pointing to the original sample/index) - likely the most efficient option, also not hard to implement
- we could even assemble an effective input/output TensorList and work with that (most expensive).
All options are better than giving up batched processing.
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.
Empty samples in the input are a very much edge case, I assume, so it wouldn't affect regular execution. Nevertheless, I implemented the solution with global index array
Signed-off-by: Rafal Banas <[email protected]>
!build |
CI MESSAGE: [18651120]: BUILD STARTED |
CI MESSAGE: [18651120]: 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.
Looks like it works and is tested
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 believe it would be good to add explicit (and extensive) info, how does fn.experimental.resize
differ from fn.resize
. I know user can read CV-CUDA docs and then fn.resize
docs, but putting the difference in fn.experimental.resize
I believe is highly accurate. Plus, I don't think there's anywhere good explanation, how does DALI resize differ from HQResize.
Category:
New feature
Description:
This PR adds new resize operator that uses CV-CUDA HQResize as its implementation.
It uses existing infrastructure of the resize operators to handle arguments and the cv-cuda op more or less replaces DALI kernel.
The testing involves extending existing tests to run the new operator in the same way the CPU and GPU versions are run.
Additional information:
Affected modules and functionalities:
New operator, small changes in ResizeBase
Key points relevant for the review:
Tests:
Checklist
Documentation
DALI team only
Requirements
REQ IDs: N/A
JIRA TASK: N/A