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

Fix torch listconstruct errors when dependent on inputs flexible shapes #2050

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

xorange
Copy link

@xorange xorange commented Nov 9, 2023

Recommodation for merge:

No.

There're some unit tests still needs to be added.

What it fixes

This PR is trying to fix #1921, along with other bugs that suffers the same reason.

The root cause is discussed in #1926, that when converting a torch.listconstruct, if any value in it depends on a dynamic shape which roots in the net input, the converted result is a bare python list and failed to parse in the subsequent op.

Design of this fix

This fix happends inside listconstruct parsing, runs a DFS from the current node to each parent layer all the way to the net root. And works on this asssumption:

If any value depends on a dynamic shape which roots in the net input, then there is an op gather, that gathers that dynamic shape from net input dims

What it does and does not

  1. This fix does not affect the original branch for all compile-time scalar constants case
  2. This fix still holds the original branch for all unexpected case
  3. This fix addes another branch, only targets for an op gather, and what it gathers from is not a name in context.torch_graph.nodes, i.e. the net inputs.

@xorange
Copy link
Author

xorange commented Nov 9, 2023

memoization for DFS must be added to accelerate, otherwise it's way too slow for relatively larger networks.

Edit:
Improvements mentioned above is added.

@YifanShenSZ
Copy link
Collaborator

YifanShenSZ commented Nov 13, 2023

Hi @xorange, I left a comment in #1926. Would you prefer to go for general fix in this PR, or specific fix for your pad?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Conversion for Padding related to dynamic input shape failed
2 participants