Skip to content

Implement mish as a function#4350

Merged
gramalingam merged 13 commits intoonnx:mainfrom
andife:mish
Jul 27, 2022
Merged

Implement mish as a function#4350
gramalingam merged 13 commits intoonnx:mainfrom
andife:mish

Conversation

@andife
Copy link
Copy Markdown
Member

@andife andife commented Jul 7, 2022 •
edited
Loading

Description
Introduce the mish activation function

Motivation and Context

Mish is used in YoloV4

Mish was requested here:
#3475 and #2818

It is implemented in

It can be calculated using the following formula:
mish(x) = x * tanh(softplus(x)) = x * tanh(ln(1 + e^{x}))

Note
It's my first function and I've tried to use other implementations as a guide. If you have any suggestions on how to improve the proposal or what I missed, please let me know.

I wonder if it would be useful / helpful to create a small documentation with technical details. "How to implement a new operator or function". How to deal with defs.cc ? How to create attributes.

Signed-off-by: andife fehlner@arcor.de

@andife andife requested a review from a team as a code owner July 7, 2022 20:37
@andife andife changed the title WIP: Implement mish as a function [WIP] Implement mish as a function Jul 7, 2022
@andife andife added the topic: operator Issues related to ONNX operators label Jul 8, 2022
@andife andife marked this pull request as draft July 8, 2022 08:49
@andife
add mish as function
7070517 
Signed-off-by: andife <fehlner@arcor.de>
@andife andife marked this pull request as ready for review July 10, 2022 09:31
@andife andife requested a review from a team as a code owner July 10, 2022 09:31
@andife andife changed the title [WIP] Implement mish as a function Implement mish as a function Jul 10, 2022
@gramalingam
Copy link
Copy Markdown
Contributor

gramalingam commented Jul 12, 2022 •
edited
Loading

Re:

I wonder if it would be useful / helpful to create a small documentation with technical details.
"How to implement a new operator or function". How to deal with defs.cc ? How to create attributes.

Yes, it would be very useful.

Just to clarify: there is already a doc file here: https://github.com/onnx/onnx/blob/main/docs/AddNewOp.md ... if you have suggestions for what new stuff to add, that would be welcome, thanks!

gramalingam
gramalingam reviewed Jul 12, 2022
View reviewed changes
Comment thread onnx/defs/operator_sets.h Outdated
gramalingam
gramalingam reviewed Jul 12, 2022
View reviewed changes
Comment thread onnx/test/test_with_ort.py
gramalingam
gramalingam reviewed Jul 12, 2022
View reviewed changes
Comment thread onnx/defs/operator_sets.h Outdated
@gramalingam
Copy link
Copy Markdown
Contributor

gramalingam commented Jul 12, 2022

You need to generate the test-case data and include it in the PR. Eg., see https://github.com/onnx/onnx/blob/main/tools/update_doc.sh for the steps to generate the updated files.

andife added 2 commits July 13, 2022 19:36
@andife
Merge branch 'main' into mish
a4f29e1
@andife
Fix opset 17 -> 18
41bb72d 
Signed-off-by: andife <fehlner@arcor.de>
andife added 2 commits July 13, 2022 22:33
@andife
gen doc
ec01af0 
Signed-off-by: andife <fehlner@arcor.de>
@andife
#clang
ed105c8 
Signed-off-by: andife <fehlner@arcor.de>
@andife
Copy link
Copy Markdown
Member Author

andife commented Jul 13, 2022

You need to generate the test-case data and include it in the PR. Eg., see https://github.com/onnx/onnx/blob/main/tools/update_doc.sh for the steps to generate the updated files.
I generated the files and pushed the mish related ones.

I wonder why also

geändert:       onnx/backend/test/data/node/test_acos/test_data_set_0/output_0.pb
geändert:       onnx/backend/test/data/node/test_acosh/test_data_set_0/output_0.pb
geändert:       onnx/backend/test/data/node/test_asin/test_data_set_0/output_0.pb
geändert:       onnx/backend/test/data/node/test_asinh/test_data_set_0/output_0.pb
geändert:       onnx/backend/test/data/node/test_atan/test_data_set_0/output_0.pb
geändert:       onnx/backend/test/data/node/test_atanh/test_data_set_0/output_0.pb
geändert:       onnx/backend/test/data/node/test_cosh/test_data_set_0/output_0.pb
geändert:       onnx/backend/test/data/node/test_cosh_example/test_data_set_0/output_0.pb
geändert:       onnx/backend/test/data/node/test_lstm_batchwise/test_data_set_0/output_0.pb
geändert:       onnx/backend/test/data/node/test_lstm_batchwise/test_data_set_0/output_1.pb
geändert:       onnx/backend/test/data/node/test_lstm_defaults/test_data_set_0/output_0.pb
geändert:       onnx/backend/test/data/node/test_lstm_with_initial_bias/test_data_set_0/output_0.pb
geändert:       onnx/backend/test/data/node/test_lstm_with_peepholes/test_data_set_0/output_0.pb
geändert:       onnx/backend/test/data/node/test_reduce_log_sum_exp_do_not_keepdims_example/test_data_set_0/output_0.pb
geändert:       onnx/backend/test/data/node/test_reduce_log_sum_exp_do_not_keepdims_random/test_data_set_0/output_0.pb
geändert:       onnx/backend/test/data/node/test_reduce_log_sum_exp_keepdims_example/test_data_set_0/output_0.pb
geändert:       onnx/backend/test/data/node/test_reduce_log_sum_exp_keepdims_random/test_data_set_0/output_0.pb
geändert:       onnx/backend/test/data/node/test_reduce_log_sum_exp_negative_axes_keepdims_example/test_data_set_0/output_0.pb
geändert:       onnx/backend/test/data/node/test_reduce_log_sum_exp_negative_axes_keepdims_random/test_data_set_0/output_0.pb
geändert:       onnx/backend/test/data/node/test_rnn_seq_length/test_data_set_0/output_0.pb
geändert:       onnx/backend/test/data/node/test_simple_rnn_batchwise/test_data_set_0/output_0.pb
geändert:       onnx/backend/test/data/node/test_simple_rnn_batchwise/test_data_set_0/output_1.pb
geändert:       onnx/backend/test/data/node/test_simple_rnn_defaults/test_data_set_0/output_0.pb
geändert:       onnx/backend/test/data/node/test_sinh/test_data_set_0/output_0.pb
geändert:       onnx/backend/test/data/node/test_tan/test_data_set_0/output_0.pb
geändert:       onnx/backend/test/data/node/test_tanh/test_data_set_0/output_0.pb

