Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[PyROOT] Example with inheriting from ROOT.Math.IMultiGenFunction doesn't work after recent cppyy upgrade #15315

Closed
guitargeek opened this issue Apr 23, 2024 · 1 comment · Fixed by #16197

Comments

@guitargeek
Copy link
Contributor

guitargeek commented Apr 23, 2024

Description

One example from the ROOT manual doesn't work anymore with ROOT 6.32:
https://root.cern/manual/python/#alternative-for-tpymultigenfunction-and-tpymultigradfunction

import ROOT

from array import array

class MyMultiGenFCN( ROOT.Math.IMultiGenFunction ):
    def NDim( self ):
        return 1

    def DoEval( self, x ):
        return (x[0] - 42) * (x[0] - 42)

    def Clone( self ):
        x = MyMultiGenFCN()
        ROOT.SetOwnership(x, False)
        return x

def main():
    fitter = ROOT.Fit.Fitter()
    myMultiGenFCN = MyMultiGenFCN()
    params = array('d', [1.])
    fitter.FitFCN(myMultiGenFCN, params)
    fitter.Result().Print(ROOT.std.cout, True)

if __name__ == '__main__':
    main()

The error is:

input_line_33:38:23: error: expected '(' for function-style cast or type construction
      return unsigned int{};
             ~~~~~~~~ ^
Traceback (most recent call last):
  File "/home/rembserj/example.py", line 5, in <module>
    class MyMultiGenFCN( ROOT.Math.IMultiGenFunction ):
TypeError: no python-side overrides supported (failed to compile the dispatcher code)

See:

@aaronj0
Copy link
Collaborator

aaronj0 commented Aug 8, 2024

For the IMultiGenFunction crossinheritance I see that we inject theNDim() method into the dispatched code:

    PyObject* iself = (PyObject*)_internal_self;
    if (!iself || iself == Py_None) {
      PyErr_Warn(PyExc_RuntimeWarning, (char*)"Call attempted on deleted python-side proxy");
      return unsigned int{};
    }

using this code block:

// possible crash
code << " PyObject* iself = (PyObject*)_internal_self;\n"
" if (!iself || iself == Py_None) {\n"
" PyErr_Warn(PyExc_RuntimeWarning, (char*)\"Call attempted on deleted python-side proxy\");\n"
" return";
if (retType != "void") {
if (retType.back() != '*')
code << " " << CPyCppyy::TypeManip::remove_const(retType) << "{}";
else
code << " nullptr";
}

Cling fails to compile the initialization in return unsigned int{}; since it is not valid C++. We should also consider unsigned int and other multi keyword types that require a function style cast for value initialization (like return (unsigned int){}. If I am not wrong, this should just work for all primitives(as zero initialization) and user-defined types(as default initialization).

I was able to get this example from the manual to work with this fix. Based on the issue, I was of the opinion we should land this upstream but I see that this is a high priority issue and @wlav is on vacation.

What do you think @guitargeek @dpiparo

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
No open projects
Status: Done
Development

Successfully merging a pull request may close this issue.

3 participants