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][OP] Register topi schedule for Relay fast_exp and fast_tanh #5131

Merged
merged 8 commits into from
Mar 26, 2020
Merged

[Relay][OP] Register topi schedule for Relay fast_exp and fast_tanh #5131

merged 8 commits into from
Mar 26, 2020

Conversation

selo1412
Copy link
Contributor

@selo1412 selo1412 commented Mar 23, 2020 •
edited
Loading

I raised a question in TVM community.
(https://discuss.tvm.ai/t/relay-op-fast-exp-cant-be-built/6046).
If there's any problem, please don't hesitate to point it out. Thanks.

@comaniac
Copy link
Contributor

Could we have a unit test to guard this issue in the future?

cc @icemelon9

@icemelon icemelon self-assigned this Mar 23, 2020
@selo1412 selo1412 changed the title register topi schedule for Relay fast_exp and fast_tanh [RELAY][TOPI]register topi schedule for Relay fast_exp and fast_tanh Mar 24, 2020
@selo1412 selo1412 changed the title [RELAY][TOPI]register topi schedule for Relay fast_exp and fast_tanh [RELAY][TOPI] Register topi schedule for Relay fast_exp and fast_tanh Mar 24, 2020
@selo1412 selo1412 changed the title [RELAY][TOPI] Register topi schedule for Relay fast_exp and fast_tanh [Relay][TOPI] Register topi schedule for Relay fast_exp and fast_tanh Mar 24, 2020
@icemelon
Copy link
Member

@selo1412 Could you address @comaniac comments?

@icemelon icemelon changed the title [Relay][TOPI] Register topi schedule for Relay fast_exp and fast_tanh [Relay][OP] Register topi schedule for Relay fast_exp and fast_tanh Mar 24, 2020
@selo1412
Copy link
Contributor Author

@comaniac @icemelon9
I took
tvm/tests/python/relay/test_pass_fast_math.py
tvm/topi/tests/python/test_topi_math.py
as reference to add unit test
tvm/tests/python/relay/test_op_fast_math.py .
and I found some error in topi/tests/python/ make the commit 5 cannot be built.

Is there any suggestions for the above situation?

comaniac
comaniac approved these changes Mar 25, 2020
View reviewed changes
Copy link
Contributor

@comaniac comaniac left a comment

Choose a reason for hiding this comment

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

LGTM. The CI failure seems not related to the change in this PR. You could push a dummy commit to re-trigger the CI to see if the error is gone.

icemelon
icemelon reviewed Mar 25, 2020
View reviewed changes
tests/python/relay/test_op_fast_math.py Outdated
test_apply(relay.exp, "fast_exp", np.exp, low=-88, high=88, step=0.01)
test_apply(relay.tanh, "fast_tanh", np.tanh, low=-10, high=10, step=0.01)

if __name__ == "__main__":
Copy link
Member

Choose a reason for hiding this comment

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

fix the indent

@selo1412
Copy link
Contributor Author

selo1412 commented Mar 26, 2020 •
edited
Loading

@comaniac @icemelon9
Thanks for your help.
And, what should I do next?

By the way, I find that
tvm/topi/tests/python/test_topi_math.py
maybe, test_apply(fast_tanh) is missing in test_fastmath().

@icemelon
Copy link
Member

Yes, could you also add fast_tanh in the test_fastmath?

@icemelon icemelon merged commit 35ee8f6 into apache:master Mar 26, 2020
@icemelon
Copy link
Member

Thanks @selo1412 @comaniac. This is now merged.

trevor-m pushed a commit to trevor-m/tvm that referenced this pull request Apr 16, 2020
@selo1412
[Relay][OP] Register topi schedule for Relay fast_exp and fast_tanh (a…
d52474b 
…pache#5131)

* register for fast_exp and fast_tanh

* Add unit test for fast math

* Add unit test for op fast math

* Add unit test for op fast math

* Add unit tests to guard registering topi schedule for Relay fast_exp and fast_tanh

* Fix ident

* Fix the indent

* Add fast_tanh in the test_fastmath of topi tests
zhiics pushed a commit to neo-ai/tvm that referenced this pull request Apr 17, 2020
@selo1412 @zhiics
[Relay][OP] Register topi schedule for Relay fast_exp and fast_tanh (a…
66f6f2f 
…pache#5131)

* register for fast_exp and fast_tanh

* Add unit test for fast math

* Add unit test for op fast math

* Add unit test for op fast math

* Add unit tests to guard registering topi schedule for Relay fast_exp and fast_tanh

* Fix ident

* Fix the indent

* Add fast_tanh in the test_fastmath of topi tests
@ZihengJiang ZihengJiang mentioned this pull request Sep 25, 2020
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@icemelon icemelon icemelon approved these changes

@comaniac comaniac comaniac approved these changes

Assignees

@icemelon icemelon

Labels
status: accepted
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@selo1412 @comaniac @icemelon