Open cbourjau opened 1 year ago
Do you have a production use case that cannot be satisfied by the existing types and requires the API to be expanded?
There are many places where we implement a subset of the spec due to things being rarely or never used. The implementation/testing/maintenance/binary size cost of doing so is not worth it, at least not until we no longer have to pay developers to do their jobs.
Thanks for your reply!
Please let me explain my motivation for creating this issue. I have experienced the opposite cost calculation in the past with onnxruntime: While developing a model or custom operator I found some functionality (surprisingly) missing. Having to open a PR against onnxruntime and waiting for the next release in those cases tanks productivity and makes the development process around the onnxruntime unpredictable. This PR aims to smothen the development process in the future by providing a more complete API.
That said, adding APIs which are not usable due to some other blockers would not make sense and on closer inspection that may apply to some of the above apis. Below are a few comments/use cases about the different functions:
KernelInfoGetAttribute_graph
and KernelInfoGetAttributeArray_graph
A use case would be a specialized control flow operator. That said, I'm afraid there is not much one can do with a Graph
through the C-API at this point. KernelInfoGetAttribute_sparse_tensor
Sparse tensors are already (partially?) supported in the current C-API (e.g. GetSparseTensorValues). Sparse tensors have many use cases which equally apply to custom operators. KernelInfoGetAttribute_type_proto
and KernelInfoGetAttributeArray_type_proto
Same issue as with graphs. There is currently no way to do anything meaningful with a TypeProto
object given the current C-API.KernelInfoGetAttributeArray_string
Possible use case: A custom LabelEncoder
which works on more output types.KernelInfoGetAttributeArray_tensor
and KernelInfoGetAttributeArray_sparse_tensors
Possible use case: A custom operator similar to LabelEncoder
which allows mapping to tensor values. This can be useful if a downstream operator requires different weights based on a user-provided "label". KernelInfoGetAttribute_undefined
I must admit that the use-case of an undefined
attribute is a bit unclear to me. All in all, I think there reasonable utility in adding KernelInfoGetAttribute_sparse_tensor
, KernelInfoGetAttributeArray_string
, KernelInfoGetAttributeArray_tensor
, KernelInfoGetAttributeArray_sparse_tensors
. Does that sound reasonable, @skottmckay ?
Describe the feature request
The current C-API only offers the following functions for loading attributes:
KernelInfoGetAttribute_float
KernelInfoGetAttribute_int64
KernelInfoGetAttribute_string
KernelInfoGetAttributeArray_float
KernelInfoGetAttributeArray_int64
KernelInfoGetAttribute_tensor
The standard allows the following types:
The following functions are therefore missing from the C-API and should be added:
KernelInfoGetAttribute_graph
KernelInfoGetAttribute_sparse_tensor
KernelInfoGetAttribute_type_proto
KernelInfoGetAttributeArray_string
KernelInfoGetAttributeArray_tensor
KernelInfoGetAttributeArray_graph
KernelInfoGetAttributeArray_sparse_tensors
KernelInfoGetAttributeArray_type_proto
KernelInfoGetAttribute_undefined
(?)Describe scenario use case
Several of the above attribute types are used in the standard. Others might be useful to C-API authors precisely because the operator that would use them is not in the standard. Either way, it is an unfortunate surprise for C-API users to find out that the API does not support an attribute type they were planning to use.