Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

feat: support conv dynamo converter #2252

Merged
merged 2 commits into from
Aug 30, 2023
Merged

feat: support conv dynamo converter #2252

merged 2 commits into from
Aug 30, 2023

Conversation

zewenli98
Copy link
Collaborator

@zewenli98 zewenli98 commented Aug 21, 2023 •
edited
Loading

Description

Support conv dynamo converter.

Fixes #2250, #2185

Type of change

  • New feature (non-breaking change which adds functionality)

Checklist:

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

@github-actions github-actions bot added component: api [Python] Issues re: Python API component: conversion Issues re: Conversion stage component: converters Issues re: Specific op converters component: dynamo Issues relating to the `torch.compile` or `torch._dynamo.export` paths labels Aug 21, 2023
gs-olive
gs-olive reviewed Aug 23, 2023
View reviewed changes
Copy link
Collaborator

@gs-olive gs-olive left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great to have this converter in Dynamo. Made some suggestions for fixes from issues encountered in the FX path.

py/torch_tensorrt/dynamo/conversion/impl/conv.py Show resolved Hide resolved
py/torch_tensorrt/dynamo/conversion/impl/conv.py Outdated Show resolved Hide resolved
py/torch_tensorrt/dynamo/conversion/impl/conv.py Outdated Show resolved Hide resolved
@zewenli98 zewenli98 mentioned this pull request Aug 24, 2023
Closed
gs-olive
gs-olive reviewed Aug 24, 2023
View reviewed changes
Copy link
Collaborator

@gs-olive gs-olive left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added a small comment on the changes to the FX directory. After that, just needs a rebase and should be good to go!

py/torch_tensorrt/fx/converters/converter_utils.py Outdated Show resolved Hide resolved
@zewenli98 zewenli98 force-pushed the conv_dynamo_converter branch 2 times, most recently from adc7295 to 17003c4 Compare August 24, 2023 21:31
@zewenli98
Copy link
Collaborator Author

Thanks! rebased!

@zewenli98
feat: support conv dynamo converter
0644709 
fix a squeeze bug

minor fix and issue pytorch#2185

add conv validator
gs-olive
gs-olive reviewed Aug 29, 2023
View reviewed changes
Copy link
Collaborator

@gs-olive gs-olive left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See below comments on calling convention for converters, then should be good to go!

py/torch_tensorrt/dynamo/conversion/impl/conv.py Outdated Show resolved Hide resolved
py/torch_tensorrt/dynamo/conversion/impl/conv.py Outdated Show resolved Hide resolved
py/torch_tensorrt/dynamo/conversion/impl/conv.py Outdated Show resolved Hide resolved
gs-olive
gs-olive approved these changes Aug 29, 2023
View reviewed changes
Copy link
Collaborator

@gs-olive gs-olive left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great! Approved, pending CI

@gs-olive gs-olive mentioned this pull request Aug 30, 2023
Merged
@gs-olive gs-olive merged commit d43c5bb into pytorch:main Aug 30, 2023
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@gs-olive gs-olive gs-olive approved these changes

@narendasan narendasan Awaiting requested review from narendasan

Assignees
No one assigned
Labels
cla signed component: api [Python] Issues re: Python API component: conversion Issues re: Conversion stage component: converters Issues re: Specific op converters component: dynamo Issues relating to the `torch.compile` or `torch._dynamo.export` paths component: fx
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Expose IConvolutionLayer in dynamo.conversion.impl
3 participants
@zewenli98 @gs-olive @facebook-github-bot