Skip to content

small fix: Index validator enable int64 #2642

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Merged
merged 2 commits into from
Feb 6, 2024
Merged

Conversation

gs-olive
Copy link
Collaborator

@gs-olive gs-olive commented Feb 5, 2024

Description

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 gs-olive requested a review from narendasan February 5, 2024 23:41
@gs-olive gs-olive self-assigned this Feb 5, 2024
@github-actions github-actions bot added component: conversion Issues re: Conversion stage component: api [Python] Issues re: Python API component: dynamo Issues relating to the `torch.compile` or `torch._dynamo.export` paths labels Feb 5, 2024
@github-actions github-actions bot requested a review from apbose February 5, 2024 23:41
@gs-olive gs-olive force-pushed the index_conv_validator_fix branch from 1b13159 to 5501ace Compare February 5, 2024 23:42
@github-actions github-actions bot added the component: tests Issues re: Tests label Feb 5, 2024
@@ -43,7 +43,7 @@ def forward(self, x: torch.Tensor, y: torch.Tensor):
# For the default settings, we can simply call torch.compile
# with the backend "torch_tensorrt", and run the model on an
# input to cause compilation, as so:
optimized_model = torch.compile(model, backend="torch_tensorrt")
optimized_model = torch.compile(model, backend="torch_tensorrt", dynamic=False)
Copy link
Collaborator

Choose a reason for hiding this comment

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

any reason why you explicitly set dynamic=False instead of None here ?

@@ -61,6 +61,7 @@
optimized_model = torch.compile(
model,
backend="torch_tensorrt",
dynamic=False,
Copy link
Collaborator

Choose a reason for hiding this comment

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

^ same question here

@gs-olive
Copy link
Collaborator Author

gs-olive commented Feb 6, 2024

@peri044 - when dynamic=None, as is currently the (Default) case, torch.compile will insert SymInts upon detection of a new batch dimension, which we cannot handle currently, causing errors in the compiler. We currently specify it manually in the torch_tensorrt.compile convenience frontend, as here:

# TODO: Remove dynamic=False when SymInt Dynamic shape support is ready
boxed_fn = torch.compile(
module, backend=torch_tensorrt_backend, dynamic=False, options={**kwargs}
)

When users call torch.compile directly, however, they need to specify dynamic=False to avoid errors.

@gs-olive gs-olive merged commit ffbcc7a into main Feb 6, 2024
@gs-olive gs-olive deleted the index_conv_validator_fix branch February 6, 2024 17:29
gs-olive added a commit that referenced this pull request Feb 6, 2024
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
cla signed component: api [Python] Issues re: Python API component: conversion Issues re: Conversion stage component: dynamo Issues relating to the `torch.compile` or `torch._dynamo.export` paths component: tests Issues re: Tests priority: high
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants