export: add explicit decomposition for aten.expand_copy and unit test #161688
Conversation
🔗 Helpful Links🧪 See artifacts and rendered test results at hud.pytorch.org/pr/161688
Note: Links to docs will display an error until the docs builds have been completed. ⏳ No Failures, 6 PendingAs of commit 47021d7 with merge base d636c18 ( This comment was automatically generated by Dr. CI and updates every 15 minutes. |
|
hi sorry I am still not very familiar with the pr process. I will fix the other issues when I wake up tomorrow. Thanks. I am not sure if the changes in 30d971e are needed but I will just leave them there for now. |
…to 161080 sync fork
|
@pytorchbot label "topic: not user facing" |
torch/_decomp/decompositions.py
Outdated
|
|
||
| @register_decomposition(aten.expand_copy) | ||
| @out_wrapper() | ||
| def expand_copy(self, size, *, implicit: bool = False): |
There was a problem hiding this comment.
expand also takes in implicit as a flag, so I think we should just add the flag to its decomposition
There was a problem hiding this comment.
hi I have spent several hours on this trying to trace down the root cause and attempting various fixes. However, it seems that the issue lies within python function binding. The schema is working fine as modifying it does not change anything and it was propagated properly throughout all the modules I have traced through. It was very interesting as it seems that the schema is effective in identifying unrecognized argument, but if the implicit keyword is used, the process would go further and break because it just randomly stops following the schema.
The latest commit I have submitted just straight up dereference the implicit argument since it's not used by aten anyway.
I hope this helps, sorry for not being able to do more.
There was a problem hiding this comment.
I think all you need to do is just add the implicit flag here and set a default value for it
pytorch/torch/_refs/__init__.py
Lines 3016 to 3019 in 4acdbb8
Both expand and expand_copy accept implicit as an input, and since expand makes a copy view from expand_copy, we can fix it in the same place.
If there is another issue, you can paste the output here or start a new issue in the community :)
There was a problem hiding this comment.
thank you so much @can-gaa-hou! This fixes the problem completely and I will make sure to learn from this experience.
@angelayi @can-gaa-hou I think the pr can be merged now to close #161688.
albertw7711
changed the title
…to 161080 sync with remote fork
|
@angelayi hi I really hope my first pr here can be accepted I am planning on making future contributions as well so if the code is still not up to standard please let me know much appreciated! Please approve the review otherwise. |
|
also it should be able pass all tests now and ready to merge :) |
|
thanks! |
|
@pytorchbot merge |
Merge startedYour change will be merged once all checks pass (ETA 0-4 Hours). Learn more about merging in the wiki. Questions? Feedback? Please reach out to the PyTorch DevX Team |
|
Thanks everyone for being supportive! I will start working on a new issue. |
…pytorch#161688) Fixes pytorch#161080 torch.export.export fails with TypeError: expand() got an unexpected keyword argument 'implicit' when calling torch.expand_copy(..., implicit=True). This happened because expand_copy = _make_copy_from_view(aten.expand) register aten. expand as the decomposition path for aten.expand_copy, which doesn’t accept the implicit argument. I have added an explicit a decomposition for aten.expand_copy in torch/_decomp/decompositions.py to ignore the implicit argument, and a simple unit test to demonstrate the bug being fixed. Pull Request resolved: pytorch#161688 Approved by: https://github.com/angelayi, https://github.com/can-gaa-hou
…pytorch#161688) Fixes pytorch#161080 torch.export.export fails with TypeError: expand() got an unexpected keyword argument 'implicit' when calling torch.expand_copy(..., implicit=True). This happened because expand_copy = _make_copy_from_view(aten.expand) register aten. expand as the decomposition path for aten.expand_copy, which doesn’t accept the implicit argument. I have added an explicit a decomposition for aten.expand_copy in torch/_decomp/decompositions.py to ignore the implicit argument, and a simple unit test to demonstrate the bug being fixed. Pull Request resolved: pytorch#161688 Approved by: https://github.com/angelayi, https://github.com/can-gaa-hou
…pytorch#161688) Fixes pytorch#161080 torch.export.export fails with TypeError: expand() got an unexpected keyword argument 'implicit' when calling torch.expand_copy(..., implicit=True). This happened because expand_copy = _make_copy_from_view(aten.expand) register aten. expand as the decomposition path for aten.expand_copy, which doesn’t accept the implicit argument. I have added an explicit a decomposition for aten.expand_copy in torch/_decomp/decompositions.py to ignore the implicit argument, and a simple unit test to demonstrate the bug being fixed. Pull Request resolved: pytorch#161688 Approved by: https://github.com/angelayi, https://github.com/can-gaa-hou
…pytorch#161688) Fixes pytorch#161080 torch.export.export fails with TypeError: expand() got an unexpected keyword argument 'implicit' when calling torch.expand_copy(..., implicit=True). This happened because expand_copy = _make_copy_from_view(aten.expand) register aten. expand as the decomposition path for aten.expand_copy, which doesn’t accept the implicit argument. I have added an explicit a decomposition for aten.expand_copy in torch/_decomp/decompositions.py to ignore the implicit argument, and a simple unit test to demonstrate the bug being fixed. Pull Request resolved: pytorch#161688 Approved by: https://github.com/angelayi, https://github.com/can-gaa-hou
5 participants
Fixes #161080
torch.export.export fails with TypeError: expand() got an unexpected keyword argument 'implicit' when calling torch.expand_copy(..., implicit=True). This happened because expand_copy = _make_copy_from_view(aten.expand) register aten. expand as the decomposition path for aten.expand_copy, which doesn’t accept the implicit argument.
I have added an explicit a decomposition for aten.expand_copy in torch/_decomp/decompositions.py to ignore the implicit argument, and a simple unit test to demonstrate the bug being fixed.