Refactored operator set generation

[nopeslide]

• no single/central global operator set anymore
• kconfig friendly
• operator set generation is now independent from operator generation
• related Build system ideas #58

[nopeslide]

@alrevuelta
open for comments :)
I would also add a parameter to specify the domain when searching operators, so we can exclude domains which are not imported by the model.

[alrevuelta]

Nice, could you elaborate more on why adding this set_operator__ai_onnx__argmax.c in a decentralised way? Benefits? Problem trying to solve? Impact? Just so we can have some nice "documentation" that we can add later on to the real docs.

First time I see __COUNTER__. After a quick read it scares me a bit. Are you sure about this?

[nopeslide]

Nice, could you elaborate more on why adding this set_operator__ai_onnx__argmax.c in a decentralised way?

the operator set structure is now aligned with our directory structure.
set_operator.c includes all domains
set_operator__<domain>.c include all operators of the domain
set_operator__<domain>__<operator>.c include all versions of the operator
set_operator__<domain>__<operator>__<version>.c the actual operator implementation reference
My intention is to generate matching #ifdef exclusions around the includes and pointers to select implemented operator from all existing ones. the current version only features implemented operators, because the switches will come with the kconfig proposal.

Benefits?

operators can be added easily, clean structure, hierarchical overview

Problem trying to solve?

get rid of the currently generated blob that is our operator set

Impact?

we need to resolve a few pointers more and move the "is this operator part of the desired opset" into the runtime, instead of having it directly.

Just so we can have some nice "documentation" that we can add later on to the real docs.

:)

First time I see __COUNTER__. After a quick read it scares me a bit. Are you sure about this?

to be honest:
It should scare you. It is context aware logic inside the preprocessor and therefore bad.
buuuut, it is fine as long it is contained and easy to spot.
it is contained since it is only used in implementation files. not inside any header.
it is easy to spot, since it is not hidden.
it is the only way to count list elements with help of the preprocessor.
an alternative would be to NULL-terminate these lists (yeah maybe we should do that :D).

[nopeslide]

switched to NULL termination.
was a fun experiment though

[alrevuelta]

switched to NULL termination.
was a fun experiment though

You rock :D

[nopeslide]

@alrevuelta
rebased this onto master with the prepare/execute split.
should be a drop-in replacement for our operator_set.

as a reminder:
I had the plan to switch to kconfig for operator management (which ops to include/exclude), this PR restructures our operator set so it can be easily configured by kconfig or extended manually.
as always: open for comments :)

[nopeslide]

one thing I wanted to do was to generate the whole onnx operator set and use kconfig to choose which are included in the build process (aka are implemented).
but with this current approach of each operator needs a header and this would mean we need the directory structure of the whole operator set, regardless of implementation status.
so maybe all these headers are too much?
I could easily move the extern declaration into the level below, so we do not need the directory with the header inside.

[nopeslide]

I moved the extern declaration into the implementation file, much cleaner now :)

[alrevuelta]

I had the plan to switch to kconfig for operator management (which ops to include/exclude), this PR restructures our operator set so it can be easily configured by kconfig or extended manually.

Really looking forward to see kconfig in action. Hope that #57 and this PR doesn't add much stuff on top and we can have a reasonable binary when using very few operators.

one thing I wanted to do was to generate the whole onnx operator set and use kconfig to choose which are included in the build process (aka are implemented).
but with this current approach of each operator needs a header and this would mean we need the directory structure of the whole operator set, regardless of implementation status.
so maybe all these headers are too much?

I would say that generating the whole onnx operator set and all the headers should wait until we implement few more operators and we polish the generation process. I don't want to generate everything now, then change some stuff in the generator script and then have to regenerate everything again. So what I suggest is.

• Lets merge this whenever you feel ready.
• Start developing and using kconfig to build a subset of what we have implemented (lets say just the operators for mnist)
• Keep working on new operators and polish the generation process. I'm sure that when developing new operators we will have ideas that can improve the generator, or perhaps some things we've missed.
• Once we are almost sure that the generator script is ready and most likely won't change, we can generate the whole directory structure with everything, and then use kconfig to build just the operators that are implemented. With this if someone wants to implement a new operator, they just have to fill the code, no need anymore to generate.

What do you think?

[nopeslide]

Hope that #57 and this PR doesn't add much stuff on top and we can have a reasonable binary when using very few operators.

I don't think this will add up, since it's only a few pointers, but I'll let the numbers speak.
before prepare/execute split (f3b0b6d):

$ ls -sh build/connxr  
1.7M build/connxr
$ strip build/connxr
$ ls -sh build/connxr
144K build/connxr

with prepare/execute split and recursive sets:

$ ls -sh build/connxr
2.1M build/connxr
$ strip build/connxr
$ ls -sh build/connxr
144K build/connxr

hope this removes any doubt from your side :)

I would say that generating the whole onnx operator set and all the headers should wait until we implement few more operators and we polish the generation process. I don't want to generate everything now, then change some stuff in the generator script and then have to regenerate everything again. So what I suggest is.

the idea was we do this once (for a domain with implemented operators) and then we only need to regenerate the operator set when onnx updates. but this will only be relevant (and possible) when kconfig is working.

• Lets merge this whenever you feel ready.

on it :>

• Start developing and using kconfig to build a subset of what we have implemented (lets say just the operators for mnist)

would be next on my list

• Keep working on new operators and polish the generation process. I'm sure that when developing new operators we will have ideas that can improve the generator, or perhaps some things we've missed.

I like the way the generator is moving: from necessary to a "simple" templating tool.

• Once we are almost sure that the generator script is ready and most likely won't change, we can generate the whole directory structure with everything, and then use kconfig to build just the operators that are implemented. With this if someone wants to implement a new operator, they just have to fill the code, no need anymore to generate.

this PR decouples the operator set generation from the actual operators: as long as we don't touch the operator set structure, we are free to do anything with the operators.