[tmva][sofie] Add ONNX InstanceNormalization operator#21331
[tmva][sofie] Add ONNX InstanceNormalization operator#21331gluonparticle wants to merge 1 commit intoroot-project:masterfrom
Conversation
Test Results 21 files 21 suites 2d 23h 9m 46s ⏱️ For more details on these failures, see this check. Results for commit 94047f9. ♻️ This comment has been updated with latest results. |
|
@lmoneta I've re run the clang-format with correct flags I'd missed before. Regarding the other CI pipeline failures (specifically on mac-beta ARM64 and ubuntu2604), looking at the logs, these appear to be existing infrastructure regressions in the PyROOT/cppyy bindings and the DistRDF Spark backends. Please let me know if you need anything else from my end, Thanks! ; |
|
@lmoneta @bellenot @sanjibansg Sorry to bother you again. Please let me know if you need anything else from my side to re-run the checks and/or anything from my side on this PR. The other failed checks (specifically on mac-beta ARM64 and ubuntu2604) appear to be existing infrastructure regressions in the PyROOT/cppyy bindings and the DistRDF Spark backends. Thanks! ; |
sanjibansg
left a comment
There was a problem hiding this comment.
Hi @gluonparticle,
Thanks for your PR on adding the implementation for the InstanceNormalization operator. I have a question regarding the test you propose to add.
Please let me know if you need anything else from my side. Thanks! ; |
bec3a76 to
2be6493
Compare
Hi @gluonparticle, Thanks, |
Hi @sanjibansg We definitely just crossed paths like a merge conflict. I was in the middle of fixing that last remaining conflict when you commented. Everything is now fully resolved and synced with master. I've also ran the git-clang-format wrt origin/master and clang format on the files. Thank you for your cooperation ; |
|
Hi @sanjibansg Could you please re-run the workflow trigger for the CI/CD checks. Thanks for your cooperation ; |
sanjibansg
left a comment
There was a problem hiding this comment.
I see there are still some conflicts in tmva/sofie_parsers/src/RModelParser_ONNX.cxx, could you try rebasing with the master?
81f62f5 to
94047f9
Compare
Hi @sanjibansg , I've rebased and manually resolved the new conflicts along with local rebuild for the changes. Thanks! ; |
sanjibansg
left a comment
There was a problem hiding this comment.
Hi @gluonparticle,
Thanks for this implementation, could of comments.
| #include <unordered_map> | ||
| #include <functional> | ||
| #include "TMVA/SOFIE_common.hxx" | ||
| #include "TMVA/ROperator_InstanceNormalization.hxx" |
There was a problem hiding this comment.
I do not think this is where it should be included
| // ----------------------------------------------------------------------------- | ||
| // InstanceNormalization | ||
| // ----------------------------------------------------------------------------- | ||
| std::unique_ptr<ROperator> ParseInstanceNormalization(RModelParser_ONNX & /*parser*/, const onnx::NodeProto &node) |
There was a problem hiding this comment.
Could you follow the convention in this subdirectory on adding the parsing methods like we do in https://github.com/gluonparticle/root/blob/94047f9f573478b77d53cecd27de46481e7da7e2/tmva/sofie_parsers/src/ParseBatchNormalization.cxx
InstanceNormalization Operator for SOFIE Inference Engine
Why
InstanceNormalizationis widely used in modern computer vision models (e.g., Style Transfer, GANs).Supporting this operator in the SOFIE ONNX parser expands the range of physics-adjacent ML models that can be converted to high-performance C++.
What
ROperator_InstanceNormalization.hxx$N, C, H, W$ normalization logic.
New template implementing the
RModelParser_ONNX.cxx
Registered the operator and added epsilon attribute parsing.
test_InstanceNormalization.cxx
Added a unit test to verify the correctness of emitted C++ code.
Testing
-Ddev=ONusing GCC 15.2.1.test_instancenormpassed with[SUCCESS].