Failling aidge_onnx tests on multiple operators

added StatusTODO label

assigned to @gregkub and @pineapple

added Bug 🐛 label

changed the description

mentioned in commit a34819f4

mentioned in commit fb317159

How was this not detected by CI? :/

added StatusWork in Progress label and removed StatusTODO label

I found some more errors/warnings in https://gitlab.eclipse.org/eclipse/aidge/aidge_onnx/-/jobs/476900

Warning: Unsupported attribute 'dilations' with value [2, 2, 2] for operator 'AveragePool' with opset 19. This node will be filled by a GenericOperator. -> due to unsupported 3D avgpool
Warning: Unsupported attribute 'count_include_pad' with value 1 for operator 'AveragePool' with opset 19. This node will be filled by a GenericOperator.
Warning: Unsupported attribute 'ceil_mode' with value 1 for operator 'AveragePool' with opset 19. This node will be filled by a GenericOperator.
Unsupported attribute 'transA' with value 1 for operator 'Gemm' with opset 13. This node will be filled by a GenericOperator.
cannot import bias of dims: [3, 4] for operator 'Gemm' with opset 13.
Unsupported attribute 'beta' with value 0.5 for operator 'Gemm' with opset 13. This node will be filled by a GenericOperator.
Unsupported attribute 'alpha' with value 0.5 for operator 'Gemm' with opset 13. This node will be filled by a GenericOperator.
Unsupported attribute 'dilations' with value [2, 2] for operator 'MaxPool' with opset 12. This node will be filled by a GenericOperator.
Unsupported attribute 'dilations' with value [2, 2, 2] for operator 'MaxPool' with opset 12. This node will be filled by a GenericOperator.
Pad class Pad4D is not implemented for operator pad.[0- y (Pad | GenericOperator)
missing or invalid registrar key: [Float32, Default, [(3, 3)], Float64, Default, [(1, 1)], Float64, Default, [(1, 1)]], [Float32, Default, []] for registrable object N5Aidge16OperatorImpl_cpuINS_7Clip_OpEFvffPKvmPvEFvffmS3_S3_S4_EEE
missing or invalid registrar key: [Float32, Default, [(3, 3), (4, 4), (5, 5)], Float64, Default, [(1, 1)], Any, Any, []], [Float32, Default, []] for registrable object N5Aidge16OperatorImpl_cpuINS_7Clip_OpEFvffPKvmPvEFvffmS3_S3_S4_EEE
missing or invalid registrar key: [UInt8, Default, [(4, 4)], Int64, Default, [(1, 1)]], [Float32, Default, []] for registrable object N5Aidge16OperatorImpl_cpuINS_6Sub_OpEFvSt6vectorImSaImEES4_RKS4_PKvS8_PvEFvvEEE
`Unsupported attribute 'block_size' with value 2 for operator 'DequantizeLinear' with opset 21. This node will be fill- y (DequantizeLinear | GenericOperator)
- axis : 1
- block_size : 2`
Assertion failed: op.getInput(1)->dataType() == op.getInput(0)->dataType() in /builds/eclipse/aidge/aidge_backend_cpu/src/operator/PowImpl.cpp:37
missing or invalid registrar key: [Float32, Default, [(6, 6)], Float64, Default, [(1, 1)]], [Float32, Default, []] for registrable object N5Aidge16OperatorImpl_cpuINS_6Div_OpEFvmmmPKvS3_PvEFvvEEE
Warning: Unsupported attribute 'block_size' with value 2 for operator 'QuantizeLinear' with opset 21. This node will be filled by a GenericOperator.

You might need to cherry-pick a34819f4 & fb317159 I mentionned in this issue to solve part of the problem.

I updated the todo list for this issue however I didn't include all the warning you mentioned @pineapple, I left the ones concerning the types mismatch because this doesn't only concern the mentioned ops but all the ops that take more than 1 input

I found why the error has been "undercover" previously : its not the same test command that is used for tests in test & in release jobs.

test

python -m xmlrunner discover -s unit_tests/ -v -b --output-file xmlrunner-results.xml

release

python -m pytest unit_tests/*

I remember initially trying to pass from unittest to pytest command in ubunut_python:test jobs but failing to properly detect all tests and hence simply go back to previous command. However, since tests added in !83 (merged) are purely based on pytests they cannot be discovered by unittest vanilla.

I see!

I think in the future we need to bring back these yaml to the repositories and not have them in gitlab_shared_files. It makes the CI really hard to understand and add little value.

Well well seems there is some work here once tests are fixed

I'll put my 2 cents here : I don't think this is a good idea.

This will create huge code duplication.
Maintenance : It means that each fix in the CI leading to 6+ MR (cf recent fixes on release pipelines on the go).
This architecture of having template files is conventional and considered good practice. The fact that you are not used to the convention doesn't mean that it doesn't add value.
Scaling to new projects will be really simpler.

Previous version used function that were parametrizable and mutualized. The current version is not configurable and hard to understand (but very mutualized).

Scaling to new project is not simpler as you need to update static lists in multiple files that are not organized following gitlabci convention. I think all of this is not "good practice".

So in conclusion I am in favor of simplifying the CI by adding back yml files where they belong and avoiding code duplication by creating function that are configurable as it was the case.

These issues have been identified and could be solved by :

Solving gitlab_shared_files#1
Using Docker to create dedicated environment for each repo

Not the only issue, this is not good. https://gitlab.eclipse.org/eclipse/aidge/gitlab_shared_files/-/blob/main/.gitlab/ci/ubuntu_python.gitlab-ci.yml?ref_type=heads#L50-53

solved by having dedicated docker

This means updating a dockerfile instead of a yml.

Yeah I am really strongly against having dedicated docker for each modules it add another layer of complexity to an already complex CI.

A dedicated docker with cuda installed is required for reducing the time for a release pipeline for example. But we cannot have all the environment in a docker which will induce the need to update a dockerfile, rebuild and republish. This is why I prefer for this kind of dependencies to add them as a pip install.

I also would be in favor of 1 docker for cuda & 1 for the "casual" repos (& 1 for opencv)

Most of the if / else done here in the CI are just here to avoid re installing a whole library when it is not needed because a lib install costs more than pulling a docker.

The ideal would be to have these two dockers stored in our servers to minimize download time.

relates to &11

changed title from Failling aidge_onnx tests on resize to Failling aidge_onnx tests on multiple operators

added StatusReview Ready label and removed StatusWork in Progress label

added StatusWork in Progress label and removed StatusReview Ready label

@pineapple You told me people needed to to have some tests fixed. I am awaiting for a priority list to know which one I should fix 1st.

Otherwise these operators will be fixed in my preferred order.

added StatusTODO label and removed StatusWork in Progress label

assigned to @hrouis and unassigned @pineapple

changed the description

mentioned in merge request aidge_core!311 (merged)

Failling aidge_onnx tests on multiple operators

TODO

Designs

Child items ...

Activity