fix/feat: Move convolution core to impl + add feature (FX converter refactor)

[gs-olive]

Description

• Centralize convolution implementation in FX across all source IR variants, including support for conv1d, quantized, and other configurations
• Update reference implementations across the stack to use centralized utility and remove individual replicated implementations
• Allow conv layers to take bias inputs in FX, per new functionality from TRT
• Enable pass-through of build errors in Dynamo e2e tests to ensure errors are not being hidden (this PR fixes a bug which disallowed that pass-through)

Fixes #1954
Addresses first bug in #1565

Type of change

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)

Checklist:

• [ x ] My code follows the style guidelines of this project (You can use the linters)
• [ x ] I have performed a self-review of my own code
• [ x ] I have commented my code, particularly in hard-to-understand areas and hacks
• [ x ] I have made corresponding changes to the documentation
• [ x ] I have added tests to verify my fix or my feature
• [ x ] New and existing unit tests pass locally with my changes
• [ x ] I have added the relevant labels to my PR in so that relevant reviewers are notified

[gs-olive]

Did not add an unsqueeze operation to bias term since the requirement in TRT for the bias term is that it must have number of elements equal to the number of output features of the convolution, so the same bias as is used for Conv1D would work for Conv2D, with the number of output features being fixed.

[frank-wei]

just to clarify, you mean 1D or 2D conv? For 1D, we need bias to be unsqueezed.

[gs-olive]

I initially meant for all conv layers since this documentation seems to indicate we just need the number of elements in the bias Tensor to the be correct, and not necessarily the dimensions, but if bias needs to be unsqueezed for 1D, I can add that functionality back. I am wondering if the intended unsqueeze should be in the first dimension (torch.unsqueeze(bias, 0)) or the last dimension (torch.unsqueeze(bias, -1))?

Note: I think initially, it was torch.unsqueeze(bias, 0), while the weights and inputs were unsqueezed in the last dimension

[frank-wei]

I am wondering if TRT has done the broadcast internally since the unit test for 1D works good even though you did not unsqueeze it.

[gs-olive]

I verified on a small sample that Conv1D with bias compiles + runs inference successfully without unsqueezing the bias term

[narendasan]

Should we have a unit test for this just to catch if TRT behavior changes?

[gs-olive]

I could add one, but it would likely be very similar to this case, which interprets, builds, and runs inference on a Conv1D model with TRT both with and without bias:

TensorRT/py/torch_tensorrt/fx/test/converters/acc_op/test_convolution.py

Lines 8 to 45 in cae6b7c

	class TestConvolutionConverter(AccTestCase):
	@parameterized.expand(
	[
	("default", 1),
	param("no_bias", 1, bias=False),
	("tuple_parameters", 1, (1), (1)),
	param("non_zero_padding", 1, padding=1),
	param("dilation", 1, dilation=2),
	param("groups", 1, groups=3),
	]
	)
	def test_conv1d(
	self,
	_,
	kernel_size,
	stride=1,
	padding=0,
	dilation=1,
	groups=1,
	bias=True,
	):
	class TestModule(torch.nn.Module):
	def __init__(self):
	super().__init__()
	self.conv = torch.nn.Conv1d(
	3, 6, kernel_size, stride, padding, dilation, groups, bias
	)
	def forward(self, x):
	return self.conv(x)
	inputs = [torch.randn(1, 3, 32)]
	self.run_test(
	TestModule(),
	inputs,
	expected_ops={acc_ops.conv1d},
	test_explicit_precision=True,
	)

A breaking TRT change to that specific case should cause the accuracy check in the above test to fail.

[narendasan]

Mostly organization stuff

[narendasan]

Should we have a unit test for this just to catch if TRT behavior changes?

[gs-olive]

HI @wushirong - thank you for the review. I was wondering if you could have another look at the changes, as I've moved convolution.py contents to nn_ops_converters.py for code cleanliness + organization and removed unused imports, in response to review comments by @narendasan