Strided pointwise convolution on rectangular feature maps not correctly handled by InferConvInpGen

[hleblevec]

Quick summary

The tool does not correctly handle strided pointwise convolution with rectangular feature maps anymore. With the previous build, I would use the RTL variant of ConvInpGen which supports this config, but now the transformation is trying to infer a DownSampler node because it detects a strided pointwise Im2Col, but the DownSampler node does not work with rectangular input.

Details

Steps to Reproduce

1. Use main branch with release 0.10.0
2. Have a model with a Convolution having stride>1 and kernel_size=1 but with non-square feature maps.
3. Decompose into Im2Col + MatMul in the streamlining step.
4. Run step_convert_to_hw_layers.

Expected behavior

ConvInpGen node created with this configuration.

Actual behavior

Triggers a warning:

finn/src/finn/transformation/fpgadataflow/convert_to_hw_layers.py

Line 133 in e3087ad

warnings.warn(f"Couldn't infer Downsample from {n.name},check config.")

Optional

Possible fix

1. Is it still relevant to have a DownSampler node for the specific case of strided pointwise convolutions? Does it work better than a ConvInpGen in this specific scenario?
2. If yes, a possible fix would be, in the case of a rectangular input, to move to normal ConvInpGen in the InferConvInpGen transformation rather than skipping the conversion. This node would need to be rewired to the RTL variant in the SpecializeLayers transformation, which shouldn't be a problem because the _swg_hls_possible function verifies that the input is square.
   

   finn/src/finn/transformation/fpgadataflow/specialize_layers.py

   Lines 201 to 233 in e3087ad

   	def _swg_hls_possible(node):
   	# there are some constraints to
   	# the HLS variant of the SWG
   	# first constraint to check is
   	# if user has set dynamic_mode to 1
   	# this is only supported in rtl variant
   	swg = getCustomOp(node)
   	if swg.get_nodeattr("dynamic_mode"):
   	return False
   	# the 2D HLS implementation for SWG
   	# can only be used for square inputs
   	# and no dilation
   	if swg.get_nodeattr("is1D"):
   	return True
   	else:
   	# extract all attributes to check
   	k = swg.get_nodeattr("ConvKernelDim")
   	ifm_dim = swg.get_nodeattr("IFMDim")
   	ofm_dim = swg.get_nodeattr("OFMDim")
   	s = swg.get_nodeattr("Stride")
   	d = swg.get_nodeattr("Dilation")
   	# check if square and dilation=1
   	if (
   	k[0] == k[1]
   	and ifm_dim[0] == ifm_dim[1]
   	and ofm_dim[0] == ofm_dim[1]
   	and s[0] == s[1]
   	and d[0] == d[1] == 1
   	):
   	return True
   	else:
   	return False

[auphelia]

Hi @hleblevec,
Thanks for pointing that out! I actually think we could integrate the functionality of the DownSampler into the ConvolutionInputGenerator custom op structure as one additional variant. This would make this extra node obsolete. Let me know if that would be something you would like to contribute to the repo yourself!

[hleblevec]

Hi @auphelia,
Yeah sure! If I understood well, it would be a variant introduced by SpecializeLayers depending on the given parameters?

[auphelia]

Yes, so it requires a little bit of refactoring work. But the idea would be to delete DownSampler + DownSampler_hls and convert for the previous cases also to ConvolutionInputGenerator in convert_to_hw_layers and then in SpecializeLayers you would default to the RTL variant but with a square or 1D input, you could also specialize to HLS if the user wants that.
That would mean that the functionality of DownSampler_hls needs to be integrated into ConvolutionInputGenerator_hls as an additional variant. It is similar to the integration of the previous custom op ConvolutionInputGenerator1D that was summarized with ConvolutionInputGenerator into ConvolutionInputGenerator_hls. If you need additional pointers, you can reach out via email and we can schedule a meeting to discuss the details.