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.

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

feat: support native_dropout dynamo converter #2931

Merged
merged 3 commits into from
Jun 19, 2024

Conversation

zewenli98
Copy link
Collaborator

@zewenli98 zewenli98 commented Jun 17, 2024

Description

Support native_dropout dynamo converter.

Per the schema, this function returns two tensors. The first is output and the other is mask. For inference, i.e., train is not True or None, whatever the p is, it always returns input and all Trues.

Fixes #2494

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

@zewenli98 zewenli98 self-assigned this Jun 17, 2024
@zewenli98 zewenli98 requested a review from chohk88 June 17, 2024 21:22
@github-actions github-actions bot added component: tests Issues re: Tests component: conversion Issues re: Conversion stage component: converters Issues re: Specific op converters component: api [Python] Issues re: Python API component: dynamo Issues relating to the `torch.compile` or `torch._dynamo.export` paths labels Jun 17, 2024
@zewenli98 zewenli98 requested a review from narendasan June 17, 2024 21:22
@github-actions github-actions bot requested a review from peri044 June 17, 2024 21:22
def forward(self):
return torch.ops.aten.native_dropout.default(input, p, train)

inputs = []
Copy link
Collaborator

Choose a reason for hiding this comment

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

Could you please explain the reason for dividing the test cases into inputs = [torch.randn(input_shape)] and inputs = []?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

If passing in inputs = [torch.randn(input_shape)], the input will be an ITensor. If passing the input directly into the pytorch function, the input will be a normal tensor.

Copy link
Collaborator

@chohk88 chohk88 Jun 19, 2024

Choose a reason for hiding this comment

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

LGTM. The code looks good to merge.

I have one more question about input types for my understanding. Usually, we handle inputs as input: TRTTensor and only consider ITensor input. However, sometimes we use input_val: Union[TRTTensor, torch.Tensor, np.ndarray] or input: Sequence[Union[TRTTensor, torch.Tensor, np.ndarray]] to handle both ITensor and normal tensor types. Even though the schema doesn't show a difference, how do we distinguish between them?

Copy link
Collaborator Author

@zewenli98 zewenli98 Jun 19, 2024

Choose a reason for hiding this comment

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

Yeah very good questions.

  1. Even though the schema doesn't show a difference, how do we distinguish between them?

From my understanding, ITensor is a concept in TRT so PyTorch schema doesn't distinguish them from the design perspective. (TRT is just one backend among many backends)

  1. Usually, we handle inputs as input: TRTTensor and only consider ITensor input. However, sometimes we use input_val: Union[TRTTensor, torch.Tensor, np.ndarray] or input: Sequence[Union[TRTTensor, torch.Tensor, np.ndarray]] to handle both ITensor and normal tensor types.

I think in the entry point the inputs are not promised to be ITensor. Instead, they could be torch.Tensor or np.ndarray. So there are two ways to handle it. 1) use enforce_tensor_types to make it as ITensor 2) manually cast it to ITensor inside of the function. I think in most cases both are fine, but if this function is called by another function that passes in non-ITensor, the first solution will error out.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I understand better now. Thank you so much for the explanation!

@zewenli98 zewenli98 merged commit cbefcbb into main Jun 19, 2024
42 of 47 checks passed
@zewenli98 zewenli98 deleted the native_dropout_dynamo_converter branch June 19, 2024 00:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
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: tests Issues re: Tests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

aten.native_dropout
3 participants