Extra InferDataLayouts in ConvertQONNXtoFINN breaks QONNX MobileNetV1

[fpjentzsch]

The data layout inference I added here (3e59fd3) to fix the cnv data layout issue seems to break the MobileNetV1 flow based on a QONNX input model (the old FINN-ONNX version in the regression test suite is fine) that is exercised in test_end2end_mobilenet_tidy_and_merge_with_preproc:

  assert (
        thresholds.shape[0] == 1 or thresholds.shape[0] == num_output_channels
    ), """Quant node cannot be converted to MultiThreshold because only
        per tensor or per channel quantization supported."""

E AssertionError: Quant node cannot be converted to MultiThreshold because only
E per tensor or per channel quantization supported.

The threshold shape is (64, 15) and num_output_channels is 111, which looks like the spatial dimension.

[fpjentzsch]

Interestingly, the QONNX conversion unit test has been marked xfail for MobileNet for 2 years:

finn-plus/tests/transformation/test_qonnx_to_finn.py

Line 92 in d57bbc4

pytest.xfail("MobileNet test is temporarily excluded from QONNX testing.")

Maybe @auphelia knows why?

[auphelia]

Interestingly, the QONNX conversion unit test has been marked xfail for MobileNet for 2 years:

finn-plus/tests/transformation/test_qonnx_to_finn.py

Line 92 in d57bbc4

pytest.xfail("MobileNet test is temporarily excluded from QONNX testing.")
Maybe @auphelia knows why?

If I recall correctly, then there was a rounding error from the PyTorch model to the exported QONNX, so that the functional verification failed. I believe it had something to do with the definition of the QuantAvgPool layer, which was revised recently in Brevitas (Xilinx/brevitas#1042). These changes still need to be propagated into qonnx and finn.
We never had time to track the PyTorch/QONNX bug down and solve it but we didn't see a big drop in overall accuracy, the QONNX model and the FINN-ONNX model should be functionally equivalent.

[fpjentzsch]

@iksnagreb After the first Quant node, the layout is incorrectly annotated as NHWC:

So it seems we might have a chicken/egg problem. MT conversion requires proper input data layout annotation to set the data_layout attribute properly, which is required in the non-standard case of a NCHW MT that is (inelegantly) handled in InferThresholdingLayer by an ad-hoc conversion to a NHWC MT. But data layout inference always assumes NHWC output layout for a Quant node, which is just plain wrong? Shouldn't we make the data layout inference propagate the layout from the input for "finn ops" like Quant, like it already does for standard ONNX operators?

[fpjentzsch]

Something similar actually happens in the test_streamline_cnv tests that are currently failing with "Threshold shape incorrect":

Here, everything after the first Conv is incorrectly annotated as NHWC. But the Conv is not a "finn op", so it happens because the Conv input is not annotated, not because the output layout is always assumed to be NHWC. Curiously, the second Conv has an (incorrect) input layout annotation, but no output annotation.

Seems like we need to call InferDataLayout in a few more places. Maybe some transforms also inadvertently remove layout annotations from tensors without calling InferDataLayout again, resulting in an illegal graph.

[fpjentzsch]

@iksnagreb I was able to fix both of these issues in #87 by adding basic input layout propagation here: fpjentzsch/qonnx@fab7dca

Some tensors were still left un-annotated after streamlining, so, for good measure, I also added layout inference after MT conversion and after every round of the default streamlining transformation: 91bc6a0

So far it doesn't seem to break anything.