Skip to content

fix: Add automatic type promotion for FX elementwise ops #2055

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.

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

Closed
wants to merge 1 commit into from

Conversation

gs-olive
Copy link
Collaborator

@gs-olive gs-olive commented Jun 23, 2023

Description

  • Implement functionality to cast tensors to alternative types
  • Add functionality to elementwise ops to promote types and perform necessary casts
  • Address issues in FX ops where mixed-precision computations can cause errors
  • Add test cases to validate fix

Fixes #1995

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

Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

Code conforms to C++ style guidelines

Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

Code conforms to Python style guidelines

Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

Code conforms to Python style guidelines

Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

Code conforms to C++ style guidelines

Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

Code conforms to Python style guidelines

Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

Code conforms to C++ style guidelines

@apbose apbose force-pushed the converter_reorg_elementwise branch from 546f975 to c8a9559 Compare June 26, 2023 16:11
@gs-olive gs-olive force-pushed the fx_type_promotion branch from 9cc2d9d to 24f4781 Compare June 26, 2023 16:29
Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

Code conforms to C++ style guidelines

Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

Code conforms to Python style guidelines

@gs-olive gs-olive force-pushed the fx_type_promotion branch from 24f4781 to a5b3fae Compare June 26, 2023 19:15
Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

Code conforms to C++ style guidelines

Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

Code conforms to Python style guidelines

@apbose apbose force-pushed the converter_reorg_elementwise branch 2 times, most recently from 18e503b to 2caac76 Compare July 6, 2023 21:39
@gs-olive gs-olive force-pushed the fx_type_promotion branch from a5b3fae to d15962e Compare July 6, 2023 22:40
Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

Code conforms to C++ style guidelines

Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

There are some changes that do not conform to Python style guidelines:

--- py/torch_tensorrt/fx/converters/impl/elementwise/base.py	2023-07-06 22:40:20.742725 +0000
+++ py/torch_tensorrt/fx/converters/impl/elementwise/base.py	2023-07-06 22:40:40.009356 +0000
@@ -127,11 +127,12 @@

    lhs_val = get_trt_tensor(network, lhs_val, f"{name}_lhs", lhs_dtype)
    rhs_val = get_trt_tensor(network, rhs_val, f"{name}_rhs", rhs_dtype)

    promoted_type = torch.promote_types(
-        unified_dtype_converter(lhs_val.dtype, Frameworks.TORCH), unified_dtype_converter(rhs_val.dtype, Frameworks.TORCH)
+        unified_dtype_converter(lhs_val.dtype, Frameworks.TORCH),
+        unified_dtype_converter(rhs_val.dtype, Frameworks.TORCH),
    )
    trt_promoted_type = unified_dtype_converter(promoted_type, Frameworks.TRT)

    if trt_promoted_type != lhs_val.dtype:
        lhs_val = cast_trt_tensor(network, lhs_val, trt_promoted_type, name)

- Implement functionality to cast tensors to alternative types
- Add functionality to elementwise ops to promote types and perform
necessary casts
- Address issues in FX ops where mixed-precision computations can
cause errors
- Add test cases to validate fix
@gs-olive gs-olive force-pushed the fx_type_promotion branch from d15962e to a720f5d Compare July 6, 2023 22:55
Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

Code conforms to Python style guidelines

Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

Code conforms to C++ style guidelines

@narendasan
Copy link
Collaborator

Closed as changes were merged into Dynamo #2070

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

🐛 [Bug] Encountered bug when compiling fasterrcnn_mobilenet_v3_large_320_fpn in Dynamo
3 participants