Skip to content

Allow Softplus activation in ONNX surrogate parser#1743

Open
karllindvall wants to merge 2 commits intoIDAES:mainfrom
karllindvall:main
Open

Allow Softplus activation in ONNX surrogate parser#1743
karllindvall wants to merge 2 commits intoIDAES:mainfrom
karllindvall:main

Conversation

@karllindvall
Copy link

Summary
Add Softplus to the list of allowed activation functions when loading ONNX neural network surrogates.

Motivation
Softplus is a commonly used smooth activation function and is already supported by OMLT. However, when loading ONNX surrogates through IDAES, idaes.core.surrogate.onnx_surrogate currently overwrites the OMLT activation allowlist with:

["Relu", "Sigmoid", "LogSoftmax", "Tanh"]

This prevents ONNX networks using Softplus from being parsed and results in:

ValueError: Unhandled node type Softplus

Change
Add "Softplus" to the activation allowlist:

["Relu", "Sigmoid", "LogSoftmax", "Tanh", "Softplus"]

Result
ONNX networks exported with Softplus activation functions can now be loaded successfully through the IDAES ONNX surrogate interface.

Notes
OMLT already includes support for Softplus, so this change simply removes an unnecessary restriction in the IDAES wrapper.

Copy link
Contributor

@avdudchenko avdudchenko left a comment

Choose a reason for hiding this comment

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

I suggest we try removing the line of code that modifies _activation_op_types as it should be redundant with the newer version of OMLT. We can consider adding an option to modify this list if user supplies their own list of supported activation functions, but that might be problematic and unnecessary.

Copy link
Contributor

Choose a reason for hiding this comment

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

@karllindvall So I added that line at the time because OMLT did not expose tanh (as OMLT was only released on pipy for version of 1.1, which excluded tanh,, with current version being 1.22. I believe now all of the functions (including softplus) should be included in _ACTIVATION_OP_TYPES out of the box.

We probably can remove this line of code entirely, as it is now redundant.

@karllindvall
Copy link
Author

Agreed! In OMLT’s onnx_parser.py they now have a hard-coded _ACTIVATION_OP_TYPE with allowed activation functions, so I agree that the line is now redundant.

I have also added a pull-request to OMLT’s repository with the logic for adding softplus as an accepted activation function, as well as to accept activation function nodes as stand-alone nodes and not only attached to another layer. Hopefully these changes will go through as well, after which softplus will be an accepted activation function.

@avdudchenko
Copy link
Contributor

Agreed! In OMLT’s onnx_parser.py they now have a hard-coded _ACTIVATION_OP_TYPE with allowed activation functions, so I agree that the line is now redundant.

I have also added a pull-request to OMLT’s repository with the logic for adding softplus as an accepted activation function, as well as to accept activation function nodes as stand-alone nodes and not only attached to another layer. Hopefully these changes will go through as well, after which softplus will be an accepted activation function.

Do you need any changes to IDAES onnx surrogate models to support the soft plus stand alone nodes? or is it purely just OMLT issue?

I guess we are in agreement on removing the _ACTIVATION_OP_TYPE line from the onnx_surrogate, (can you make the change and I can probably approve the workflow runs and hopefully after it can be merged).

@karllindvall
Copy link
Author

I have completely removed the line now, so any activation functions that are passed from OMLT to IDAES should now go through. Regarding the standalone node edit, it is purely on the OMLT-side, so there is nothing you need to change.

Let me know if any problems arise.

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.

2 participants