Getting strange split and Lambda function when converting onnx

[kobygold]

Hi,
Trying to convert a pretty simple onnx to keras I'm getting a strange split from the input and 2 Lambda functions...
I would expect the output h5 to be similar to the onnx file, i.e. without the split and those Lambda functions that are not on the source onnx file.

Attached you can see a screenshot and both onnx and h5 files in zip file.

onnx+h5.zip

Thanks,
Koby

[kobygold]

I managed to narrow down the problem: the split and the new Lambda function were generated due to the "Add" layer (the last one in the graph). This "Add" layer was originated from a FullyConnected (Dense) layer that was transformed to MatMul+Add (because as far I know ONNX doesn't support FullyConnected (Dense)). The onnx2keras conversion manages to convert the MatMul into Dense in Keras, but the following Add is not understood by the converter as the 2nd part of the Dense, and thus it is added as a Lambda layer.
To validate that I managed to create an ONNX file without the "Add" layer, and the conversion was made correctly without the split and the Lambda function (see screenshot below).
Is it possible to fix that issue, and add to the onnx2keras converter the option to understand that an "Add" layer following a "MatMul" layer should be treated as a bias to the same "Dense" layer in Keras?

Thanks,
Koby

[kobygold]

I managed to fix it, and add the bias as part of the Dense layer!
Let me know if you want me to push that fix to the repo, so someone could review the code change and hopefully merge it.
Do you have some testing methodology, like CI/CD, for testing such changes, to make sure a certain fix doesn't cause an error somewhere else (i.e. to make sure other models still work fine)?

Thanks,
Koby

[kobygold]

After I got to know your code better, I think I managed to support better the 'channels_last' option (in the code it is called: change_ordering=True) which is Keras's default (channels_last = NHWC). And when doing that I added a Transpose at the beginning of the model to change the input from 'channels_first' (ONNX) to 'channels_last' (Keras default). I've also checked if the model has already Transpose layer at the beginning and if so merged it with the input conversion Transpose I add (some ONNX models have 'channels_last' to 'channels_first' Transpose at the beginning, if they were created from a 'channels_last' model themselves). I also check if those Transpose cancel one another (i.e. 'channels_last' -> 'channels_first' -> 'channels_last') and remove the Transpose altogether instead of keeping two Transpose layers. I also managed to merge Activation layers that follow Conv layer into the Conv layer, and also merge ZeroPadding2D that precedes Conv into the Conv as well (to some extent, because Keras supports only 'same' and 'valid' padding on the Conv layers, where ONNX has more flexibility by using pads array). See the screenshot below with all of my changes (you can compare to the previous screenshot I've sent), and let me know if you would like me to submit all those changes to your repository, for you to review and hopefully merge.

p.s.: On the screenshot you can see that the Transpose layers were removed, that's because the ONNX model I've used had exactly the opposite channels conversion Transpose inside, so after converting that to 'channels_last' they were both canceling each other and no Transpose was needed.

Thanks,
Koby

[tea1528]

@kobygold, hi can you share your code on merging MatMul and Add to Dense layer?

[kobygold]

Hi @tea1528,
I tried to push now a branch with my code that merges MatMul + Add to Dense layer (without my other changes),
but it seems I don't have permission to do that, or need to do it differently. I got this error message:

remote: Permission to gmalivenko/onnx2keras.git denied to kobygold.
fatal: unable to access 'https://github.com/gmalivenko/onnx2keras.git/': The requested URL returned error: 403

My change is to add these few lines in onnx2keras/converter.py right after line 120:

        # Check if current layer is MatMul and next layer is Add, and merge them together into a single Dense layer
        if node_type == 'MatMul' and node_index < len(onnx_nodes)-1 and onnx_nodes[node_index+1].op_type == 'Add':
            next_node = onnx_nodes[node_index+1]  # Grab the next layer
            node.input.append(next_node.input[1])  # Take the Bias from the next layer ("Add") and place in this layer ("MatMul")
            onnx_nodes[node_index + 1].op_type = 'Identity'  # Mark the next layer as 'Identity' to have no effect on the output anymore

p.s.: My other changes regarding the treatment of channels_last and adding additional Transpose layers (and merging two sequential Transpose layers into one (or none)) is not generic enough, and will not always work correctly. I need to think how to make it more generic to work in all networks (basically only CONV based layers need such Transpose, while Dense doesn't need that, so I have to add this Transpose before each CONV layer, and add Transpose back after each CONV layer, while trying to avoid multiple Transpose layers as much as possible).