Skip to content

[Relay/TOPI][Op] Add erf intrinsic and op - #3702

Merged
yzhliu merged 12 commits into
apache:masterfrom
icemelon:erf
Sep 9, 2019
Merged

[Relay/TOPI][Op] Add erf intrinsic and op#3702
yzhliu merged 12 commits into
apache:masterfrom
icemelon:erf

Conversation

@icemelon

@icemelon icemelon commented Aug 3, 2019

Copy link
Copy Markdown
Member

@yzhliu @masahi @yongwww @kevinthesun could you help review this pr?

@soiferj

soiferj commented Aug 3, 2019

Copy link
Copy Markdown
Contributor

Would you mind also adding this to the TensorFlow frontend? The op name is Erf.

@icemelon

icemelon commented Aug 3, 2019

Copy link
Copy Markdown
Member Author

@soiferj Sure, will add it.

@soiferj

soiferj commented Aug 8, 2019

Copy link
Copy Markdown
Contributor

@icemelon9 , is there a timeline to get this PR ready for review? This would be really nice to unblock running BERT-based models!

@icemelon

Copy link
Copy Markdown
Member Author

@soiferj Sorry about the delay. I was working on the dynamic shape function support for Any. I'll start updating this PR this week.

Comment thread topi/tests/python/test_topi_basic.py Outdated
@icemelon
icemelon marked this pull request as ready for review September 4, 2019 03:25
@icemelon

icemelon commented Sep 4, 2019

Copy link
Copy Markdown
Member Author

@tqchen Could you help check if the vectorizable intrinsic list is complete?



RELAY_REGISTER_UNARY_OP("erf")
.describe(R"code(Returns the error function value for input array, computed element-wise.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
.describe(R"code(Returns the error function value for input array, computed element-wise.
.describe(R"code(Returns the Gauss error function value for input array, computed element-wise.

@yongwww yongwww left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM

@yzhliu yzhliu left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

erf changes looks good to me

inputs[0] = _op.nn.batch_flatten(inputs[0])
data_shape = _infer_type(inputs[0]).checked_type.shape
if len(data_shape) > 2:
inputs[0] = _op.reverse_reshape(inputs[0], [-1, 0])

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

could you elaborate a bit what are these changes for?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

This is because mxnet dense op allows (d1, d2, ..., dk, in_dim), (out_dim, in_dim) --> (d1, d2, .., dk, out_dim). But this isn't allowed in topi's dense op.

@yzhliu
yzhliu merged commit 2f5b155 into apache:master Sep 9, 2019
@yzhliu

yzhliu commented Sep 9, 2019

Copy link
Copy Markdown
Member

Thanks @icemelon9 @yongwww

@icemelon
icemelon deleted the erf branch September 9, 2019 16:56
wweic pushed a commit to wweic/tvm that referenced this pull request Sep 16, 2019
* add more ops

* stop vectorization for erf

* x

* cleanup

* fix

* add whitelist for vectorizable intrin

* add tf converter

* fix dense

* fix

* add missing intrin

* fix mxnet frontend

* fix nvptx
wweic pushed a commit to wweic/tvm that referenced this pull request Sep 16, 2019
* add more ops

* stop vectorization for erf

* x

* cleanup

* fix

* add whitelist for vectorizable intrin

* add tf converter

* fix dense

* fix

* add missing intrin

* fix mxnet frontend

* fix nvptx
wweic pushed a commit to neo-ai/tvm that referenced this pull request Sep 16, 2019
* add more ops

* stop vectorization for erf

* x

* cleanup

* fix

* add whitelist for vectorizable intrin

* add tf converter

* fix dense

* fix

* add missing intrin

* fix mxnet frontend

* fix nvptx
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.

5 participants