search

kraiskil / onnx2c

Open Neural Network Exchange to C compiler.
Other
204 stars 34 forks source link

Split operator #31

Open cedricjoulain opened 1 year ago

cedricjoulain commented 1 year ago

Hello I'm trying to convert a "classical" YoloV8 (10MB) .onnx and I get this error:

./onnx2c yolov8n.onnx
2023-03-27 15-28-59.799 [Fatal] (createNode) Unimplemented: node operation Split Assertion failed: (false), function createNode, file graph.cc, line 483. zsh: abort ./onnx2c yolov8n.onnx

bayesiandog commented 1 month ago

Hi, any updates on this?

kraiskil commented 1 month ago

@mryndzionek started with this: https://github.com/kraiskil/onnx2c/pulls Doesn't look like there would be any showstoppers (i.e. dynamic memory allocation) with Split.

mryndzionek commented 1 month ago

@mryndzionek started with this: https://github.com/kraiskil/onnx2c/pulls Doesn't look like there would be any showstoppers (i.e. dynamic memory allocation) with Split.

In the end I managed to simplify my model to not use Split, so I probably won't continue with this, but I have a naive and untested implementation of print for Split. I can push it to my PR. Writing tests for it would be necessary though (also tried, but encountered problems with installing all the necessary Python modules with python 3.10-3.11).

kraiskil commented 1 month ago

Please do push it, would be cool to have it in. It is always easier to fix something than write it from scratch :) (This is trivially true, since after writing something from scratch, it still would need that fixing...). And it's nicer to have something than nothing.

Python shouldn't be necessary to run the unittests, btw. I consider it sufficient enough to have the ONNX node backend tests in place (i.e. onnx/onnx/backend/test/data/node/test_split_*). They are automatically taken into use when added to the list here: https://github.com/kraiskil/onnx2c/blob/687d5239abe0a49c273ccc7fb5408424cf6a0168/test/CMakeLists.txt#L724 If some tests don't pass, it would be super nice to have the resolve step catch what is causing them to fail, and error out. That corner case of the split operator can then be considered as not implemented.

mryndzionek commented 1 month ago

Python shouldn't be necessary to run the unittests, btw.

Okay, good to know. I was looking at test/local_ops and Python scripts there.

kraiskil commented 1 month ago

Ah, right. That directory has extra tests for corner cases that ONNX backend tests doesn't test for. I used the python scripts to generate such tests, and left the scripts in place as a reference for the future. Sorry for the confusion :)

kraiskil commented 1 month ago

PR #51 just got merged in. Maybe it helps?