Skip to content

Convert single layer caffe tests to onnx format#25581

Closed
WanliZhong wants to merge 1 commit intoopencv:5.xfrom
WanliZhong:singleCaffeLayer2onnx
Closed

Convert single layer caffe tests to onnx format#25581
WanliZhong wants to merge 1 commit intoopencv:5.xfrom
WanliZhong:singleCaffeLayer2onnx

Conversation

@WanliZhong
Copy link
Copy Markdown
Member

@WanliZhong WanliZhong commented May 14, 2024

Merged with: opencv/opencv_extra#1176 (testdata also should be review)
part of #25314

This PR is used to convert all caffe tests with 'single' layer to onnx format. Tests in test_int8_layer.cpp and test_caffe_importer.cpp will be removed in #25323

Most layers converted by caffe2onnx
Some of them are generated by pytorch and onnxruntime
Some of them are wrong after converting, so I use onnx-modifier to correct it.

EXTRA NOTE

UPDATE(2024-08): All below tests are kept because we decide to keep Caffe platform.

  • Remove layer_lrn_spatial: norm_region is WITHIN_CHANNEL , but ONNX defines the default mode as ACROSS_CHANNELS, and if users want it, they may try BatchNormalization. (ref: LRN in ONNX)

  • Change layer_deconvolution: onnx doesn't define deconvolution operator, use ConvTranspose to replace. (ref: ConvTranspose in ONNX)

  • Remove InnerProduct: ONNX doesn't define this layer, user may convert it to GEMM or MatMul

  • Change layer_mvn: onnx doesn't define MVN operator, use InstanceNormalization to replace.

  • Remove layer_batch_norm_local_stats: use_global_stats: false is useless in ONNX. (ref: BatchNormalization in ONNX)

  • Remove layer_eltwise: the model has a node with 2 inputs but from the same layer, which then causes an error. However, we have enough eltwise tests.

  • Remove accum and accum_ref: they use 1x2x2x4 accum 1x3x8x12, we can't use ADD operator to replace it.

  • Remove flow_wrap: ONNX doesn't define this operator.

  • Remove DataAugmentation: ONNX doesn't define this operator.

  • Remove nearest_2inps and nearest: ONNX doesn't define Resample operator.

  • Remove Correlation: ONNX doesn't define this operator.

  • Remove conv_2_inps: ONNX doesn't support this defination, no need to test it in ONNX.

  • Remove layer_interp: ONNX doesn't define Interp operator.

  • Disabled net_roi_pooling: can be replaced by RoiAlign operator in ONNX, but OpenCV dnn doesn't support.

  • Remove reshape_splice_split: Can't be converted to ONNX format. Because there are many re-use layers in caffe format.

Pull Request Readiness Checklist

See details at https://github.com/opencv/opencv/wiki/How_to_contribute#making-a-good-pull-request

  • I agree to contribute to the project under Apache 2 License.
  • To the best of my knowledge, the proposed patch is not based on a code under GPL or another license that is incompatible with OpenCV
  • The PR is proposed to the proper branch
  • There is a reference to the original bug report and related work
  • There is accuracy test, performance test and test data in opencv_extra repository, if applicable
    Patch to opencv_extra has the same branch name.
  • The feature is well documented and sample code can be built with the project CMake

@WanliZhong WanliZhong added test category: dnn cleanup Code cleanup (e.g, drop legacy C-API, legacy unmaintained code) OpenCV5 labels May 14, 2024
@WanliZhong WanliZhong added this to the 5.0 milestone May 14, 2024
@WanliZhong WanliZhong requested review from asmorkalov and dkurt May 14, 2024 08:29
@asmorkalov asmorkalov changed the title Convert single caffe tests to onnx format Convert single layer caffe tests to onnx format May 14, 2024
@WanliZhong
Copy link
Copy Markdown
Member Author

I removed some layer tests because ONNX doesn't support them. Please leave comments if I should try to keep some of them.

@asmorkalov
Copy link
Copy Markdown
Contributor

Please restore cafe-specific tests for now.

@asmorkalov
Copy link
Copy Markdown
Contributor

Friendly reminder.

@WanliZhong WanliZhong force-pushed the singleCaffeLayer2onnx branch from 0346cb5 to 7e4a326 Compare August 20, 2024 20:13
class Test_Caffe_layers : public DNNTestLayer
{
public:
void testLayerUsingOnnxModels(const String& basename, bool useCommonInputBlob = true, double l1 = 0.0, double lInf = 0.0,
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's misleading change. Test naming and hierarchy defines that the test are Caffe tests. I proposed to extract base class and 2 derivatives: Test_Caffe_Layers and Test_ONNX_Layers or just have TestLayers as common class. @vpisarev What do you think?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Onnx layers tests should be in test_onnx_importer.cpp. If we want to extract the tests for Caffe layer, I think we should move them to test_onnx_importer.cpp.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Keep tests as simple as possible. There are no tests for tests so test code should avoid complications.
It makes sense to create new set of ONNX tests. At least we don't break already existed tests.

@asmorkalov asmorkalov modified the milestones: 5.0, 5.0-release Sep 28, 2024
@asmorkalov asmorkalov closed this Jan 31, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

category: dnn cleanup Code cleanup (e.g, drop legacy C-API, legacy unmaintained code) OpenCV5 test

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

3 participants