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.

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

Add Swish operator #5964

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Add Swish operator #5964

wants to merge 1 commit into from

Conversation

isdanni
Copy link
Contributor

@isdanni isdanni commented Feb 26, 2024

Description

These changes have been made to support the SiLU/Swish operator as a function op.

Motivation and Context

Close #5853

The SiLU function can be defined as $x * Sigmoid (beta * x)$ where $beta = 1.0$.

Swish ops is used in Yolo series and EfficientNet.

@isdanni
Copy link
Contributor Author

isdanni commented Feb 26, 2024

Would appreciate some guidance on adding tests to backend/test/data/node 😃

onnx/defs/math/defs.cc Fixed Show fixed Hide fixed
onnx/defs/math/defs.cc Fixed Show fixed Hide fixed
Copy link

codecov bot commented Feb 26, 2024

Codecov Report

Attention: Patch coverage is 12.50000% with 14 lines in your changes are missing coverage. Please review.

Project coverage is 57.01%. Comparing base (83194ed) to head (7691a35).
Report is 33 commits behind head on main.

Files Patch % Lines
onnx/backend/test/case/node/swish.py 0.00% 14 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #5964      +/-   ##
==========================================
+ Coverage   56.95%   57.01%   +0.06%     
==========================================
  Files         506      507       +1     
  Lines       30467    30951     +484     
  Branches     4592     4593       +1     
==========================================
+ Hits        17353    17648     +295     
- Misses      12285    12478     +193     
+ Partials      829      825       -4     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

.SetDoc(SiLU_ver21_doc)
.Input(0, "X", "Input tensor", "T", OpSchema::Single, true, 1, OpSchema::Differentiable)
.Output(0, "Y", "Output tensor", "T", OpSchema::Single, true, 1, OpSchema::Differentiable)
.TypeConstraint(
Copy link
Contributor

Choose a reason for hiding this comment

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

beta is missing?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If we are using SiLU I'm assuming beta is 1.0? I can add it tho

Copy link
Contributor

Choose a reason for hiding this comment

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

I notice HardSwish uses alpha and beta in the form alpha*x + beta ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Based on #5964 (comment) I will add beta and rename the operator to swish, thanks for reviewing this!

@@ -644,6 +644,25 @@ ONNX_OPERATOR_SET_SCHEMA(
.SetContextDependentFunctionBodyBuilder(BuildContextDependentFunctionBodyGelu)
.TypeAndShapeInferenceFunction(propagateShapeAndTypeFromFirstInput));

static const char* SiLU_ver21_doc = R"DOC(
Sigmoid Linear Unit (SiLU), or known as the Swish function, takes one input data (Tensor<T>)
Copy link
Contributor

Choose a reason for hiding this comment

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

Since we have HardSwish, would it be more uniform to call this function-op Swish ? Just a minor matter, just wondering.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

good idea! I will update this to swish and add the beta parameter.

@gramalingam
Copy link
Contributor

Would appreciate some guidance on adding tests to backend/test/data/node 😃

Please take a look at this line and this line for updating generated files.

@isdanni isdanni changed the title Add SiLU operator Add Swish operator Mar 5, 2024
@justinchuby justinchuby added this to the 1.17 milestone Mar 14, 2024
@isdanni isdanni force-pushed the swish branch 4 times, most recently from 46a4ce8 to e38a91a Compare March 18, 2024 04:40
.FunctionBody(
R"ONNX(
{
S_X = Sigmoid<beta = 1.0>(X)
Copy link
Contributor

Choose a reason for hiding this comment

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

Sigmoid doesn't have an attribute. We should be doing something like below:

   Beta = Constant <value_float: float = @beta>()
   BetaCast = CastLike (Beta, X)
   BetaX = Mul (BetaCast, X)
   SigmoidBetaX = Sigmoid(BetaX)
   Y = Mul (X, SigmoidBetaX)

Copy link
Contributor

Choose a reason for hiding this comment

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

However, as commented above, it seems to me we should be calling this alpha instead of beta, based on the convention in HardSwish.


ONNX_OPERATOR_SET_SCHEMA(
Swish,
21,
Copy link
Contributor

Choose a reason for hiding this comment

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

This will now need to be 22

@@ -1198,6 +1198,9 @@ def test_Sigmoid(self) -> None:
def test_Sign(self) -> None:
self._test_op_upgrade("Sign", 9)

def test_Swish(self) -> None:
self._test_op_upgrade("Swish", 21)
Copy link
Contributor

Choose a reason for hiding this comment

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

Now 22

@justinchuby justinchuby added review needed: operators approvers Require reviews from members of operators-approvers and removed review needed: operators approvers Require reviews from members of operators-approvers labels Apr 25, 2024
@isdanni isdanni force-pushed the swish branch 2 times, most recently from f737693 to 2633a85 Compare April 26, 2024 02:03
@isdanni
Copy link
Contributor Author

isdanni commented Apr 26, 2024

Would appreciate some guidance on adding tests to backend/test/data/node 😃

Please take a look at this line and this line for updating generated files.

Thanks for the review!! One followup question: when I run both commands after updating the operator to 22, there are ~20 .onnx files changed, instead of just a few output_0.pb in the last commit, is this normal behaviour?

@isdanni isdanni force-pushed the swish branch 6 times, most recently from 75ae9df to b0ceb4d Compare May 2, 2024 02:19
@isdanni isdanni force-pushed the swish branch 3 times, most recently from a29a276 to 66233c4 Compare May 7, 2024 03:58
Signed-off-by: isdanni <leedanni@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: In progress
Development

Successfully merging this pull request may close these issues.

Request for Swish Op
4 participants