Add support for mv. #1444
Add support for mv. #1444
Conversation
| @@ -415,6 +415,42 @@ class ConvertAtenMatmulOp : public OpConversionPattern<AtenMatmulOp> { | |||
| }; | |||
| } // namespace | |||
|
|
|||
| namespace { | |||
| class ConvertAtenMvOp : public OpConversionPattern<AtenMvOp> { | |||
There was a problem hiding this comment.
I think it would make more sense to add a decomposition of this op into AtenMatmulOp, since that op performs this same handling:
The decomposition would happen in the DecomposeComplexOps.cpp file
This is is probably due to someone adding the
Can you elaborate more on the error you were seeing? This op should be working.
Adding support for other dialects is always optional. The expectation is that people simply add what they need. (However, if you add it as a decomposition, other dialects that already support
The main file is in a different directory than the test suite, so the import of |
When I ran one of the scripts (I believe it was |
| @@ -399,7 +400,6 @@ def emit_with_mutating_variants(key, **kwargs): | |||
| emit("aten::nll_loss_backward : (Tensor, Tensor, Tensor, Tensor?, int, int, Tensor) -> (Tensor)") | |||
| emit("aten::bincount : (Tensor, Tensor?, int) -> (Tensor)") | |||
| emit("aten::linalg_vector_norm : (Tensor, Scalar, int[]?, bool, int?) -> (Tensor)") | |||
| emit("aten::frobenius_norm.dim : (Tensor, int[], bool) -> (Tensor)") | |||
There was a problem hiding this comment.
Did you remove this line before or after running ./build_tools/update_torch_ods.sh? This is the line that generates the ODS for the op. Also, when I run thing locally on main, I don't see this op disappear.
There was a problem hiding this comment.
I hadn't removed anything; I didn't have any good reason to pick on frobenius :P
Perhaps it was something temporary with us running against head pytorch? If it's working fine for you at head now I'll try clearing it all out and rerunning once I'm able to move over to the decomposition approach.
There was a problem hiding this comment.
Ah, maybe it was a temporary thing with PyTorch. There is a commit that makes changes to frobenius_norm that would appear when running update_torch_ods.sh: pytorch/pytorch@1c0f0b3
The commit has been merged into PyTorch and reverted twice, so this might come back in the future.
|
Closing this in favor of #1453 |
See #1423.
Please let me know if there are better places to put anything or conventions I missed. One thing I'm unclear on: I see in the codebase a lot of explicit
__init__s with calls to super that do nothing else. Is there a reason for this I'm missing?Also, I ran into issues with what I assume is an op that was deleted (frobenius norm). I had to do a lot of manual surgery to get things working again. Is there a better procedure for this? I can do that in a separate commit if so and rebase this off of it so this is cleaner.
Also: should I do the same for other dialects? I see a number of other dialects for matmul, but I only see
TorchToLinalgin the PR I was referencing. I figured it would be good to get feedback on this first before doing it.I tried to move the new test case to its own file, but I'm getting an import error when I add an entry to the import list in
test_suite/__init__.py:I really have no clue what it could be. The imports are the exact same as the other files in the directory and don't appear circular. The file is definitely present on disk. I also tried changing the name of the file. It's the same error I get when I put in a bogus name. Is there some magic I'm missing?