Add metadata-only inputs. #5635
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Signed-off-by: Michał Zientkiewicz <mzient@gmail.com>
Signed-off-by: Michał Zientkiewicz <mzient@gmail.com>
|
CI MESSAGE: [18471715]: BUILD STARTED |
Signed-off-by: Michał Zientkiewicz <mzient@gmail.com>
|
CI MESSAGE: [18471967]: BUILD STARTED |
JanuszL
approved these changes
Sep 17, 2024
Signed-off-by: Michał Zientkiewicz <mzient@gmail.com>
|
CI MESSAGE: [18473499]: BUILD STARTED |
|
CI MESSAGE: [18471967]: BUILD PASSED |
|
CI MESSAGE: [18473499]: BUILD PASSED |
stiepan
reviewed
Sep 18, 2024
Signed-off-by: Michał Zientkiewicz <mzient@gmail.com>
mzient
commented
Sep 18, 2024
Comment on lines
-281
to
-285
| // If the output order of the operator is `host` then we don't wait for GPU | ||
| // inputs - they can't be accessed directly on host and the operator will | ||
| // have to issue some sort of synchronization if and when necessary. | ||
| // This optimization is essential to avoid oversynchronization | ||
| // when the operator needs to access the metadata only (e.g. getting the shape). |
There was a problem hiding this comment.
Here we get rid of the dangerous special treatment of GPU inputs to host-ordered operators. The new metadata flag takes care of the optimization now.
|
CI MESSAGE: [18504351]: BUILD STARTED |
stiepan
approved these changes
Sep 18, 2024
stiepan
pushed a commit
that referenced
this pull request
Sep 18, 2024
* Assign a stream to CPU inputs with GPU (non-metadata) inputs * Add Metadata input device - this declares that the input is used for metadata (shape, dtype, etc) access only * Don't synchronize metadata inputs in executor. * Don't prolong the lifetime of metadata-only inputs. * Add InputDevice specifier to random number generators shape_like input. --------- Signed-off-by: Michał Zientkiewicz <mzient@gmail.com>
|
CI MESSAGE: [18504351]: BUILD PASSED |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Category:
Bug fix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)
Description:
The bug:
The Copy operator from device to host executed in host order, but the synchronization was skipped.
The solution:
Use a stream for CPU operators with GPU inputs.
Add a special class of metadata-only inputs which do need to wait for the data, so the executor can skip synchronization.
The necessary synchronization happens in the consumers of the copy results (the D2H copy result is a pinned buffer and the copy is asynchronous).
Additional information:
Affected modules and functionalities:
New executor:
Anyinput now take aMetadatainputKey points relevant for the review:
Tests:
Stream assignment tests have been updated.
The
pipeline_test.test_gpu2cpu_conditionalsserves as a bug-fix test.Checklist
Documentation
DALI team only
Requirements
REQ IDs: N/A
JIRA TASK: N/A