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

OnnxToTorch support for onnx.InstanceNormalization op #2710

Merged
merged 1 commit into from
Feb 19, 2024
Merged

OnnxToTorch support for onnx.InstanceNormalization op #2710

merged 1 commit into from
Feb 19, 2024

Conversation

aldesilv
Copy link
Contributor

@aldesilv aldesilv commented Dec 29, 2023 •
edited by AmosLewis
Loading

@aldesilv aldesilv force-pushed the in branch 6 times, most recently from fb3c6c5 to 7c54a46 Compare January 9, 2024 17:21
@AmosLewis AmosLewis mentioned this pull request Jan 9, 2024
Closed
@vivekkhandelwal1
Copy link
Collaborator

Hi @aldesilv, I did take a look at your code and I think there's an issue with the dtype check for the instance norm op. The PyTorch doesn't support the device cpu/meta (both of which I tried) for the tensors to this op. I think for now you can just add a comment saying the device is not supported hence unable to check the dtype function and remove the check.

@ramiro050, would you like to add something to this?

@aldesilv aldesilv marked this pull request as ready for review January 11, 2024 17:21
@aldesilv aldesilv force-pushed the in branch 3 times, most recently from 9877011 to 353be55 Compare January 15, 2024 20:06
vivekkhandelwal1
vivekkhandelwal1 requested changes Jan 16, 2024
View reviewed changes
Copy link
Collaborator

@vivekkhandelwal1 vivekkhandelwal1 left a comment

Choose a reason for hiding this comment

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

Hi @aldesilv, can you please add some e2e tests for quantized::instance_norm op so that your TorchToLinalg lowering can be verified?

lib/Conversion/TorchOnnxToTorch/DefaultDomainGtoP.cpp Outdated Show resolved Hide resolved
@aldesilv aldesilv force-pushed the in branch 2 times, most recently from bb8d9a4 to e4e9670 Compare January 18, 2024 19:56
@aldesilv
Copy link
Contributor Author

Hi @aldesilv, can you please add some e2e tests for quantized::instance_norm op so that your TorchToLinalg lowering can be verified?

when using quantized instance norm (torch.ao.nn.quantized.InstanceNorm2d) in an e2e test,
NotImplementedError: Could not run
'quantized::instance_norm' with arguments from the 'CPU' backend. This could be because the operator doesn't exist for this backend, or was omitted during the selective/custom build process (if using custom build).

@aldesilv aldesilv force-pushed the in branch 3 times, most recently from 8483964 to 69343c3 Compare January 20, 2024 01:17
@aldesilv
Copy link
Contributor Author

Hi @aldesilv, can you please add some e2e tests for quantized::instance_norm op so that your TorchToLinalg lowering can be verified?

when using quantized instance norm (torch.ao.nn.quantized.InstanceNorm2d) in an e2e test, NotImplementedError: Could not run 'quantized::instance_norm' with arguments from the 'CPU' backend. This could be because the operator doesn't exist for this backend, or was omitted during the selective/custom build process (if using custom build).

will add a test in torch-mlir/test/Conversion/TorchToLinalg

@aldesilv aldesilv force-pushed the in branch 2 times, most recently from bf4d751 to 1cdbb20 Compare January 30, 2024 19:19
ramiro050
ramiro050 requested changes Jan 31, 2024
View reviewed changes
Copy link
Collaborator

@ramiro050 ramiro050 left a comment

Choose a reason for hiding this comment

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

Appologies for the late review. I have one question

lib/Conversion/TorchOnnxToTorch/DefaultDomainGtoP.cpp Outdated Show resolved Hide resolved
@aldesilv aldesilv force-pushed the in branch 4 times, most recently from 3a82460 to c509fe3 Compare February 5, 2024 22:05
@aldesilv aldesilv force-pushed the in branch 16 times, most recently from b1422fa to 382bfc1 Compare February 9, 2024 19:41
ramiro050
ramiro050 requested changes Feb 14, 2024
View reviewed changes
Copy link
Collaborator

@ramiro050 ramiro050 left a comment

Choose a reason for hiding this comment

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

Looks good! Just a couple small changes

projects/pt1/python/torch_mlir_e2e_test/test_suite/norm_like.py Outdated Show resolved Hide resolved
lib/Conversion/TorchToLinalg/Uncategorized.cpp Outdated Show resolved Hide resolved
lib/Dialect/Torch/Transforms/DecomposeComplexOps.cpp Show resolved Hide resolved
ramiro050
ramiro050 approved these changes Feb 16, 2024
View reviewed changes
Copy link
Collaborator

@ramiro050 ramiro050 left a comment

Choose a reason for hiding this comment

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

Thanks!

@vivekkhandelwal1 vivekkhandelwal1 merged commit d29157b into llvm:main Feb 19, 2024
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@vivekkhandelwal1 vivekkhandelwal1 vivekkhandelwal1 approved these changes

@ramiro050 ramiro050 ramiro050 approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@aldesilv @vivekkhandelwal1 @ramiro050