feat(//core/conversion): Add support for aten::size with dynamic shaped models for Torchscript backend. by peri044 · Pull Request #1647 · pytorch/TensorRT

peri044 · 2023-02-03T00:28:03Z

Description

#1577

Fixes issues listed in the above discussion

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

Signed-off-by: Dheeraj Peri <peri.dheeraj@gmail.com>

peri044 · 2023-02-03T00:28:25Z

cc: @mfeliz-cruise

github-actions

Code conforms to Python style guidelines

mfeliz-cruise · 2023-02-03T00:37:30Z

core/conversion/evaluators/aten.cpp

+             [](ConversionCtx* ctx, const torch::jit::Node* n, kwargs& args) -> c10::optional<torch::jit::IValue> {
+               auto list_input = args.at(n->input(0));
               auto idx = args.at(n->input(1)).unwrapToInt();
+               if (list_input.isIValue()){


It looks like this implements one layer of look back, are there cases where this would fail in a new way or do those fallback to list_input.isIValue() to preserve the existing functionality?

By look back, I'm imagining a case where we have something like list [idx1] [idx2]. I don't have an answer here (how both ITensor and IValue interplay) as it depends on how it is represented in the torchscript graph

Worth adding a test?

core/conversion/converters/impl/shuffle.cpp

mfeliz-cruise · 2023-02-03T01:07:56Z

This looks great @peri044. I left some questions about edge cases, but generally I fully support this change to add the ConversionCtx to the evaluator inputs. This opens up the possibility for us to implement alternative conversion paths for dynamic shapes for any evaluator. Thank you!

core/conversion/converters/BUILD

core/conversion/evaluators/aten.cpp

core/conversion/converters/impl/shuffle.cpp

Signed-off-by: Dheeraj Peri <peri.dheeraj@gmail.com>

github-actions

Code conforms to C++ style guidelines

github-actions

Code conforms to Python style guidelines

Signed-off-by: Dheeraj Peri <peri.dheeraj@gmail.com>

github-actions

Code conforms to C++ style guidelines

github-actions

Code conforms to Python style guidelines

Signed-off-by: Dheeraj Peri <peri.dheeraj@gmail.com>

github-actions

Code conforms to C++ style guidelines

github-actions

Code conforms to Python style guidelines

Signed-off-by: Dheeraj Peri <peri.dheeraj@gmail.com>

github-actions

Code conforms to C++ style guidelines

github-actions

Code conforms to Python style guidelines

github-actions

Code conforms to C++ style guidelines

github-actions

Code conforms to Python style guidelines

core/conversion/var/Var.cpp

Signed-off-by: Dheeraj Peri <peri.dheeraj@gmail.com>

github-actions

Code conforms to C++ style guidelines

github-actions

Code conforms to Python style guidelines

github-actions

Code conforms to C++ style guidelines

github-actions

Code conforms to Python style guidelines

github-actions

Code conforms to Python style guidelines

github-actions

Code conforms to C++ style guidelines

github-actions

Code conforms to C++ style guidelines

github-actions

Code conforms to Python style guidelines

Signed-off-by: Dheeraj Peri <peri.dheeraj@gmail.com>

github-actions

Code conforms to C++ style guidelines

github-actions

Code conforms to Python style guidelines

feat(//core/conversion): Add support for aten::size with dynamic shaped models for Torchscript backend.

peri044 added 5 commits September 29, 2022 11:34

fix: Implement duality support for evaluators

ccad996

Signed-off-by: Dheeraj Peri <peri.dheeraj@gmail.com>

chore: play around with aten::size

04ded55

Signed-off-by: Dheeraj Peri <peri.dheeraj@gmail.com>

chore: resolve conflicts

0e42d97

Signed-off-by: Dheeraj Peri <peri.dheeraj@gmail.com>

feat: Implement dynamic version of aten::size

4c17994

Signed-off-by: Dheeraj Peri <peri.dheeraj@gmail.com>

chore: refactor code

b2c8f59

Signed-off-by: Dheeraj Peri <peri.dheeraj@gmail.com>

peri044 added the WIP Work is in progress, pull request should not be merged yet label Feb 3, 2023

facebook-github-bot added the cla signed label Feb 3, 2023

github-actions bot added component: api [Python] Issues re: Python API component: conversion Issues re: Conversion stage component: converters Issues re: Specific op converters component: core Issues re: The core compiler component: evaluators Issues re: Specific op evaluators labels Feb 3, 2023

github-actions bot requested a review from bowang007 February 3, 2023 00:28

This comment was marked as resolved.

Sign in to view

github-actions bot approved these changes Feb 3, 2023

View reviewed changes

mfeliz-cruise reviewed Feb 3, 2023

View reviewed changes

core/conversion/converters/impl/shuffle.cpp Outdated Show resolved Hide resolved

mfeliz-cruise reviewed Feb 3, 2023

View reviewed changes

core/conversion/converters/BUILD Outdated Show resolved Hide resolved

andi4191 reviewed Feb 3, 2023

View reviewed changes

core/conversion/evaluators/aten.cpp Outdated Show resolved Hide resolved

core/conversion/converters/impl/shuffle.cpp Outdated Show resolved Hide resolved

chore: linter fixes

47ae984

Signed-off-by: Dheeraj Peri <peri.dheeraj@gmail.com>

github-actions bot approved these changes Feb 3, 2023

View reviewed changes

chore: Add testcase

6754c79

Signed-off-by: Dheeraj Peri <peri.dheeraj@gmail.com>

github-actions bot added the component: tests Issues re: Tests label Feb 4, 2023

github-actions bot approved these changes Feb 4, 2023

View reviewed changes

chore: Add ivalue type detections

6c69b41

Signed-off-by: Dheeraj Peri <peri.dheeraj@gmail.com>

github-actions bot approved these changes Feb 9, 2023

View reviewed changes

chore: Refactor utilities and support new Var utils and testcase

10aaaf4

Signed-off-by: Dheeraj Peri <peri.dheeraj@gmail.com>

github-actions bot approved these changes Feb 9, 2023

View reviewed changes

peri044 requested a review from narendasan February 9, 2023 08:39

github-actions bot approved these changes Feb 9, 2023

View reviewed changes

narendasan reviewed Feb 13, 2023

View reviewed changes

core/conversion/var/Var.cpp Outdated Show resolved Hide resolved

peri044 added 2 commits February 13, 2023 15:57

Merge branch 'main' into aten_size_fix

1d9200d

refactor: Implement a macro for pytorch type checking

34e7b09

Signed-off-by: Dheeraj Peri <peri.dheeraj@gmail.com>

github-actions bot approved these changes Feb 14, 2023

View reviewed changes

Merge branch 'main' into aten_size_fix

adc29b0

github-actions bot approved these changes Feb 28, 2023

View reviewed changes

Merge branch 'main' into aten_size_fix

e499ed1

github-actions bot approved these changes Mar 7, 2023

View reviewed changes

andi4191 mentioned this pull request Apr 5, 2023

feat: Added support for aten::unflatten converter #1808

Closed

7 tasks

Merge branch 'main' into aten_size_fix

8fc0730

github-actions bot approved these changes Apr 5, 2023

View reviewed changes

fix: Fix how ITensorList is detected

76dc804

Signed-off-by: Dheeraj Peri <peri.dheeraj@gmail.com>

github-actions bot approved these changes Apr 5, 2023

View reviewed changes

peri044 merged commit 745af55 into main Apr 6, 2023

bowang007 pushed a commit that referenced this pull request Apr 28, 2023

Merge pull request #1647 from pytorch/aten_size_fix

d5edd7c

feat(//core/conversion): Add support for aten::size with dynamic shaped models for Torchscript backend.

Conversation

peri044 commented Feb 3, 2023

Description

Type of change

Checklist:

Uh oh!

peri044 commented Feb 3, 2023

Uh oh!

This comment was marked as resolved.

Uh oh!

github-actions bot left a comment

Choose a reason for hiding this comment

Uh oh!

mfeliz-cruise Feb 3, 2023

Choose a reason for hiding this comment

Uh oh!

peri044 Feb 4, 2023

Choose a reason for hiding this comment

Uh oh!

mfeliz-cruise Feb 4, 2023

Choose a reason for hiding this comment

Uh oh!

Uh oh!

mfeliz-cruise commented Feb 3, 2023

Uh oh!

Uh oh!

Uh oh!

Uh oh!

github-actions bot left a comment

Choose a reason for hiding this comment

Uh oh!

github-actions bot left a comment

Choose a reason for hiding this comment

Uh oh!

github-actions bot left a comment

Choose a reason for hiding this comment

Uh oh!

github-actions bot left a comment

Choose a reason for hiding this comment

Uh oh!

github-actions bot left a comment

Choose a reason for hiding this comment

Uh oh!

github-actions bot left a comment

Choose a reason for hiding this comment

Uh oh!

github-actions bot left a comment

Choose a reason for hiding this comment

Uh oh!

github-actions bot left a comment

Choose a reason for hiding this comment

Uh oh!

github-actions bot left a comment

Choose a reason for hiding this comment

Uh oh!

github-actions bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

github-actions bot left a comment

Choose a reason for hiding this comment

Uh oh!

github-actions bot left a comment

Choose a reason for hiding this comment

Uh oh!

github-actions bot left a comment

Choose a reason for hiding this comment

Uh oh!

github-actions bot left a comment

Choose a reason for hiding this comment

Uh oh!

github-actions bot left a comment

Choose a reason for hiding this comment

Uh oh!

github-actions bot left a comment

Choose a reason for hiding this comment

Uh oh!

github-actions bot left a comment

Choose a reason for hiding this comment

Uh oh!