TensorRT icon indicating copy to clipboard operation
TensorRT copied to clipboard

feat: Fixed conv1d converter when weights are Tensor

Open andi4191 opened this issue 1 year ago • 8 comments

Description

This PR enables the conv1d -> conv2d converter mapping work when the weights are Tensor. PS: Kernel tensor is also unsqueezed when the filterDims are unsqueezed for conv1d -> conv2d scenario.

Please include a summary of the change and which issue is fixed. Please also include relevant motivation and context. List any dependencies that are required for this change.

Fixes # (issue)

Type of change

Please delete options that are not relevant and/or add your own.

  • Bug fix (non-breaking change which fixes an issue)

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
  • [ ] 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

andi4191 avatar Dec 14 '23 23:12 andi4191

@apbose : I see my PR is breaking in CI. Is the CI broken? I see some torch-tensorrt dependency related issue in the workflow run.

andi4191 avatar Dec 15 '23 18:12 andi4191

@andi4191 There was a problem with CI. I re-triggered it to verify if the latest changes fixed it.

peri044 avatar Dec 18 '23 21:12 peri044

@andi4191 Can you rebase your changes and resolve conflicts ?

peri044 avatar Dec 20 '23 02:12 peri044

@andi4191 Can you rebase your changes and resolve conflicts ?

Thanks, I resolved the conflicts. However, I still see CI build failure.

andi4191 avatar Dec 20 '23 03:12 andi4191

@bowang007 : Is there anything that I can do to merge this PR? Any update on the root cause for CI failure?

andi4191 avatar Jan 17 '24 20:01 andi4191

@bowang007 : Is there anything that I can do to merge this PR? Any update on the root cause for CI failure?

Hi @andi4191 thanks for supporting this. I think we have removed the CI test for torchscript, correct me if I'm wrong @narendasan. I'll take a look at this PR tonight and test locally, then I think it should be good to merge.

Thanks!

bowang007 avatar Jan 23 '24 22:01 bowang007

@bowang007 : Is there anything that I can do to merge this PR? Any update on the root cause for CI failure?

Hi @andi4191 thanks for supporting this. I think we have removed the CI test for torchscript, correct me if I'm wrong @narendasan. I'll take a look at this PR tonight and test locally, then I think it should be good to merge.

Thanks!

Thank you for the update @bowang007

andi4191 avatar Jan 26 '24 03:01 andi4191

Hi @bowang007,

Is this PR good to merge now?

andi4191 avatar Feb 02 '24 19:02 andi4191