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.

Sign up for GitHub

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

Jump to bottom

[Relay, TOPI] Add numpy style cumsum op #7334

Merged
merged 27 commits into from
Jan 26, 2021
Merged

[Relay, TOPI] Add numpy style cumsum op #7334

merged 27 commits into from
Jan 26, 2021

Conversation

masahi
Copy link
Member

@masahi masahi commented Jan 25, 2021 •
edited
Loading

This adds a numpy style cumsum op to Relay/TOPI. The spec is identical with numpy one except there is no promotion of int32 -> int64 output dtype, for the case when the input is a int32 tensor and output dtype is not provided.
https://numpy.org/doc/stable/reference/generated/numpy.cumsum.html

Both CPU and GPU are supported, and it is especially efficient if thrust is available. I updated TIR scan IR to support cumsum on any rank. But scan is still done only on the innermost axis, so transposing is required when the scan axis is not the innermost one.

please review @jwfromm @mbrookhart @zhiics @kevinthesun @junrushao1994 @antinucleon

@masahi masahi marked this pull request as ready for review January 25, 2021 06:09
tkonolige
tkonolige requested changes Jan 25, 2021
View reviewed changes
python/tvm/relay/op/transform.py Show resolved Hide resolved
python/tvm/topi/cuda/scan.py Outdated Show resolved Hide resolved
python/tvm/topi/cuda/scan.py Show resolved Hide resolved
python/tvm/topi/cumsum.py Outdated Show resolved Hide resolved
src/relay/op/tensor/transform.cc Outdated Show resolved Hide resolved
src/runtime/contrib/thrust/thrust.cu Outdated Show resolved Hide resolved
tests/python/relay/test_op_level3.py Outdated Show resolved Hide resolved
@masahi masahi force-pushed the relay-cumsum branch 2 times, most recently from 0eadcc6 to 7612cfd Compare January 25, 2021 21:22
mbrookhart
mbrookhart approved these changes Jan 25, 2021
View reviewed changes
Copy link
Contributor

@mbrookhart mbrookhart left a comment

Choose a reason for hiding this comment

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

A few nitpicks, but overall LGTM

python/tvm/topi/cuda/scan.py Outdated Show resolved Hide resolved
python/tvm/topi/cuda/scan.py Outdated Show resolved Hide resolved
python/tvm/topi/cumsum.py Outdated Show resolved Hide resolved
@masahi
Copy link
Member Author

masahi commented Jan 26, 2021 •
edited
Loading

@tkonolige @mbrookhart Comments were addressed. CPU cumsum is now done in parallel over non-scan axes, and binop argument is now a function. Thanks for reviews.

tkonolige
tkonolige approved these changes Jan 26, 2021
View reviewed changes
Copy link
Contributor

@tkonolige tkonolige left a comment

Choose a reason for hiding this comment

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

Looks good! I put a couple minor nits for documentation, but otherwise this is good to go.

python/tvm/relay/op/transform.py Outdated Show resolved Hide resolved
python/tvm/topi/cuda/scan.py Show resolved Hide resolved
masahi and others added 22 commits January 27, 2021 05:15
@masahi
Add cumsum relay/topi op
66ebfdb
@masahi
relay tests working
2f7c418
@masahi
add torch frontend converter
e2d498b
@masahi
fix for importing detr
205a9a0
@masahi
fix bad merge
91a2c66
@masahi
begin cuda cumsum
49f7ce7
@masahi
support non innermost axis
eca7420
@masahi
support rank higher than 3
0e6b2c5
@masahi
making binop parameter
2c33718
@masahi
fix overflow issue in thrust scan
7cf958e
@masahi
generic binop parameter working
0b056a1
@masahi
relay test working
a8324fa
@masahi
fixed for bool input
bb6340e
@masahi
remove pytorch change
ae434e5
@masahi
fix pylint
3afe514
@masahi
doc update
55065df
@masahi @tkonolige
Update python/tvm/topi/cumsum.py
050d6aa 
Co-authored-by: Tristan Konolige <[email protected]>
@masahi @tkonolige
Update tests/python/relay/test_op_level3.py
a6b71d2 
Co-authored-by: Tristan Konolige <[email protected]>
@masahi
add example outputs
3ca20d8
@masahi
add supported input and output dtype in thrust log
0dabe12
@masahi
adding more loop var names
8ec333e
@masahi
fix cpplint
6f481fc
@masahi masahi merged commit 1e0d356 into apache:main Jan 26, 2021
@masahi
Copy link
Member Author

masahi commented Jan 26, 2021

Thanks @tkonolige @mbrookhart

@ybai62868
Copy link

Hi @masahi ,
Thanks for your contribution for the "cumsum". I want to use this op in onnx model.
I meet the problem when I want to compile an onnx model with "CumSum" op to relay.
When I use "relay.frontend.from_onnx" and add "CumSum": AttrCvt("cumsum",{"axis": axis, "dtype": dtye})," in python/tvm/relay/frontend/onnx.py

But it still does not work.
Thanks for your attention!

@ybai62868
Copy link

typo -> "CumSum": AttrCvt("cumsum",{"axis": "axis", "dtype": "dtye"}),"

@masahi
Copy link
Member Author

masahi commented Jan 28, 2021

@ybai62868 please post this to the discuss forum or open an issue, clarifying what exactly you mean by "it still does not work"

@ybai62868
Copy link

