Change the default order of data storage objects #4276
Conversation
|
CI MESSAGE: [5983215]: BUILD STARTED |
|
CI MESSAGE: [5983215]: BUILD PASSED |
mzient
force-pushed
the
TestHostOrder
branch
from
9699204
to
67b790a
Compare
|
CI MESSAGE: [6011132]: BUILD STARTED |
|
CI MESSAGE: [6012006]: BUILD STARTED |
|
CI MESSAGE: [6012006]: BUILD FAILED |
|
CI MESSAGE: [6012651]: BUILD STARTED |
|
CI MESSAGE: [6011132]: BUILD FAILED |
|
CI MESSAGE: [6012651]: BUILD FAILED |
Signed-off-by: Michal Zientkiewicz <michalz@nvidia.com>
Signed-off-by: Michal Zientkiewicz <michalz@nvidia.com>
Signed-off-by: Michal Zientkiewicz <michalz@nvidia.com>
Signed-off-by: Michal Zientkiewicz <michalz@nvidia.com>
Signed-off-by: Krzysztof Lecki <klecki@nvidia.com>
|
!build |
|
CI MESSAGE: [6097411]: BUILD STARTED |
|
CI MESSAGE: [6097411]: BUILD PASSED |
klecki
changed the title
mzient
changed the title
dali/pipeline/data/tensor_list.cc
Outdated
| @@ -463,6 +463,8 @@ const TensorListShape<> &TensorList<Backend>::shape() const & { | |||
|
|
|||
| template <typename Backend> | |||
| void TensorList<Backend>::set_order(AccessOrder order, bool synchronize) { | |||
| if (!order) | |||
There was a problem hiding this comment.
So you cannot nullify the order of the tensor list?
There was a problem hiding this comment.
I also have my reservations about it, but maybe it's similar to setting DALI_NO_TYPE for a type. Another thing is, we do an explicit error there, and silently do nothing here.
There was a problem hiding this comment.
And.. what do we need AccessOrder::null_stream() for?
There was a problem hiding this comment.
And.. should the warp.h use context.gpu.stream = ws.has_stream() ? ws.stream() : AccessOrder::null_stream(); or rather host_stream?
There was a problem hiding this comment.
Hm, okay, {} results in a nullstream too, so there are more places like that.
There was a problem hiding this comment.
I added a restriction that disallows setting the empty order to TensorList and Buffer, and allowed it to mean no-op in the APIs where it was used that way (so it can be an optional argument). For the null_stream, I think it is invalid, and if used I guess we will get an error?
Add a bit of docstring and a CPU-only test used for debugging Signed-off-by: Krzysztof Lecki <klecki@nvidia.com>
|
!build |
|
CI MESSAGE: [6157450]: BUILD STARTED |
|
!build |
|
CI MESSAGE: [6157782]: BUILD STARTED |
|
CI MESSAGE: [6157782]: BUILD FAILED |
Signed-off-by: Krzysztof Lecki <klecki@nvidia.com>
|
!build |
|
CI MESSAGE: [6158427]: BUILD STARTED |
|
CI MESSAGE: [6158427]: BUILD FAILED |
|
CI MESSAGE: [6158427]: BUILD PASSED |
klecki
changed the title
Signed-off-by: Michal Zientkiewicz michalz@nvidia.com
Category: Refactoring, Bug fix?
Description:
Change the default order for data storage objects to
host- regardless if they contain any data or not.Additional information:
Affected modules and functionalities:
Key points relevant for the review:
Tests:
The performance in RN50 and retina was validated.
Checklist
Documentation
DALI team only
Requirements
REQ IDs: N/A
JIRA TASK: N/A