TypeError: no python-side overrides supported (failed to compile the dispatcher code)

Hi @jprochaz,

Welcome back to the ROOT forum! Thanks for the report.

@vpadulan have you seen this before? If not, I could try debugging the coming week. In particular, the following error is particularly suspicious:

Cheers,
J.

I have reproduced the issue in ROOT master.

The problem seems to be related to the const-ness mismatch in the returned object (note the missing const in the first occurrence.

I have tracked down the introduction of the Fitter::ObjFunction() to commit 9d322a6a38fca106d0e5b6064bd09c8c6dbb979a. @moneta any ideas on a possible fix?
Should I create a GitHub issue for this, @moneta?

Cheers,
J.

Hi @jalopezg

I have no idea how to fix this. I don’t see any problem or const-ness mismatch in the Fitter::ObjFunction that has been introduced for 6.28.
What is the problem you see ?

Lorenzo

The most up to date ROOT tag/branch which works for both py3.10 and py3.11 is v6-26-00-patches.

ROOT v6-26-10 works with py3.10 but not with py3.11 due to Fail to include "code.h" again v6.26/{08,10} again

The topic was closed 14 days after the last reply - without being solved…

@jalopezg any progress?

Hi @jprochaz,

No, sorry, I pointed out above at the commit which introduced the changes that trigger the problem, but I could not get to investigate further.
I have created a GitHub issue to track this problem here: [PyROOT] TypeError: no python-side overrides supported (failed to compile the dispatcher code) · Issue #12391 · root-project/root · GitHub. Help us to prioritize this issue: is this still blocking you?

Cheers,
J.

Hi @jalopezg,

thanks for the update! Yes, it is still blocking me.

Cheers,
Jiri

Hi!

Just my 2 cents:

This question is similar to one that was asked about RDataFrame here:

In that thread, I explained how it’s a bad idea to inherit from a class in Python that is not meant to be inherited from in C++ and what the alternatives are (composition over inheritance, dynamically patch the class in Python. If you inherit from the Fitter like this, I’m 95 % sure you will run into other problems eventually because the Fitter class has not virtual functions, so I’m not sure how you expect overriding to work.

So in my opinion, the error that you get in ROOT 6.28 is a feature and not a bug, because it prevents users from writing unstable code.

Why are you depending of inheriting from ROOT Fitter? Maybe we can suggest you an alternative!

Cheers,
Jonas

The error is caused by a typedef not understood in PyROOT, not sure why.
In [math] Add virtual destructor to Fitter and fix an issue for PyROOT by lmoneta · Pull Request #12397 · root-project/root · GitHub the correct type is used and the code above works.

There could be cases where one wants to extend the Fitter class, and since this is not performance critical, so I think a virtual destructor can be added to the Fitter. The missing virtual destructor was causing just a a warning and not the error.

Lorenzo

Okay, thanks for the clarification! If the reason for the actual error was the typedef, then indeed it doesn’t make a practical difference if the destructor is virtual or not.

Many thanks for looking into it!

@moneta
The fix should probably solve the problem as I do not observed similar problem for other ROOT classes and the error occurred with introduction of Fitter::ObjFunction (it will be easier for me to test it on my side once it is merged in main/master branch).

@jonas
Thanks for your explanations and suggestions. It helped me to better understand several things. I noticed the warning concerning no virtual destructor (RDataFrame has no virtual destructor) but I did not know what it means (I have never observed a problem in practice).

It looks to me that in ROOT c++ methods are often virtual only if they must be implemented by user. Therefore, there are far less reasons for inheritance than they could be. It means that there are far less practical options to experiment with changed behavior of a class in ROOT already on c++ side (composition of classes does not help much here). Python provides more flexibility but, indeed, one should be aware that there might be some unforeseen effects.

There are cases where the mentioned composition of classes is indeed better option than inheritance.
The RDataFrame example (RDataFrame has no virtual destructor) is an example where one would like to only add on python side a member function to a class independently whether the C++ class has a virtual method (the same can be done in C++). Inheritance may have the advantage that it may not be necessary to modify a code base to accept the derived class (or only types need to be changes when mypy is used). Composition typically requires more changes to adapt the code base as it is necessary to distinguish between the wrapper and wrapped class. Even if a c++ non-virtual method cannot be overwritten on python side using inheritance, it can be shadowed and this can be used for intended side effects at least on python side (the parent method can be called within the python child method).

I noticed the warning concerning no virtual destructor (RDataFrame has no virtual destructor) but I did not know what it means (I have never observed a problem in practice).

The problem is the same as in pure C++: the Python instance has state, thus if the destructor is not virtual and the instance is deleted C++ side through the base class pointer (the only type known to C++), then the Python side state will not be cleaned up. If Python owns and deletes the instances in all cases, you’d be good. Otherwise, there’s a memory leak.

(There’s also a more esoteric problem in that auto-casting can’t work w/o virtual destructor b/c the object will not have the expected layout to find RTTI. This is only an issue if you send the instance from Python into C++ and back to Python, as identity will be lost, which in turn could lead to memory access problems.)

Hi @wlav

Thanks a lot for the clear explanation. It makes sense then to have a virtual destructor in C++ and it is now included in the master (see [math] Add virtual destructor to Fitter and fix an issue for PyROOT by lmoneta · Pull Request #12397 · root-project/root · GitHub).
However, I don’t understand why we were getting that error when using a typedef for a return object of a member function. Any idea why this was a problem for PyROOT ?

Cheers

Lorenzo

@moneta I can confirm that the fix in master works for me.

@wlav Thanks for the explanations and useful comments.

Probably something deep inside ROOT/meta which, just like the cppyy version used in PyROOT, is ancient and behind on functionality. What, precisely, I don’t know, but things are fine in cppyy master.

For background to fix this in ROOT/meta, the following is happening: cppyy redeclares all protected methods as public, so that they can be available in the Python derived class. It redeclares, rather than using, to deal with specific overloads.

In ROOT, the redeclaration done for the dispatcher class looks like this:

  ROOT::Math::IBaseFunctionMultiDimTempl<double>* ObjFunction() const {
    return Fitter::ObjFunction();
  }

whereas the original function is this:

   const ROOT::Math::IMultiGenFunction * ObjFunction() const {
      return (fExtObjFunction) ? fExtObjFunction : fObjFunction.get();
   }

As you can see, there’s a const lost for the return type.

The proximate cause is not the typedef, but these two using:

      template<class T>
      using IMultiGenFunctionTempl = IBaseFunctionMultiDimTempl<T>;
      using IMultiGenFunction = IMultiGenFunctionTempl<double>;

which seems to confuse ROOT/meta and makes it eat the const. Pretty much any other combination (incl. different using and typedef) is fine, though.

Here’s a condensed, C++ only, reproducer:

template<class T>
class A;

template<class T>
using A_T = A<T>;
using A_d = A_T<double>;

class B {
public:
    virtual ~B() {}

protected:
    const A_d* ObjFunction() const { return nullptr; }
};

void rf() {
    TClass* c = TClass::GetClass("B");
    TFunction* f = c->GetMethodAny("ObjFunction");

    std::cerr << "correct: " << f->GetReturnTypeName() << std::endl;
    std::cerr << "wrong:   " << f->GetReturnTypeNormalizedName() << std::endl;
}

Which results in:

$ root -l -b -q rf.C
Processing rf.C...
correct: const A_d*
wrong:   A<double>*

clearly showing the const being lost.

Aside, @moneta , looking through the issues for GetReturnTypeNormalizedName(), I find [PyROOT] Cannot use SofieFunctor in Python · Issue #10684 · root-project/root · GitHub, which you submitted and is showing another problem with ROOT’s implementation of it. It, too, is working fine in cppyy master.

Hi @wlaw,
Thank you for your very useful insights. This can be useful for fixing ROOT meta for the lost of the const, maybe @axel can look into this.
We should then also look to update cppyy to the master version.

Cheers

Lorenzo

Thanks @wlav , indeed super useful!

That’s (as you know, @moneta ) in the books for 2024 and later - if we get the resources we’re hoping for. Not this year, though :-/

Cheers, Axel

This topic was automatically closed after 12 days. New replies are no longer allowed.