Skip to content

[tmva][sofie] Several Fixes for parsing complex models #22150

Merged
lmoneta merged 8 commits into
root-project:masterfrom
lmoneta:tmva_sofie_fixes
May 19, 2026
Merged

[tmva][sofie] Several Fixes for parsing complex models #22150
lmoneta merged 8 commits into
root-project:masterfrom
lmoneta:tmva_sofie_fixes

Conversation

@lmoneta
Copy link
Copy Markdown
Member

@lmoneta lmoneta commented May 5, 2026

  • After the changes for CLAD the mlpf modek could not be parsed anymore. Handle now correctly the variable defining the number of non zero elements coming from Non_Zero
  • Fixes also TMVA::SOFIE::Copy for different types than float making it a template function
  • Add also output shape definition in generated code as it is done for the input

@lmoneta lmoneta requested a review from guitargeek May 5, 2026 13:29
@lmoneta lmoneta self-assigned this May 5, 2026
Copy link
Copy Markdown
Contributor

@guitargeek guitargeek left a comment

Choose a reason for hiding this comment

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

Thanks you very much! Looks good to me once the test is fixed.

Copy link
Copy Markdown
Contributor

@guitargeek guitargeek left a comment

Choose a reason for hiding this comment

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

Actually I remember that the RModel has a non-zero class version, and we can't add new non-transient data members without incrementing the class version.

Does the RModel actually need to be serializable?

Same discussion as here:

Comment thread tmva/sofie/inc/TMVA/RModel.hxx
@github-actions
Copy link
Copy Markdown

github-actions Bot commented May 5, 2026

Test Results

    22 files      22 suites   3d 13h 27m 39s ⏱️
 3 855 tests  3 854 ✅ 0 💤 1 ❌
76 129 runs  76 128 ✅ 0 💤 1 ❌

For more details on these failures, see this check.

Results for commit 9e9a991.

♻️ This comment has been updated with latest results.

@lmoneta lmoneta changed the title [tmva][sofie] Fixes for parsing mlpf model [tmva][sofie] Several Fixes for parsing complex models May 8, 2026
@lmoneta lmoneta force-pushed the tmva_sofie_fixes branch 4 times, most recently from c6caf02 to 80c2c8c Compare May 12, 2026 22:29
Copy link
Copy Markdown
Contributor

@guitargeek guitargeek left a comment

Choose a reason for hiding this comment

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

Thank you very much for the fixes!

Comment thread tmva/sofie/inc/TMVA/RModel.hxx Outdated
lmoneta added 7 commits May 18, 2026 10:04
- After the changes for CLAD the mlpf modek could not be parsed anymore.
Handle now correctly the variable defining the number of non zero elements coming from Non_Zero
- Fixes also TMVA::SOFIE::Copy for different types than float making it a template function
- Add also output shape definition in generated code as it is done for the input
The casting to bool was incorrect since it was done a cast to uint8.

Fix also the special case of NonZero dynamic parameter which is defindef by NonZero operator. Add at the end a Session data member for the parameter which is then used in creating the output vector

Fix a bug introduced in softmax generated code in the generic case

Fix the writing of the data in initializer lists for uint8_t types

Add correctly new version in RModel.hxx (version 4)
- Fix Where for initialized and Shape tensors. New impelmentation was not  taking into account the Shape tensors. This caused a failure to parse the ATLAS Gnn tracking model

- Fix Slice for trivial copying. Use now std::copy since we cannot use alias tensor anymore after the change of using a free function with a const Session

- Avoid printing tensor names in the comment of Softmax generated code. There is a issue in the function RModel::CollectTensorMemberNames used to get tensor members from Session. The problem if a tensor is gaving as name "tensor_X" and used as member "tensor_tensor_X" the function assume exists a tensor with name "X". This was causing teh Keras parser to crash.

- Fix an issue writing the initialized data when are inf or NaN. Use the function from limits in this case
…pe tensor

When output is a param shape tensor the tensor values were not assigned in initialization as in a constant tensor, they need to be set at run time in the infer function
because they depend on the provided dynamic shale values

Fix also a issue on Windows in the new COnvertValuesTOString implementation dealing with inf values. Create a specialisation for float or double which will handle the infinity values in numerical limits.
Fix some operators when input and/or output is a shape tensors

Fix also in Gemm when broadcasting dynamic shape for the bias
When computing the last usage of tensors, the loop on the input tensors
of the added operator was not performed correctly. The loop was stopping if
an initialized tensor was an input
@lmoneta lmoneta force-pushed the tmva_sofie_fixes branch from d323fe0 to 86ad783 Compare May 18, 2026 08:12
Remove check to include standard library header only if within a list of allowed ones.
There is no need for that check and this was excluding addition of extra headers like chrono
@lmoneta lmoneta merged commit 63c2570 into root-project:master May 19, 2026
30 of 33 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