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

Add attributes support for onnx cumsum op #3241

Merged
merged 1 commit into from
May 10, 2024
Merged

Conversation

jinchen62
Copy link
Collaborator

No description provided.

Copy link
Collaborator

@vivekkhandelwal1 vivekkhandelwal1 left a comment

Choose a reason for hiding this comment

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

I'm still not very sure about the numerical correctness of this lowering.

lib/Conversion/TorchOnnxToTorch/DefaultDomainAtoF.cpp Outdated Show resolved Hide resolved
lib/Conversion/TorchOnnxToTorch/DefaultDomainAtoF.cpp Outdated Show resolved Hide resolved
@jinchen62 jinchen62 force-pushed the o2t_cumsum branch 3 times, most recently from 52131bf to de3bfca Compare April 30, 2024 00:23
@jinchen62
Copy link
Collaborator Author

jinchen62 commented Apr 30, 2024

@vivekkhandelwal1 Yeah you were right, there was mistake. I have made the changes. To explain what I'm doing, this is the example from https://onnx.ai/onnx/operators/onnx__CumSum.html#summary

input_x = [1, 2, 3]
axis=0
output = [1, 3, 6]
exclusive=1
output = [0, 1, 3]
exclusive=0
reverse=1
output = [6, 5, 3]
exclusive=1
reverse=1
output = [5, 3, 0]

For the exclusive case, basically it means it excludes the i-th element itself for the i-th position cumsum, so we just need to sub the original matrix from the cumsum result.

For the reverse case, we would just need to flip the matrix before and after the cumsum operation.

exclusive:
cumsum -> [1, 3, 6]
sub input -> [0, 1, 3]

reverse:
flip -> [3, 2, 1]
cumsum -> [3, 5, 6]
flip -> [6, 5, 3]

exclusive + reverse:
flip -> [3, 2, 1]
cumsum -> [3, 5, 6]
flip -> [6, 5, 3]
sub input -> [5, 3, 0]

@jinchen62 jinchen62 force-pushed the o2t_cumsum branch 2 times, most recently from 52d3768 to db292ec Compare May 9, 2024 08:31
@jinchen62 jinchen62 merged commit 4b24909 into llvm:main May 10, 2024
3 checks passed
@jinchen62 jinchen62 deleted the o2t_cumsum branch May 10, 2024 18:09
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.

2 participants