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

fix: Improve input weight handling to acc_ops convolution layers in FX #1886

Merged
merged 1 commit into from
May 10, 2023
Merged

fix: Improve input weight handling to acc_ops convolution layers in FX #1886

merged 1 commit into from
May 10, 2023

Conversation

gs-olive
Copy link
Collaborator

@gs-olive gs-olive commented May 4, 2023

Description

  • In line with recent deprecation of the kEXPLICIT_PRECISION flag in TRT
  • Add improved logic for weight handling in conv layers to bring all accelerated conv layers into agreement and fix errors in the Dynamo path arising from TRTTensor inputs
  • Fix minor typos in error messages

Fixes #1885

Type of change

  • 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
  • [ 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
fix: Input weight handling to accelerated conv
a3b654f 
- In line with recent deprecation of the kEXPLICIT_PRECISION flag in TRT
- Add improved logic for weight handling in `conv` layers to bring all
accelerated conv layers into agreement and fix errors in the Dynamo path
arising from TRTTensor inputs
- Fix minor typos
@gs-olive gs-olive added priority: high release: v1.4 Tagged to be included in v1.4 labels May 4, 2023
@gs-olive gs-olive requested review from frank-wei and wushirong May 4, 2023 22:56
@gs-olive gs-olive self-assigned this May 4, 2023
@github-actions github-actions bot requested a review from narendasan May 4, 2023 22:56
gs-olive
gs-olive commented May 4, 2023
View reviewed changes
py/torch_tensorrt/fx/converters/acc_ops_converters.py
)
bias = to_numpy(kwargs["bias"]) # type: ignore[arg-type]

if network.has_explicit_precision:
if network.has_explicit_precision or isinstance(kwargs["weight"], TRTTensor):
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Mirrors the existing code for acc_ops.conv1d

@gs-olive gs-olive added the component: dynamo Issues relating to the `torch.compile` or `torch._dynamo.export` paths label May 4, 2023
@gs-olive gs-olive mentioned this pull request May 5, 2023
Closed
frank-wei
frank-wei approved these changes May 10, 2023
View reviewed changes
Copy link
Contributor

@frank-wei frank-wei left a comment

Choose a reason for hiding this comment

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

LGTM!

@gs-olive gs-olive merged commit 39585b1 into pytorch:main May 10, 2023
@gs-olive gs-olive deleted the acc_conv_bugfix branch May 10, 2023 22:21
gs-olive added a commit that referenced this pull request May 10, 2023
@gs-olive
fix: Improve input weight handling to acc_ops convolution layers in…
9d2bd91 
… FX (#1886)
@gs-olive gs-olive mentioned this pull request May 10, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@frank-wei frank-wei frank-wei approved these changes

@wushirong wushirong Awaiting requested review from wushirong

@narendasan narendasan Awaiting requested review from narendasan

Assignees

@gs-olive gs-olive

Labels
cla signed component: api [Python] Issues re: Python API component: dynamo Issues relating to the `torch.compile` or `torch._dynamo.export` paths component: fx fx priority: high release: v1.4 Tagged to be included in v1.4
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

🐛 [Bug] Encountered bug when compiling model with acc_ops conv converter
3 participants
@gs-olive @frank-wei @facebook-github-bot