search

kraiskil / onnx2c

Open Neural Network Exchange to C compiler.
Other
184 stars 30 forks source link

Dynamic data in resolveOutput? #8

Open mrzealot opened 3 years ago

mrzealot commented 3 years ago

Hi! I'm experimenting with additional ONNX nodes, and running into issues with ones that need dynamic tensor input even to be able to determine the output's shape in resolveOutput (an easy example is Range, where all three of start, delta, and limit are inputs, not attributes). Am I correct in assuming that this is currently only supported for initialized (i.e., const) nodes? If so, what do you think would need to change to support fully dynamic input in these cases? Thanks!

kraiskil commented 3 years ago

Yes, you assumption is correct. So far all dynamic input I have seen have turned out to be static in the .onnx file. Closest thing to dynamic input is with the LSTM node, but there the dynamic input is kind of on another axis.

And how to handle this sort of dynamic input is a good question to which I don't have a good answer. What is the real-world use-case of this sort of dynamically sized input? Maybe that gives a hint on how to handle it? E.g. does it have a natural upper bound that is known at compile time (onnx2c-compile time, that is ) that the buffers can be allocated to?

Allocating dynamic memory with malloc() & free() is of course maybe the most straight-forward solution. I don't have any fundamental objection to using them, especially when buffers are allocated and free'd by computer-generated code. However, I have a nagging feeling onnx2c would need to track the liveliness of the allocated buffers. This so they are not all free()'d all at once at the end of the entry() - potentially keeping too much memory allocated - but rather free'd after they are no longer used. This of course can be added later on as an optimization.

Finally there is the question of passing the dynamic size along the tensor info as function parameters. This is going to touch the codebase all over the place. I'm looking forward to all the fringe code cleanups that are bound to show up if you make a pull request out this handling of dynamic tensors :)

kraiskil commented 2 years ago

Slightly related: I just pushed in an implementation to the Range node that will work if the inputs are compile-time determinable. The input data being know to the compiler avoids the need for dynamic allocation in these cases.

This was made possible due to a slight rework on what const tensors actually mean. Now a Tensor::isConst really means the data in them is not modifiable. Modified input tensors then are passed as const Tensor to nodes - but these are not compile time constants.

Now adding a malloc()/free() should not pose problems. Only thing is to find out when the free() can be called. And also, any dynamic allocation should post a warning, or be enabled by a command line option switch.