[Draft] C API v2.0 #5728
[Draft] C API v2.0 #5728
Conversation
mzient
force-pushed
the
C_API_2
branch
4 times, most recently
from
e2bd65a to
789df99
Compare
| daliExecType_t exec_type; | ||
| daliBool enable_checkpointing; | ||
| daliBool enable_memory_stats; | ||
| } daliPipelineParams_t; |
There was a problem hiding this comment.
Do you plan adding macros to initialize this structure? It is possible to overlook and set device_id only without setting device_id_present.
Macros like:
#define DEVICE_ID_SET(pipeline_params, device_id) pipeline_params.device_id_present = 1; pipeline_params.device_id = device_id;
could minimize such subtle errors.
There was a problem hiding this comment.
I considered that, yes.
| }; | ||
| daliDataType_t dtype; | ||
| int ndim; | ||
| } daliPipelineOutputDesc_t; |
There was a problem hiding this comment.
Same here, setting dtype/ndim requires setting additional field.
| const cudaStream_t *stream); | ||
|
|
||
| /** Gets the number of pipeline outputs */ | ||
| DALI_API daliResult_t daliPipelineGetOutputCount(daliPipeline_h pipeline, int *out_count); |
There was a problem hiding this comment.
Starting from this functions have different documentation style. No parameters description, and usually no return type description.
| * The function ensures that a readiness event is associated with the tensor list. | ||
| * It can also get the event handle, if the output parameter pointer is not NULL. | ||
| */ | ||
| DALI_API daliResult_t daliTensorListGetOrCreateReadyEvent( |
There was a problem hiding this comment.
What are possible retvals and what do they mean are they the same as in the previous call?
There was a problem hiding this comment.
Well, at this point it's still a draft so I prefer not to commit to very precise error codes. When not specified otherwise, one can expect DALI_SUCCESS on success and some error code on failure. DALI_NO_DATA is specifically documented in some of the calls to denote more than one successful result.
Before we can commit to returning specific error codes. we need to fix error handling across DALI - which means nuking most of the DALI_ENFORCE/DALI_FAIL and replacing those with proper excetpions. Currently it's a mess - we mix usage errors (invalid argument or combination thereof), data errors (data-dependent index out of range, reduction over 0 extent), system errors (file not found, network errors), logic errors and more.
| * @retval DALI_SUCCESS T | ||
| * | ||
| * The pointer returned in `out_shape` remains valid until the TensorList is destroyed or modified. | ||
| * If the caller is not intersted in some of the values, the pointers can be NULL. |
There was a problem hiding this comment.
Is it expected that for the call: daliResult_t retval = daliTensorListGetShape(tl, NULL, NULL, NULL) ;, the retval will be DALI_SUCCESS?
There was a problem hiding this comment.
Either that or DALI_NO_DATA. If we make the fields optional, there's no point in adding an artificial failure in a pointless call.
include/dali/dali.h
Outdated
| const char *layout | ||
| ); | ||
|
|
||
| /** Gets the "source info" metadata of a sample */ |
There was a problem hiding this comment.
I would appreciate more descriptive header
Signed-off-by: Michal Zientkiewicz <[email protected]>
Signed-off-by: Michał Zientkiewicz <[email protected]>
Signed-off-by: Michał Zientkiewicz <[email protected]>
Signed-off-by: Michał Zientkiewicz <[email protected]>
Signed-off-by: Michal Zientkiewicz <[email protected]>
Signed-off-by: Michal Zientkiewicz <[email protected]>
Signed-off-by: Michal Zientkiewicz <[email protected]>
Signed-off-by: Michał Zientkiewicz <[email protected]>
Signed-off-by: Michał Zientkiewicz <[email protected]>
Signed-off-by: Michał Zientkiewicz <[email protected]>
Signed-off-by: Michał Zientkiewicz <[email protected]>
Signed-off-by: Michal Zientkiewicz <[email protected]>
Category:
Key points relevant for the review:
Tests:
Checklist
Documentation
DALI team only
Requirements
REQ IDs: N/A
JIRA TASK: DALI-4124