-
Notifications
You must be signed in to change notification settings - Fork 169
[onert] Export dedicated OneRT data types to Python #16210
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
base: master
Are you sure you want to change the base?
Conversation
NNFW types and numpy data types do not map one to one because NNFW has quantized types represented as uint8 or int16. Because of that it should be more efficient to export custom data type object which will map these two types. Additionally, for convenience, dedicated types are exported in the top-level onert Python module, so one can use them as follows: > import numpy as np, onert > np.array([2, 42, 42], dtype=onert.float32) ONE-DCO-1.0-Signed-off-by: Arkadiusz Bokowy <[email protected]>
6f78932 to
c36e868
Compare
|
While reviewing this PR I noticed a couple of pre-existing issues (not introduced by these changes):
np.array([1, 2, 3], dtype=onert.qint8)will not work as expected. We likely need a bidirectional converter and clear usage rules so that dtype handling is both intuitive and reliable. These are not regressions introduced by this PR, but they directly affect how usable the Python API works. If possible, could you also consider addressing them when binding tensorinfo? |
|
One additional request: after the changes in this PR, could you please check that the existing python examples still run correctly? Currently our CI does not include python API validation. |
I've been thinking about extending numpy with OneRT quantized types, so then numpy will be able to convert between floats, integers and quantized types. I've never done that before though, so I will have to investigate the pros and cons of such approach. Anyway, I think that then the API will be clear and simple because it would be possible to use all numpy operations (hopefully) on quantized tensors. |
|
I also looked into this and found NumPy’s ongoing work on a new dtype system relevant here: https://numpy.org/neps/roadmap.html#extensibility
All three are accepted or under discussion, but full implementation is still ongoing. Until this new dtype system is complete, introducing something like Given this situation, I would suggest some practical options for now:
|
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.
LGTM
Maybe we should wait with that PR (at least until the next week)? I will also look at the possibility to extend numpy data types, because I've seen somewhere (some time ago) that it is possible but maybe not very easy to do. I will investigate how complicated it would be right now (if possible at all). I will submit my investigation result here as well. |
|
That sounds like a good idea. I’ll look forward to your investigation. |
|
@arkq Please resolve conflicts |
NNFW types and numpy data types do not map one to one because NNFW has quantized types represented as uint8 or int16. Because of that it should be more efficient to export custom data type object which will map these two types.
Additionally, for convenience, dedicated types are exported in the top-level onert Python module, so one can use them as follows:
ONE-DCO-1.0-Signed-off-by: Arkadiusz Bokowy [email protected]