Skip to content
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

Split operator #31

Open
cedricjoulain opened this issue Mar 27, 2023 · 7 comments
Open

Split operator #31

cedricjoulain opened this issue Mar 27, 2023 · 7 comments
Labels
good first issue Good for newcomers operand missing ONNX operand not yet implemented in onnx2c

Comments

@cedricjoulain
Copy link

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

@kraiskil kraiskil added good first issue Good for newcomers operand missing ONNX operand not yet implemented in onnx2c labels Mar 27, 2023
@bayesiandog
Copy link

Hi, any updates on this?

@kraiskil
Copy link
Owner

kraiskil commented Aug 7, 2024

@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
Copy link
Contributor

@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
Copy link
Owner

kraiskil commented Aug 8, 2024

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:

ONNX_backend_node_test(sqrt)

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
Copy link
Contributor

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
Copy link
Owner

kraiskil commented Aug 8, 2024

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
Copy link
Owner

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Good for newcomers operand missing ONNX operand not yet implemented in onnx2c
Projects
None yet
Development

No branches or pull requests

4 participants