@masahi Hi， what I mean "it still does not work" is that it will report this message " tvm.error.OpNotImplemented: The following operators are not supported for frontend ONNX: CumSum " although I add and change the AttrCvt("cumsum",{"axis": axis, "dtype": dtye}) and rename the CumSum to cumsum.

@tkonolige
Copy link
Contributor

@ybai62868 This PR adds cumsum support at the topi level. The work to add support to onnx has not been done yet.

alexwong pushed a commit to alexwong/tvm that referenced this pull request Feb 11, 2021
@masahi @tkonolige @alexwong
[Relay, TOPI] Add numpy style cumsum op (apache#7334)
bc0ce8e 
* Add cumsum relay/topi op

* relay tests working

* add torch frontend converter

* fix for importing detr

* fix bad merge

* begin cuda cumsum

* support non innermost axis

* support rank higher than 3

* making binop parameter

* fix overflow issue in thrust scan

* generic binop parameter working

* relay test working

* fixed for bool input

* remove pytorch change

* fix pylint

* doc update

* Update python/tvm/topi/cumsum.py

Co-authored-by: Tristan Konolige <[email protected]>

* Update tests/python/relay/test_op_level3.py

Co-authored-by: Tristan Konolige <[email protected]>

* add example outputs

* add supported input and output dtype in thrust log

* adding more loop var names

* fix cpplint

* fix missing check for the cuda target in nms thrust sort

* parallelize cpu cumsum

* making binop argument tir function

* update doc for binop

* doc update

Co-authored-by: Tristan Konolige <[email protected]>
electriclilies pushed a commit to electriclilies/tvm that referenced this pull request Feb 18, 2021
@masahi @tkonolige @electriclilies
[Relay, TOPI] Add numpy style cumsum op (apache#7334)
f0f0af4 
* Add cumsum relay/topi op

* relay tests working

* add torch frontend converter

* fix for importing detr

* fix bad merge

* begin cuda cumsum

* support non innermost axis

* support rank higher than 3

* making binop parameter

* fix overflow issue in thrust scan

* generic binop parameter working

* relay test working

* fixed for bool input

* remove pytorch change

* fix pylint

* doc update

* Update python/tvm/topi/cumsum.py

Co-authored-by: Tristan Konolige <[email protected]>

* Update tests/python/relay/test_op_level3.py

Co-authored-by: Tristan Konolige <[email protected]>

* add example outputs

* add supported input and output dtype in thrust log

* adding more loop var names

* fix cpplint

* fix missing check for the cuda target in nms thrust sort

* parallelize cpu cumsum

* making binop argument tir function

* update doc for binop

* doc update

Co-authored-by: Tristan Konolige <[email protected]>
Lokiiiiii pushed a commit to Lokiiiiii/tvm that referenced this pull request Mar 2, 2021
@masahi @tkonolige @Lokiiiiii
[Relay, TOPI] Add numpy style cumsum op (apache#7334)
d5e9aa8 
* Add cumsum relay/topi op

* relay tests working

* add torch frontend converter

* fix for importing detr

* fix bad merge

* begin cuda cumsum

* support non innermost axis

* support rank higher than 3

* making binop parameter

* fix overflow issue in thrust scan

* generic binop parameter working

* relay test working

* fixed for bool input

* remove pytorch change

* fix pylint

* doc update

* Update python/tvm/topi/cumsum.py

Co-authored-by: Tristan Konolige <[email protected]>

* Update tests/python/relay/test_op_level3.py

Co-authored-by: Tristan Konolige <[email protected]>

* add example outputs

* add supported input and output dtype in thrust log

* adding more loop var names

* fix cpplint

* fix missing check for the cuda target in nms thrust sort

* parallelize cpu cumsum

* making binop argument tir function

* update doc for binop

* doc update

Co-authored-by: Tristan Konolige <[email protected]>
trevor-m pushed a commit to neo-ai/tvm that referenced this pull request Mar 2, 2021
@masahi @tkonolige @trevor-m
[Relay, TOPI] Add numpy style cumsum op (apache#7334)
a3d6c14 
* Add cumsum relay/topi op

* relay tests working

* add torch frontend converter

* fix for importing detr

* fix bad merge

* begin cuda cumsum

* support non innermost axis

* support rank higher than 3

* making binop parameter

* fix overflow issue in thrust scan

* generic binop parameter working

* relay test working

* fixed for bool input

* remove pytorch change

* fix pylint

* doc update

* Update python/tvm/topi/cumsum.py

Co-authored-by: Tristan Konolige <[email protected]>

* Update tests/python/relay/test_op_level3.py

Co-authored-by: Tristan Konolige <[email protected]>

* add example outputs

* add supported input and output dtype in thrust log

* adding more loop var names

* fix cpplint

* fix missing check for the cuda target in nms thrust sort

* parallelize cpu cumsum

* making binop argument tir function

* update doc for binop

* doc update

Co-authored-by: Tristan Konolige <[email protected]>
@AndrewZhaoLuo AndrewZhaoLuo mentioned this pull request Mar 23, 2021
Merged
@AndrewZhaoLuo AndrewZhaoLuo mentioned this pull request Apr 20, 2021
Merged
@junrushao junrushao mentioned this pull request Nov 1, 2021
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tkonolige tkonolige tkonolige approved these changes

@mbrookhart mbrookhart mbrookhart approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@masahi @ybai62868 @tkonolige @mbrookhart