clean up mult-dimensional dense #1042
Conversation
| @@ -58,11 +58,8 @@ def transform(self, model, node): | |||
| else: | |||
| raise Exception('Cannot replace Dense over {dim}D tensor with Conv{dim}D.'.format(dim=dim)) | |||
|
|
|||
| class_name = 'PointwiseConv' + str(dim) + 'D' | |||
| class_name = 'Conv' + str(dim) + 'D' | |||
| pw_node = model.make_node(class_name, node.name, pointwise_attrs, node.inputs.copy()) | |||
There was a problem hiding this comment.
pw_codecould be renamed to conv_node, and similar things throughout the file, to get rid of the pointwise-related nomenclature.
| ('Vivado', 'io_stream'), | ||
| ('Vivado', 'io_stream'), | ||
| ('Vitis', 'io_stream'), | ||
| ('Vitis', 'Latency'), |
There was a problem hiding this comment.
We don't need to test Vivado anymore?
There was a problem hiding this comment.
I wonder if I got ahead of myself. I was thinking to keep the tests shorter, since Vivado support will soon be on the way out, and it shares things with Vitis, but maybe it shouldn't be just yet. The tests aren't very long. I can add it.
There was a problem hiding this comment.
I was mostly curious if we are starting to phase out Vivado. Still catching up on the status and plans for things :) I guess it makes sense, so I don't think you necessarily need to add them.
There was a problem hiding this comment.
I think the plan was that after 1.0 we make Vitis the default backend and restructure the code a bit. I think it makes sense to add Vivado. (There's a similar Quartus -> oneAPI evolution that will happen.)
Description
Multidimensional Dense was broken for cases where any dimensions had size 1, and for Quartus parallel. I believe it was also broken for the resource strategy.
I made the change for
ReplaceMultidimensionalDenseWithConvto convertDensetoConv1Dor2Dinstead of directly toPointwiseConv1Dor2D, since pointwise is not required to be implemented in all cases. The conv to pointwise optimizer is backend-specific, while this is a backend-agnostic optimizer. This keeps the separation clear. Of course, the conv to pointwise optimizer usually runs after this, so in the end you still end up with a pointwise convolution (if it exists).Also, the reshaping of the weights is now done once, in the
ReplaceMultidimensionalDenseWithConvoptimizer, instead of incorrectly there (I think it was correct originally but from before some code modernization) and also in the pointwise optimizers.Type of change
Tests
The existing
test_multi_dense.pywas expanded to cover the broken cases.Checklist
pre-commiton the files I edited or added.