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

Integration of HardSigmoid Operation #185

Merged
merged 3 commits into from
Jan 1, 2024

Conversation

ariaghora
Copy link
Contributor

This pull request introduces the HardSigmoid operation into the existing framework.

Motivation:

Several recent and good-performing OCR models (such as those found in PaddleOCR) utilize ConvTranspose and HardSigmoid. The ConvTranspose PR (#182) is currently in progress, and with anticipation for its completion, I decided to help implementing HardSigmoid part.

Changes:

  • Updated match expression in wonnx/src/compiler.rs
  • Updated wonnx/templates/snippets/activation_scalar.wgsl and wonnx/templates/snippets/activation_vec.wgsl (not sure whether I should implement for both files, but modified both, anyway)
  • Added test file wonnx/tests/hard_sigmoid.rs as I was not sure where to put tests for activation function like this

Please review the changes and merge if you're OK. Feel free to request any modifications if necessary.

@pixelspark
Copy link
Collaborator

@ariaghora looks good to me! Two things before I can merge this:

  • Could you please edit README.md in your PR as well to indicate suport?
  • Could you enable the HardSigmoid test(s) in the ONNX backend test, if they have tests for this op?

@ariaghora
Copy link
Contributor Author

ariaghora commented Aug 27, 2023

@pixelspark nice! Few things to clarify:

  • Should I put checkmark indicating support only on "Implemented" column, or should I put it on "Shape inference supported" column as well? I'm not sure about the second one (since I didn't explicitly implement shape inference stuff).
  • By "enable the HardSigmoid test(s)" do you mean modifying wonnx-py/tests/test_onnx_backend.py and adding something like backend_test.include(f"test_hard_sigmoid_[a-z,_]*")? I'll check if they have tests for this op, anyway.

@pixelspark
Copy link
Collaborator

@pixelspark nice! Few things to clarify:

  • Should I put checkmark indicating support only on "Implemented" column, or should I put it on "Shape inference supported" column as well? I'm not sure about the second one (since I didn't explicitly implement shape inference stuff).

  • By "enable the HardSigmoid test(s)" do you mean modifying wonnx-py/tests/test_onnx_backend.py and adding something like backend_test.include(f"test_hard_sigmoid_[a-z,_]*")? I'll check if they have tests for this op, anyway.

Yes and yes! (And no need to enable the test if there is none of course. If it fails for some reason but the op is still useful, just leave the test disabled and make a note somewhere)

Shape inference is not too difficult to implement, I assume you can just add this op in the list with other ops that take one input and provide one output with exactly the same shape. Then of course you can insert two checkmarks in the readme.

Other changes:
- Update repo README.md to indicate support for hardsigmoid
- Rename `hard_sigmoid` to `hardsigmoid` in few places to be consistent with onnx standard
@ariaghora
Copy link
Contributor Author

@pixelspark Yes, it gives an output with the same shape as the input. I put two checkmarks.

One more thing: some tests failed due to unimplemented CastLike node. Is it a known issue?

==================================================================================== short test summary info ====================================================================================
FAILED tests/test_specific_op.py::OnnxBackendNodeModelTest::test_hardsigmoid_default_expanded_ver18_cpu - pyo3_runtime.PanicException: called `Result::unwrap()` on an `Err` value: GpuError(CompileError { node: "CastLike", error: UnimplementedOp("CastLike") })
FAILED tests/test_specific_op.py::OnnxBackendNodeModelTest::test_hardsigmoid_example_expanded_ver18_cpu - pyo3_runtime.PanicException: called `Result::unwrap()` on an `Err` value: GpuError(CompileError { node: "CastLike", error: UnimplementedOp("CastLike") })
FAILED tests/test_specific_op.py::OnnxBackendNodeModelTest::test_hardsigmoid_expanded_ver18_cpu - pyo3_runtime.PanicException: called `Result::unwrap()` on an `Err` value: GpuError(CompileError { node: "CastLike", error: UnimplementedOp("CastLike") })
===================================================================== 3 failed, 3 passed, 2628 skipped, 1 warning in 2.51s ======================================================================

Other than that, I guess it is ready.

@pixelspark
Copy link
Collaborator

@pixelspark Yes, it gives an output with the same shape as the input. I put two checkmarks.

One more thing: some tests failed due to unimplemented CastLike node. Is it a known issue?

==================================================================================== short test summary info ====================================================================================
FAILED tests/test_specific_op.py::OnnxBackendNodeModelTest::test_hardsigmoid_default_expanded_ver18_cpu - pyo3_runtime.PanicException: called `Result::unwrap()` on an `Err` value: GpuError(CompileError { node: "CastLike", error: UnimplementedOp("CastLike") })
FAILED tests/test_specific_op.py::OnnxBackendNodeModelTest::test_hardsigmoid_example_expanded_ver18_cpu - pyo3_runtime.PanicException: called `Result::unwrap()` on an `Err` value: GpuError(CompileError { node: "CastLike", error: UnimplementedOp("CastLike") })
FAILED tests/test_specific_op.py::OnnxBackendNodeModelTest::test_hardsigmoid_expanded_ver18_cpu - pyo3_runtime.PanicException: called `Result::unwrap()` on an `Err` value: GpuError(CompileError { node: "CastLike", error: UnimplementedOp("CastLike") })
===================================================================== 3 failed, 3 passed, 2628 skipped, 1 warning in 2.51s ======================================================================

Other than that, I guess it is ready.

I think CastLike is not implemented - just disable these tests for now. I will then merge this PR (finally :-))

@ariaghora
Copy link
Contributor Author

I disabled the tests.

(off-topic: is CastLike being implemented now? I might be able to help on this.)

@pixelspark
Copy link
Collaborator

I disabled the tests.

OK, will run CI and then merge. Thanks!

(off-topic: is CastLike being implemented now? I might be able to help on this.)

Not as far as I know, feel free to go ahead!

@pixelspark pixelspark merged commit 2b72b51 into webonnx:master Jan 1, 2024
25 of 27 checks passed
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