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

Dynamic shape index #3039

Merged
merged 2 commits into from
Aug 30, 2024
Merged

Dynamic shape index #3039

merged 2 commits into from
Aug 30, 2024

Conversation

apbose
Copy link
Collaborator

@apbose apbose commented Jul 25, 2024

No description provided.

@apbose apbose self-assigned this Jul 25, 2024
@apbose apbose marked this pull request as draft July 25, 2024 23:21
@github-actions github-actions bot added the component: tests Issues re: Tests label Jul 25, 2024
@github-actions github-actions bot requested a review from narendasan July 25, 2024 23:21
@github-actions github-actions bot added 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 Jul 26, 2024
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:

@apbose apbose marked this pull request as ready for review July 30, 2024 02:45
@apbose apbose requested a review from peri044 July 30, 2024 02:45
@peri044
Copy link
Collaborator

peri044 commented Jul 31, 2024

@apbose is this ready ?

@apbose
Copy link
Collaborator Author

apbose commented Aug 1, 2024

@peri044 Right now the dynamic input cases are supported, I will add some test cases for dynamic index. Although the implementation should support it. Also, is there any example script to test it with mistral-7b and SD? I did not test it end to end with the models yet

@peri044
Copy link
Collaborator

peri044 commented Aug 1, 2024

@peri044 Right now the dynamic input cases are supported, I will add some test cases for dynamic index. Although the implementation should support it. Also, is there any example script to test it with mistral-7b and SD? I did not test it end to end with the models yet

For mistral model, you can use the same example https://github.com/pytorch/TensorRT/blob/llm_examples_main/examples/dynamo/torch_export_llama2.py and swap the model name to mistralai/Mistral-7B-v0.3

Comment on lines 185 to 178
(
get_shape(
ctx,
target,
source_ir,
name + f"_transpose_tensor_shape_mult_d0_{i}",
transpose_tensor,
i,
)
if transpose_tensor_shape[i] == DYNAMIC_DIM
else transpose_tensor_shape[i]
),
Copy link
Collaborator

Choose a reason for hiding this comment

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

consider storing this in a variable for better readability

Comment on lines 207 to 200
(
get_shape(
ctx,
target,
source_ir,
name + f"_transpose_tensor_shape_mult_d1_{i}",
transpose_tensor,
i,
)
if transpose_tensor_shape[i] == DYNAMIC_DIM
else transpose_tensor_shape[i]
),
Copy link
Collaborator

Choose a reason for hiding this comment

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

same as above

dim_tensor_list[adv_indx_indices[adv_indx_count - 1]],
name + "_dim_last",
)
multiplier = dim_tensor_list[adv_indx_indices[adv_indx_count - 1]]
Copy link
Collaborator

Choose a reason for hiding this comment

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

Does this change imply multiplier is always an ITensor since you were using get_trt_tensor before ?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes we do not need get_trt_tensor in this, even in the previous non dynamic cases it was not required since dim_tensor_list already stores a list of dimension tensors

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:

--- /home/runner/work/TensorRT/TensorRT/py/torch_tensorrt/_compile.py	2024-08-16 00:09:49.859558+00:00
+++ /home/runner/work/TensorRT/TensorRT/py/torch_tensorrt/_compile.py	2024-08-16 00:10:09.332451+00:00
@@ -532,6 +532,6 @@

                with enable_torchbind_tracing():
                    exp_program = torch.export.export(
                        module, tuple(arg_inputs), kwargs=kwarg_inputs, strict=False
                    )
-                    torch.export.save(exp_program, file_path)
\ No newline at end of file
+                    torch.export.save(exp_program, file_path)

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:

--- /home/runner/work/TensorRT/TensorRT/py/torch_tensorrt/_compile.py	2024-08-16 00:18:47.939073+00:00
+++ /home/runner/work/TensorRT/TensorRT/py/torch_tensorrt/_compile.py	2024-08-16 00:19:11.393152+00:00
@@ -532,6 +532,6 @@

                with enable_torchbind_tracing():
                    exp_program = torch.export.export(
                        module, tuple(arg_inputs), kwargs=kwarg_inputs, strict=False
                    )
-                    torch.export.save(exp_program, file_path)
\ No newline at end of file
+                    torch.export.save(exp_program, file_path)

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:

--- /home/runner/work/TensorRT/TensorRT/py/torch_tensorrt/_compile.py	2024-08-16 00:20:46.489048+00:00
+++ /home/runner/work/TensorRT/TensorRT/py/torch_tensorrt/_compile.py	2024-08-16 00:21:12.958728+00:00
@@ -532,6 +532,6 @@

                with enable_torchbind_tracing():
                    exp_program = torch.export.export(
                        module, tuple(arg_inputs), kwargs=kwarg_inputs, strict=False
                    )
-                    torch.export.save(exp_program, file_path)
\ No newline at end of file
+                    torch.export.save(exp_program, file_path)

@apbose
Copy link
Collaborator Author

apbose commented Aug 16, 2024

I ran the Mistral-7B-v0.3, it shows me this in the end-

INFO:torch_tensorrt.dynamo.conversion._TRTInterpreter:TRT INetwork construction elapsed time: 0:08:12.032557
INFO:torch_tensorrt [TensorRT Conversion Context]:Global timing cache in use. Profiling results in this builder pass will be stored.
ERROR:torch_tensorrt [TensorRT Conversion Context]:1: [defaultAllocator.cpp::allocate::19] Error Code 1: Cuda Runtime (out of memory)
WARNING:torch_tensorrt [TensorRT Conversion Context]:Requested amount of GPU memory (28354543872 bytes) could not be allocated. There may not be enough free memory for allocation to succeed.
double free or corruption (!prev)

Looks like it got past the dynamic index case

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:

--- /home/runner/work/TensorRT/TensorRT/py/torch_tensorrt/_compile.py	2024-08-16 00:24:39.231642+00:00
+++ /home/runner/work/TensorRT/TensorRT/py/torch_tensorrt/_compile.py	2024-08-16 00:25:02.079196+00:00
@@ -532,6 +532,6 @@

                with enable_torchbind_tracing():
                    exp_program = torch.export.export(
                        module, tuple(arg_inputs), kwargs=kwarg_inputs, strict=False
                    )
-                    torch.export.save(exp_program, file_path)
\ No newline at end of file
+                    torch.export.save(exp_program, file_path)

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:

--- /home/runner/work/TensorRT/TensorRT/py/torch_tensorrt/_compile.py	2024-08-22 23:54:07.789419+00:00
+++ /home/runner/work/TensorRT/TensorRT/py/torch_tensorrt/_compile.py	2024-08-22 23:54:27.515379+00:00
@@ -532,6 +532,6 @@

                with enable_torchbind_tracing():
                    exp_program = torch.export.export(
                        module, tuple(arg_inputs), kwargs=kwarg_inputs, strict=False
                    )
-                    torch.export.save(exp_program, file_path)
\ No newline at end of file
+                    torch.export.save(exp_program, file_path)

@peri044
Copy link
Collaborator

peri044 commented Aug 29, 2024

TestIndexDynamicConstantConverter tests are failing in CI

@peri044 peri044 merged commit 0f8f23d into main Aug 30, 2024
66 of 67 checks passed
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.

3 participants