Do I've to commit than, too?

@andife andife closed this Jul 13, 2022
@andife andife reopened this Jul 13, 2022
@jcwchen
Copy link
Copy Markdown
Member

jcwchen commented Jul 19, 2022

Do I've to commit than, too?

No need to commit other unrelated input.pb/output.pb. They were updated because different platforms might behave differently for input/output generation. Please just ignore them. Thanks!

gramalingam
gramalingam reviewed Jul 19, 2022
View reviewed changes
Comment thread onnx/defs/math/defs.cc
.Output(0, "Y", "Output tensor", "T", OpSchema::Single, true, 1, OpSchema::Differentiable)
.TypeConstraint(
"T",
{"tensor(float16)", "tensor(float)", "tensor(double)"},
Copy link
Copy Markdown
Contributor

@gramalingam gramalingam Jul 19, 2022

Choose a reason for hiding this comment

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

Wondering whether we should also include bfloat16 as an allowed type. Any reasons/concerns for or against this? Thanks!

Copy link
Copy Markdown
Contributor

@gramalingam gramalingam Jul 19, 2022

Choose a reason for hiding this comment

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

Never mind. It looks like Softplus doesn't support bfloat16. Better to add bfloat16 support to both ops at same time.

p-wysocki
p-wysocki approved these changes Jul 20, 2022
View reviewed changes
Copy link
Copy Markdown
Contributor

@p-wysocki p-wysocki left a comment

Choose a reason for hiding this comment

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

LGTM

Comment thread onnx/backend/test/case/node/mish.py Outdated
Comment on lines +20 to +28
input_data = np.array([[[[0.8439683], [0.5665144], [0.05836735]],
[[0.02916367], [0.12964272], [0.5060197]],
[[0.79538304], [0.9411346], [0.9546573]]],
[[[0.17730942], [0.46192095], [0.26480448]],
[[0.6746842], [0.01665257], [0.62473077]],
[[0.9240844], [0.9722341], [0.11965699]]],
[[[0.41356155], [0.9129373], [0.59330076]],
[[0.81929934], [0.7862604], [0.11799799]],
[[0.69248444], [0.54119414], [0.07513223]]]], dtype=np.float32)
Copy link
Copy Markdown
Contributor

@p-wysocki p-wysocki Jul 20, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

Some negative values could be included in the test, just in case. It's up to you, I don't think it's absolutely necessary.

andife added 3 commits July 21, 2022 14:38
@andife
fix flake
f40a084 
Signed-off-by: andife <fehlner@arcor.de>
@andife
#DC flake
210099e 
Signed-off-by: andife <fehlner@arcor.de>
@andife
Merge branch 'main' into mish
1c37033
@gramalingam gramalingam enabled auto-merge (squash) July 27, 2022 14:39
@gramalingam gramalingam merged commit 84f6ff6 into onnx:main Jul 27, 2022
@jcwchen jcwchen mentioned this pull request Aug 8, 2022
Closed
@andife andife deleted the mish branch December 15, 2022 10:20
broune pushed a commit to broune/onnx that referenced this pull request May 6, 2023
@andife
Implement mish as a function (onnx#4350)
04df1f2 
* add mish as function

Signed-off-by: andife <fehlner@arcor.de>

* Fix opset 17 -> 18

Signed-off-by: andife <fehlner@arcor.de>

* gen doc

Signed-off-by: andife <fehlner@arcor.de>

* #clang

Signed-off-by: andife <fehlner@arcor.de>

* add negative values to test case

Signed-off-by: andife <fehlner@arcor.de>

* fix flake

Signed-off-by: andife <fehlner@arcor.de>

* #DC flake

Signed-off-by: andife <fehlner@arcor.de>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@gramalingam gramalingam gramalingam approved these changes

+1 more reviewer

@p-wysocki p-wysocki p-wysocki approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

topic: operator Issues related to ONNX operators

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@andife @gramalingam @jcwchen @p-wysocki