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

Added kwarg support for dynamo.compile #2970

Merged
merged 23 commits into from
Aug 6, 2024
Merged

Added kwarg support for dynamo.compile #2970

merged 23 commits into from
Aug 6, 2024

Conversation

cehongwang
Copy link
Collaborator

Description

Please review my changes for adding kwarg support for dynamo.compile

@github-actions github-actions bot added component: tests Issues re: Tests component: api [Python] Issues re: Python API component: dynamo Issues relating to the `torch.compile` or `torch._dynamo.export` paths labels Jun 28, 2024
@github-actions github-actions bot requested a review from gs-olive June 28, 2024 23:25
narendasan
narendasan reviewed Jul 1, 2024
View reviewed changes
py/torch_tensorrt/dynamo/utils.py Show resolved Hide resolved
py/torch_tensorrt/dynamo/_compiler.py Outdated Show resolved Hide resolved
py/torch_tensorrt/dynamo/_compiler.py Outdated Show resolved Hide resolved
@github-actions github-actions bot added component: lowering Issues re: The lowering / preprocessing passes documentation Improvements or additions to documentation labels Jul 1, 2024
@github-actions github-actions bot added component: conversion Issues re: Conversion stage component: core Issues re: The core compiler component: converters Issues re: Specific op converters component: build system Issues re: Build system component: runtime component: torch_compile labels Jul 8, 2024
@narendasan
Copy link
Collaborator

There are other APIs like torch_tensorrt.save that use a similar input arg. we should update those to match

@github-actions github-actions bot removed component: converters Issues re: Specific op converters component: build system Issues re: Build system labels Jul 12, 2024
narendasan
narendasan reviewed Jul 30, 2024
View reviewed changes
Copy link
Collaborator

@narendasan narendasan left a comment

Choose a reason for hiding this comment

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

Seems fine to me, @peri044, @zewenli98 can you take a look?

peri044
peri044 reviewed Jul 30, 2024
View reviewed changes
py/torch_tensorrt/_compile.py Outdated Show resolved Hide resolved
py/torch_tensorrt/_compile.py Outdated Show resolved Hide resolved
py/torch_tensorrt/_compile.py Outdated Show resolved Hide resolved
py/torch_tensorrt/dynamo/_compiler.py Outdated Show resolved Hide resolved
py/torch_tensorrt/dynamo/_compiler.py Outdated Show resolved Hide resolved
py/torch_tensorrt/dynamo/_tracer.py Outdated Show resolved Hide resolved
py/torch_tensorrt/dynamo/_tracer.py Outdated Show resolved Hide resolved
py/torch_tensorrt/dynamo/conversion/_conversion.py Show resolved Hide resolved
tests/py/dynamo/models/test_export_serde.py Outdated Show resolved Hide resolved
zewenli98
zewenli98 reviewed Jul 30, 2024
View reviewed changes
py/torch_tensorrt/dynamo/_compiler.py Outdated Show resolved Hide resolved
py/torch_tensorrt/dynamo/_compiler.py Show resolved Hide resolved
py/torch_tensorrt/dynamo/_compiler.py Outdated Show resolved Hide resolved
py/torch_tensorrt/dynamo/_refit.py Show resolved Hide resolved
py/torch_tensorrt/dynamo/_exporter.py Outdated Show resolved Hide resolved
@cehongwang
Copy link
Collaborator Author

@narendasan @peri044 @zewenli98 If there is no further changes required, should I merge this now?

@cehongwang cehongwang force-pushed the kwarg-compile branch 5 times, most recently from 3bbeea3 to 4fc777e Compare July 31, 2024 04:42
narendasan
narendasan reviewed Jul 31, 2024
View reviewed changes
Copy link
Collaborator

@narendasan narendasan left a comment

Choose a reason for hiding this comment

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

Make sure that the torch.export kwargs ordering is matched by our flattening or use their flattening

narendasan
narendasan approved these changes Aug 6, 2024
View reviewed changes
Copy link
Collaborator

@narendasan narendasan left a comment

Choose a reason for hiding this comment

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

LGTM

@cehongwang cehongwang merged commit 3a0b244 into main Aug 6, 2024
39 of 61 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@peri044 peri044 peri044 left review comments

@zewenli98 zewenli98 zewenli98 left review comments

@narendasan narendasan narendasan approved these changes

@gs-olive gs-olive Awaiting requested review from gs-olive

Assignees
No one assigned
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: lowering Issues re: The lowering / preprocessing passes component: tests Issues re: Tests component: torch_compile documentation Improvements or additions to documentation
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@cehongwang @narendasan @peri044 @zewenli98 @facebook-github-bot