[tmva][sofie] Add HardSigmoid operator for ONNX inference

[AdityaDRathore]

This Pull Request

Add support for the ONNX HardSigmoid operator in SOFIE.

Checklist:

• tested changes locally (TestSofieHardSigmoid passing)
• updated the docs (if necessary)

cc: @moneta @sanjibansg @guitargeek

[sanjibansg]

Hi @AdityaDRathore,
Thanks for your contribution and developing the operator implementation for Hard-sigmoid. The implementation looks good to me. But, we are in the process of consolidating unary operations (like activation functions) into the ROperator_BasicUnary.hxx file. Can you make the corresponding changes?

Thanks.

[github-actions]

Test Results

    21 files      21 suites   2d 23h 37m 41s ⏱️
 3 837 tests  3 836 ✅  1 💤 0 ❌
72 256 runs  72 238 ✅ 18 💤 0 ❌

Results for commit d575330.

♻️ This comment has been updated with latest results.

[AdityaDRathore]

Hi @AdityaDRathore, Thanks for your contribution and developing the operator implementation for Hard-sigmoid. The implementation looks good to me. But, we are in the process of consolidating unary operations (like activation functions) into the ROperator_BasicUnary.hxx file. Can you make the corresponding changes?

Thanks.

Hi @sanjibansg, thanks for the review!

I started refactoring to move this into ROperator_BasicUnary, but I noticed a potential architectural mismatch ROperator_BasicUnary appears to be designed specifically for stateless operators via the UnaryOpTraits pattern.

Since HardSigmoid is stateful (it must store alpha and beta attributes parsed from the ONNX node), it doesn't fit naturally into the current BasicUnary design without forcing all other simple operators (like Abs, Cos) to carry unused attribute overhead.

I also checked how other stateful unary operators are handled and saw that LeakyRelu (which also carries an alpha attribute) is implemented as a standalone operator.

Given this precedent, do you think it is safer to keep HardSigmoid separate to preserve the clean, stateless nature of BasicUnary? Or do you have a specific pattern in mind for handling attributes within the traits system?

[sanjibansg]

Hi @AdityaDRathore, Thanks for your contribution and developing the operator implementation for Hard-sigmoid. The implementation looks good to me. But, we are in the process of consolidating unary operations (like activation functions) into the ROperator_BasicUnary.hxx file. Can you make the corresponding changes?
Thanks.

Hi @sanjibansg, thanks for the review!

I started refactoring to move this into ROperator_BasicUnary, but I noticed a potential architectural mismatch ROperator_BasicUnary appears to be designed specifically for stateless operators via the UnaryOpTraits pattern.

Since HardSigmoid is stateful (it must store alpha and beta attributes parsed from the ONNX node), it doesn't fit naturally into the current BasicUnary design without forcing all other simple operators (like Abs, Cos) to carry unused attribute overhead.

I also checked how other stateful unary operators are handled and saw that LeakyRelu (which also carries an alpha attribute) is implemented as a standalone operator.

Given this precedent, do you think it is safer to keep HardSigmoid separate to preserve the clean, stateless nature of BasicUnary? Or do you have a specific pattern in mind for handling attributes within the traits system?

Ah yes! That is correct. Since this is a stateful operator, maybe better to keep this separately. Can you fix the formatting issues flagged in the CI? In the meantime, I will wait for the review of @lmoneta before we can merge this,

Thanks!

[AdityaDRathore]

Hi @sanjibansg
I have pushed the formatting fixes. The implementation is now clean and ready for the review.

[sanjibansg]

Hi @AdityaDRathore,
Can you try fixing the merge conflicts, and add the test following the convention in the SOFIE subdirectory:
https://github.com/root-project/root/tree/master/tmva/sofie/test

[AdityaDRathore]

Hi, @sanjibansg
I fixed the merge conflicts and added an end-to-end test to follow the standard SOFIE convention you mentioned.

[AdityaDRathore]

Hi @sanjibansg,
I've updated my addtion with the SOFIE operator conventions and resolved the merge conflicts also. Can you please review it once again.