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

[TOPI] Sparse Add Op added #7435

Merged
merged 9 commits into from
Mar 11, 2021
Merged

[TOPI] Sparse Add Op added #7435

merged 9 commits into from
Mar 11, 2021

Conversation

ANSHUMAN87
Copy link
Contributor

1: Sparse Add Op added in Topi.
2: TF Frontend support added.

@ANSHUMAN87 ANSHUMAN87 force-pushed the sparse-37 branch 5 times, most recently from d7790fa to 7ee8692 Compare February 17, 2021 07:11
@ANSHUMAN87 ANSHUMAN87 marked this pull request as ready for review February 17, 2021 16:39
@ANSHUMAN87
Copy link
Contributor Author

cc @tkonolige , @FrozenGene!

tkonolige
tkonolige requested changes Feb 18, 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 pretty good @ANSHUMAN87. I appreciate the docs. I've left a couple of improvements.

python/tvm/relay/frontend/tensorflow.py Outdated Show resolved Hide resolved
python/tvm/topi/nn/sparse.py Outdated Show resolved Hide resolved
python/tvm/topi/nn/sparse.py Outdated Show resolved Hide resolved
src/relay/op/nn/sparse.cc Outdated Show resolved Hide resolved
src/relay/op/nn/sparse.cc Outdated Show resolved Hide resolved
src/relay/op/nn/sparse.cc Outdated Show resolved Hide resolved
src/relay/op/nn/sparse.cc Outdated Show resolved Hide resolved
src/relay/op/nn/sparse.cc Show resolved Hide resolved
python/tvm/relay/op/strategy/generic.py Outdated Show resolved Hide resolved
python/tvm/relay/op/nn/_nn.py Outdated Show resolved Hide resolved
tkonolige
tkonolige requested changes Feb 19, 2021
View reviewed changes
python/tvm/relay/op/nn/nn.py Outdated Show resolved Hide resolved
python/tvm/relay/op/nn/nn.py Outdated Show resolved Hide resolved
src/relay/op/nn/sparse.cc Outdated Show resolved Hide resolved
@ANSHUMAN87
Copy link
Contributor Author

@tkonolige : All your comments are addressed now, PTAL. Thanks!

tkonolige
tkonolige requested changes Feb 25, 2021
View reviewed changes
python/tvm/relay/op/nn/nn.py Outdated Show resolved Hide resolved
python/tvm/relay/op/strategy/generic.py Outdated Show resolved Hide resolved
python/tvm/topi/generic/nn.py Outdated Show resolved Hide resolved
python/tvm/topi/nn/sparse.py Outdated Show resolved Hide resolved
src/relay/op/nn/sparse.cc Outdated Show resolved Hide resolved
python/tvm/topi/nn/sparse.py Outdated Show resolved Hide resolved
python/tvm/topi/nn/sparse.py Outdated Show resolved Hide resolved
tests/python/frontend/tensorflow/test_forward.py Outdated Show resolved Hide resolved
tests/python/topi/python/test_topi_sparse.py Show resolved Hide resolved
python/tvm/relay/op/strategy/generic.py Show resolved Hide resolved
@ANSHUMAN87
Copy link
Contributor Author

cc @FrozenGene , @mbaret !

@ANSHUMAN87
Copy link
Contributor Author

Gentle ping @FrozenGene , @mbaret !

mbaret
mbaret reviewed Mar 8, 2021
View reviewed changes
Copy link
Contributor

@mbaret mbaret left a comment

Choose a reason for hiding this comment

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

Broadly lgtm. I'm not especially familiar with sparse implementations. Perhaps @giuseros you could also take a quick look at the TOPI stuff?

python/tvm/relay/op/nn/nn.py Show resolved Hide resolved
@ANSHUMAN87
Copy link
Contributor Author

Broadly lgtm. I'm not especially familiar with sparse implementations. Perhaps @giuseros you could also take a quick look at the TOPI stuff?

Gentle ping @giuseros !

mbaret
mbaret approved these changes Mar 11, 2021
View reviewed changes
Copy link
Contributor

@mbaret mbaret left a comment

Choose a reason for hiding this comment

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

Alright let's get this in.

@mbaret mbaret merged commit 017ff94 into apache:main Mar 11, 2021
@mbaret
Copy link
Contributor

mbaret commented Mar 11, 2021

Thanks @ANSHUMAN87, @tkonolige, this is merged.

trevor-m pushed a commit to trevor-m/tvm that referenced this pull request May 6, 2021
[TOPI] Sparse Add Op added (apache#7435)
88c2a22 
* [TOPI] Sparse Add Op added

* lint resolved

* TF frontend support added

* Test case added

* [1] Review comment handled

* [2] Review comment handled

* [3] Review comment handled

* [4] Review comment handled

* [5] Review comment handled
trevor-m pushed a commit to neo-ai/tvm that referenced this pull request May 11, 2021
@trevor-m
[TOPI] Sparse Add Op added (apache#7435)
6125b77 
* [TOPI] Sparse Add Op added

* lint resolved

* TF frontend support added

* Test case added

* [1] Review comment handled

* [2] Review comment handled

* [3] Review comment handled

* [4] Review comment handled

* [5] Review comment handled
@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

@mbaret mbaret mbaret approved these changes

@tkonolige tkonolige tkonolige 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.
3 participants
@ANSHUMAN87 @mbaret @tkonolige