Skip to content

Conversation

@guitargeek
Copy link
Contributor

@guitargeek guitargeek commented Feb 6, 2026

TMVA SOFIE development is challenging sometimes, because of how the tests are structured.

The tests that covers many possible models imported from ONNX or ROOT have the issue that they includes all emitted code in the compiled executables. This means that one gets a build failure on the first model that generated invalid code, and that was it. Therefore, it's difficult to debug what is going wrong.

This commit suggests include the generated code with the interpreter instead. Then, one can check for each individual model if the code was valid, and if not, skip over to the next test a print the emitted code that failed to compile.

It has some performance overhead, but the tests still only take about 6 seconds. The drastically improved debugging experience justifies these few extra seconds spent on testing. But it total, testing is still faster, because compilation of
the test executable takes about 12 seconds, and now it doesn't need to be rebuilt at test time.

This was motivated by the effort to refactor the SOFIE-emitted code to make it differentiable with Clad.

@guitargeek guitargeek self-assigned this Feb 6, 2026
@guitargeek guitargeek force-pushed the sofie_tests branch 2 times, most recently from cfba928 to a47de91 Compare February 7, 2026 00:38
@github-actions
Copy link

github-actions bot commented Feb 7, 2026

Test Results

    22 files      22 suites   3d 15h 35m 27s ⏱️
 3 777 tests  3 777 ✅ 0 💤 0 ❌
76 017 runs  76 017 ✅ 0 💤 0 ❌

Results for commit 2f82ccb.

♻️ This comment has been updated with latest results.

TMVA SOFIE development is challenging sometimes, because of how the
tests are structured.

The tests that covers many possible models imported from ONNX or ROOT
have the issue that they includes **all** emitted code in the
compiled executables. This means that one gets a build failure on the
first model that generated invalid code, and that was it. Therefore,
it's difficult to debug what is going wrong.

This commit suggests include the generated code with the interpreter
instead. Then, one can check for each individual model if the code was
valid, and if not, skip over to the next test a print the emitted code
that failed to compile.

It has some performance overhead, but the tests still only take about 6
seconds. The drastically improved debugging experience justifies these
few extra seconds spent on testing.

This was motivated by the effort to refactor the SOFIE-emitted code to
make it differentiable with Clad.
Copy link
Member

@lmoneta lmoneta left a comment

Choose a reason for hiding this comment

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

Hi,
Thank you Jonas for this PR. I agree this is an improvement in the testing, allowing for better flexibilility and debugging. It is something I wanted to do, but did not find the time.
The only doubt is that we have a class designed for parsong and compiling a model on the flay and then execute the inference (RSofieReader). Maybe to avoid some code duplication at some point we could refactor the tests to use the RSofieReader class, but this can be done later.

@guitargeek
Copy link
Contributor Author

Thanks for the review! Good point with the RSofieReader 👍 That would be a good followup, but from a practical point of view I'm happy with how it is right now for the development that I want to do.

@guitargeek guitargeek merged commit 9ccd0f3 into root-project:master Feb 9, 2026
71 of 76 checks passed
@guitargeek guitargeek deleted the sofie_tests branch February 9, 2026 10:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants