Skip to content

Kconfig#71

Draft
nopeslide wants to merge 3 commits into
alrevuelta:masterfrom
nopeslide:kconfig
Draft

Kconfig#71
nopeslide wants to merge 3 commits into
alrevuelta:masterfrom
nopeslide:kconfig

Conversation

@nopeslide

@nopeslide nopeslide commented Aug 25, 2020

Copy link
Copy Markdown
Collaborator

this is just a demo to evaluate if kconfig would be feasible as build system configuration.

  • install kconfig toolchain (i.e. pip install kconfiglib)
  • run menuconfig to configure the build
  • Makefiles can include the generated .config file
  • run genconfig to generate the configuration header config.h
  • the generated options are not used in any of the code
    • fisrt step would be to integrate it into the set generation
  • this demo "simulates" selection of specific domains, operators and versions
  • tensor and attribuite datatypes can be disables which may deactivate specific operators which depend on these.

why kconfig?

  • options can depend on other options
  • complex dependencies are resolved for the user
  • commonly used
  • easy to use
  • integrated help/documentation (press 'F' in menuconfig)
  • generated config can be easily shared, since it is a single file
  • targets c projects

@alrevuelta

Copy link
Copy Markdown
Owner

Nice! Looks good to me.

Slightly related to this, I think we need to discuss which configuration parameters we expect. For example, I see that currently you have the operators, versions, attribute types tensor types.

Regarding the versions, we have something like this (I used some random numbers):

  • Operator Add
    • Version 7
    • Version 8
  • Operator Abs
    • Version 7
    • Version 10

So this means that it would be possible to have in the same build operators from different versions. One of the problems that I see is that a different operator version, can imply also a different schema version. So we have to also control that the model is read with the correct schema .pb version. Not a problem, but just bringing it up.

I'm wondering if it makes sense to have that much flexibility. I mean, I think the input to Kconfig could be something simpler like the opset version and the specific operators. No need to specify the version per operator.

I know you want cONNXr as flexible as possible, but does it really make sense to have in the same build 3 operator from opset X, 7 from opset Y and 13 from opset Z? I think the user should be able to say: I want full opset 10, or I want operators X,Y,Z from opset 13. But don't allow to have weird mixes, that from my point of view doesn't makes sense on a practical level.

Side question: Is it possible to have in the same model operators from different versions? I would say no, because in the _Onnx__NodeProto there is no such information.

@nopeslide

Copy link
Copy Markdown
Collaborator Author

@alrevuelta
We need per operator settings if we have options inside an operator that need to be reflected with kconfig.
The opset selection would be similar to the global type selections, if you want opset xy, kconfig activates the corresponding operators, but for this to work, the specific operators need to be selectable (as they are now).
The schema version is another problem, we may not solve right now.
Afaik you can only select a specific opset in the model and not per operator.

@nopeslide nopeslide force-pushed the kconfig branch 2 times, most recently from d8f6007 to 3b87255 Compare March 4, 2021 15:23
@nopeslide nopeslide changed the title Kconfig demo Kconfig Mar 4, 2021
@nopeslide

nopeslide commented Mar 4, 2021

Copy link
Copy Markdown
Collaborator Author

@alrevuelta
I integrated the config (/config.h) generated by kconfig/genconfig into the opset generation.
We could now (but don't need to) generate the whole operator set for the ai.onnx domain (src/operators/ai.onnx/opdomain_operator__ai_onnx.c) without any change of our structure, operators, config or the resulting binary.
If we add a new operator via the template generator (and regenerate the config file) the new operator would be included automagically in the build process (because Kconfig adds the appropriate defines) :)
would this not simplify adding operators in general?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants