Add and modify operators to run the ConvNeXt onnx model
Context
While trying to export the ConvNeXt onnx model to cpp, we encountered several issues. First, several operators were missing, including transpose and erf. Additionally, we needed to add broadcasting support for operators like add, sub, mul, div, and matmul. For the convolution layers, we added the groups parameter. Lastly, batch normalization and global average pooling required debugging. Specifically, batch normalization had issues, and for global average pooling, there was a type error that resulted in incorrect values. ConvNeXt_export.onnx
Modified files
- added erf and transpose_diff and matching jinja files
- added convolution_groups to have the parameter groups (not to interfer with original convolution model)
- operator.py : add erf, transpose, convolution_groups operators
- pooling global : modified the type of sum for Average
- batchnorm : updated to perform a batchnorm and changed the parameters order
- modified matmul : for broadcasting support
- added transpose_diff (not to interfer with original model because i wasn't able to make it work)
Merge request reports
Activity
assigned to @mnewson
Good first MR!
For your information @mnewson I am working on adding unit test for CPP export. !24 (closed)
Will you be able to create some unit test on the operator you created?
If so, we can create issues on the operators to create and assign you to some
Cheers,
Cyril
Thank you! I can try to create unit tests for the operators I added. What should I use as a reference to compare the operator being tested? I used ONNX runtime to compare the results. I believe this is necessary in certain cases, like with
convolution_groups
, since thegroups
parameter isn’t currently available in the Aidge core.
changed milestone to %aidge v0.6.0
added Feature 🚀 StatusReview Ready labels
- Resolved by Olivier BICHLER
Note: This is not documented yet but @olivierbichler worked on a feature to skip transpose thanks to the memory manager, so this kernel should not be necessary.
Also what is transpose_diff? Expecially:
(not to interfer with original model because i wasn't able to make it work)
Did you find an issue with current Transpose?
- Resolved by Matthew Newson
- Resolved by Matthew Newson
- Resolved by Matthew Newson
- Resolved by Matthew Newson
- Resolved by Matthew Newson
- Resolved by Matthew Newson
- Resolved by Matthew Newson
Beware of include you used to debug the export!
Some typo need to be changed.
I did not review the content of the code as the MR is quite big.
I would be in favor of adding unit test on it.
Note: we need to update &2
22 53 const Input_T* __restrict inputs, 23 54 Output_T* __restrict outputs) 24 55 { 56 57 float inputs_ordered[NB_CHANNELS * CHANNELS_HEIGHT * CHANNELS_WIDTH]; 58 float outputs_unordered[NB_OUTPUTS * OUTPUTS_HEIGHT * OUTPUTS_WIDTH]; 59 reorder_NCHW_NHWC_pool(inputs, inputs_ordered, 1, NB_CHANNELS, CHANNELS_HEIGHT, CHANNELS_WIDTH, true); This was necessary to obtain the correct output in certain cases, because the format NHCW needs to be inverted. There was a similar case with the convolution which Clément Fisher corrected, if I remember correctly. Just so you know, I used the ONNX runtime to compare with the Aidge export, not the Aidge core.
@cmoineau Changes are needed or is that ok for you ?
We need to talk about it with @axelfarr @mick94 and @olivierbichler A lot of MR have been proposed for export cpp and we are too overwhelmed to respond right now.
hi @mnewson. We should be able to give you some guidelines for May 7. The goal is to have this feature in the next milestone...
mentioned in merge request !24 (closed)
added 4 commits
-
f06fd92b...45d12a67 - 2 commits from branch
eclipse/aidge:dev
- 5389bb9e - Merge remote-tracking branch 'origin/dev'
- 9fc76615 - Delete debug includes
-
f06fd92b...45d12a67 - 2 commits from branch