Skip to content

Conversation

inocsin
Copy link
Contributor

@inocsin inocsin commented Feb 5, 2021

Description

There is a case

code
  x1 = torch.to(torch.ge(x0, 0.5), torch.device("cuda:0"), 3, False, False)
  _136 = torch.sum(torch.mul(x1, CONSTANTS.c0), [1], False)

graph
  %1284 : Tensor = prim::Constant[value=<Tensor>]()
  %1275 : Tensor = aten::ge(%x0.1, %1274)
  %x1.1 : Tensor = aten::to(%1275, %1315, %1278, %484, %484, %486)
  %1285 : Tensor = aten::mul(%x1.1, %1284)

The output of aten::ge is Bool type, and there are three way to support the above graph

  1. implement aten::to, to convert datatype from Bool to Int
  2. convert datatype in aten::mul (see Add support for bool in elementwise ops  #321), but I found the trtorch framework doesn't support specifying input data type, so I cann't test this method
  3. convert the output type of Boolean Operation (that's what this PR does)

Type of change

  • New feature (non-breaking change which adds functionality)
    Convert ouput type from bool to int

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

Signed-off-by: inocsin <vcheungyi@163.com>
@github-actions github-actions bot added component: conversion Issues re: Conversion stage component: converters Issues re: Specific op converters component: core Issues re: The core compiler labels Feb 5, 2021
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

@narendasan
Copy link
Collaborator

Can you write test cases for this?

@peri044 peri044 self-requested a review February 8, 2021 23:27
@inocsin
Copy link
Contributor Author

inocsin commented Feb 10, 2021

Can you write test cases for this?

I think we already have test cases for boolean operation

@developer0hye
Copy link
Contributor

developer0hye commented Feb 18, 2021

Does this code only works when we set precision to INT?

Error happens.
image

@developer0hye
Copy link
Contributor

developer0hye commented Feb 19, 2021

Please check my pull #351

@narendasan narendasan added the on hold PR is on hold pending another PR or feature being implemented label Mar 19, 2021
@peri044
Copy link
Collaborator

peri044 commented Aug 24, 2021

We now support aten::to operator which handles such masked multiplications. Closing this PR

@peri044 peri044 closed this Aug 24, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component: conversion Issues re: Conversion stage component: converters Issues re: Specific op converters component: core Issues re: The core compiler on hold PR is on hold pending another PR or feature being implemented
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants