Skip to content

Conversation

guitargeek
Copy link
Contributor

In some cases, it might happen that a CallFunc wrapper for the RooAbsReal or RooAbsPdf classes is created, which gives compile errors because these classes are virtual.

It is however not a big deal to make these classes non-virtual and avoid these errors. The RooAbsArg::clone() method should have been overridden anyway since RooAbsReal and RooAbsPdf already have copy constructors. The only other method that needs to be implemented is RooAbsReal::evaluate(), which is done as well. The implementation throws an exception, because evaluate() should always be called from derived classes.

This is a bit of a hotfix and one can argue that it would be nicer to first understand why these CallFunc wrappers are even created, but this issue needs to be resolved quickly and with minimal changes: the fix also needs to be backported to older ROOT versions and is crucial for the ATLAS Run 2 Higgs combination analysis.

Issue reported on the ROOT forum:
https://root-forum.cern.ch/t/cannot-open-a-rooworkspace-once-it-is-saved-allocating-an-object-of-class-type-error/63954

In some cases, it might happen that a CallFunc wrapper for the
`RooAbsReal` or `RooAbsPdf` classes is created, which gives compile
errors because these classes are virtual.

It is however not a big deal to make these classes non-virtual and avoid
these errors. The `RooAbsArg::clone()` method should have been
overridden anyway since `RooAbsReal` and `RooAbsPdf` already have copy
constructors. The only other method that needs to be implemented is
`RooAbsReal::evaluate()`, which is done as well. The implementation
throws an exception, because `evaluate()` should always be called from
derived classes.

This is a bit of a hotfix and one can argue that it would be nicer to
first understand why these CallFunc wrappers are even created, but this
issue needs to be resolved quickly and with minimal changes: the fix
also needs to be backported to older ROOT versions and is crucial for
the ATLAS Run 2 Higgs combination analysis.
@guitargeek guitargeek self-assigned this Jul 31, 2025
@guitargeek guitargeek requested a review from lmoneta as a code owner July 31, 2025 14:50
@dpiparo dpiparo self-requested a review July 31, 2025 15:33
Copy link
Member

@dpiparo dpiparo left a comment

Choose a reason for hiding this comment

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

please go ahead with the hotfix, and let's not forget that we may want to revisit the reason why wrappers are generated in the first place.

Copy link

Test Results

    21 files      21 suites   3d 7h 10m 56s ⏱️
 3 220 tests  3 220 ✅ 0 💤 0 ❌
65 843 runs  65 843 ✅ 0 💤 0 ❌

Results for commit 4a490f2.

@pcanal
Copy link
Member

pcanal commented Jul 31, 2025

Was a ticket opened for the underlying issue?

@guitargeek
Copy link
Contributor Author

Not yet. I'm working on creating a minimal reproducer together with Matthew from ATLAS who posted in the forum. But I'll open the issue before merging this PR

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.

3 participants