wlav / cppyy

Other
387 stars 39 forks source link

updating the type mapping+resolution system for numba extended functions #147

Open maximusron opened 1 year ago

maximusron commented 1 year ago

I stumbled upon an odd bug that led me to try and fix the inherently flawed type mapping as well as overload call for multiple arguments in numba_ext.py

The cpp2numba dictionary is perfect since it does exhibit a one-to-one mapping for cpp types and numba types. While doing traces for adding support for const char* arguments, I realized that the primary problem

I though this shortcoming only affected this odd case where I can't add a duplicate nb_type for const char*.

However while continuing to work on trying to figure out ideas to improve the type resolution for the TemplateProxy object, I received a long long signature mismatch error despite using an int64_t type in my C++ definition.

Running this function:

       def int64_test():
        cppyy.cppdef("""
        int64_t add(int64_t a) {
            return a+a;
        }
        """)

        @numba.jit(nopython = True)
        def run_add(a):
            return cppyy.gbl.add(a)

        print(run_add(5))

Would give me the following errors:

/usr/bin/python3.8 /home/maximus/cppyy-dev/cppyy/numba_trace/numba_ext_experiments.py 
numba.core.errors.TypingError: Failed in nopython mode pipeline (step: nopython frontend)
Internal error at <numba.core.typeinfer.CallConstraint object at 0x7ff7ce5f0cd0>.
signature "long long" not found
During: resolving callee type: CppFunction(<C++ overload "add" at 0x7ff7ce585e80>)
During: typing of call at /home/maximus/cppyy-dev/cppyy/numba_trace/print_test.py (20)

This is similar to the error obtained by @sudo-panda in this issue https://github.com/wlav/cppyy/issues/84 (have not examined the Literal case yet though)

I realized that despite having an apparently clean mapping(that is expected to handle such a trivial function) for int64_t in cpp2numba: https://github.com/wlav/cppyy/blob/340e8330579f528f8f8e31de29a0ba51cf4d3d9e/python/cppyy/numba_ext.py#L51 the nb_types are not unique as they would seem. On resolving nb_types., it turns out this is the actual mapping:

*_nb_types. <-> cpp type_**

none : void
void* : void*
int8 : int8_t
uint8 : uint8_t
int16 : short
uint16 : unsigned short
int32 : int
uint32 : unsigned int
int32 : int32_t
uint32 : uint32_t
int64 : int64_t
uint64 : uint64_t
int64 : long
uint64 : unsigned long
int64 : long long
uint64 : unsigned long long
float32 : float
float64 : double

Where we can observe the repetition of int64 multiple times. In this case three different nb_types.* are mapped to the same thing:

 nb_types.int64     =  int64    <->    int64_t         
 nb_types.long_     =  int64    <->    unsigned long   
 nb_types.longlong  =  int64    <->    long long

This means the actual value in _numba2cpp actually depends on the order in which the cpp2numba maps to an equivalent nb_types.* type, which just should not happen.

In this case, final _numba2cpp dictionary being generated actually has the last mapping to a nb_types,* that resolves to a int64: https://github.com/wlav/cppyy/blob/340e8330579f528f8f8e31de29a0ba51cf4d3d9e/python/cppyy/numba_ext.py#L55 overwrites the previous mappings, and ends up being the only cpp type that the numba inferred type (int64) is mapped to.

My solution was to modify the _numba2cpp dictionary to contain a one-to-many mapping:

_numba2cpp:

none : ['void']
void* : ['void*']
int8 : ['int8_t']
uint8 : ['uint8_t']
int16 : ['short']
uint16 : ['unsigned short']
int32 : ['int', 'int32_t']
uint32 : ['unsigned int', 'uint32_t']
int64 : ['int64_t', 'long', 'long long']
uint64 : ['uint64_t', 'unsigned long', 'unsigned long long']
float32 : ['float']
float64 : ['double']
unicode_type : ['const char*']

Since the signature has to match one of the signatures from CPyCppyy::CPPOverload::FindOverload while calling the __overload__method of the cppyy.CppOverload object(or cppyy.TemplateProxy) while creating the instance of the CppFunctionNumbaType, I generated a set of possible combinations that would replace the ol = CppFunctionNumbaType(self._func.__overload__(*(numba2cpp(x) for x in args)), self._is_method) call.

I did run into some issues that exposed the format of the current pass to overload being supported for only a single argument.

    def multi_args_test():
        cppyy.cppdef("""
                void print_str(const char *a, const char *b, const char *c) {
                    printf(\"Received: %s %s \\n\", a, b, c);
                }
                """)

        @numba.njit()
        def print_cpp_str(msg1, msg2, msg3):
            cppyy.gbl.print_str(msg1, msg2, msg3)

        print_cpp_str("Hello", "World", "!!")

For CPyCppyy to obtain the PyString in a way that matches the PyText as a string from the GetSignature method when the method proxy overload is called in CPPOverload.cxx, the following formatting was done: signature = ", ".join(arg_combo)

This prevented the errors of this type: __overload__() takes at most 2 arguments (3 given)

The result is this:

Numba typeinfer in dispatcher: int64
_func is a  <class 'cppyy.CPPOverload'>
get_call_type args: (int64,)
reflex return type before creating overload: long
ARGS: (int64,)
ARG COMBO ('int64_t',)
CPyCppyy checking __overload__ signature:(int64_t)
Matched CPyCppyy Signature 2:(int64_t)
ARG COMBO ('long',)
CPyCppyy checking __overload__ signature:(long)
ARG COMBO ('long long',)
CPyCppyy checking __overload__ signature:(longlong)
function reflex return type:  int64

Obtaining the function __overload__ in get_pointer:
ARG COMBO ('int64_t',)
CPyCppyy checking __overload__ signature:(int64_t)
Matched CPyCppyy Signature 2:(int64_t)
ARG COMBO ('long',)
CPyCppyy checking __overload__ signature:(long)
ARG COMBO ('long long',)
CPyCppyy checking __overload__ signature:(longlong)
Succesful arg combo match= ('int64_t',) 

15 + 15 = 30

Process finished with exit code 0

This now allows the support of functions with such arguments, as well as extends it to work with n number of possible argument sets like so:

cppyy.cppdef("""
                void print_str(const char *a, long b, unsigned long c) {
                    printf(\"Received: %s %ld %lu \\n\", a, b, c);
                }
                """)

        @numba.njit()
        def print_cpp_str(msg1, a, b):
            cppyy.gbl.print_str(msg1, a, b)

which provides this set matching, where the correct C++ signature is in the last line of this block:

ARGS: (unicode_type, int64, uint64)
ARG COMBO ('const char*', 'int64_t', 'uint64_t')
CPyCppyy checking __overload__ signature:(('constchar*','int64_t','uint64_t'))
ARG COMBO ('const char*', 'int64_t', 'unsigned long')
CPyCppyy checking __overload__ signature:(('constchar*','int64_t','unsignedlong'))
ARG COMBO ('const char*', 'int64_t', 'unsigned long long')
CPyCppyy checking __overload__ signature:(('constchar*','int64_t','unsignedlonglong'))
ARG COMBO ('const char*', 'long', 'uint64_t')
CPyCppyy checking __overload__ signature:(('constchar*','long','uint64_t'))
ARG COMBO ('const char*', 'long', 'unsigned long')
CPyCppyy checking __overload__ signature:(('constchar*','long','unsignedlong'))

And even:

 def int64_sum_test():
        cppyy.cppdef("""
        int64_t int64_adder(int64_t a, int64_t b, const char *c) {
            printf("%s \\n", c);
            return a+b;
        }
        """)

        @numba.njit()
        def run_add(a1, a2, msg):
            k = cppyy.gbl.int64_adder(a1, a2, msg)
            return k

        x = 15
        y = 20
        msg = "cppyy rocks"
        print(x, "+", y, "=", run_add(y, x, msg))

with output:

Numba typeinfer in dispatcher: int64
Numba typeinfer in dispatcher: int64
Numba typeinfer in dispatcher: unicode_type
_func is a  <class 'cppyy.CPPOverload'>
get_call_type args: (int64, int64, unicode_type)
reflex return type before creating overload: long
ARGS: (int64, int64, unicode_type)
ARG COMBO ('int64_t', 'int64_t', 'const char*')
CPyCppyy checking __overload__ signature:(int64_t,int64_t,constchar*)
Matched CPyCppyy Signature 2:(int64_t,int64_t,constchar*)
function reflex return type:  int64

Obtaining the function __overload__ in get_pointer:
ARG COMBO ('int64_t', 'int64_t', 'const char*')
CPyCppyy checking __overload__ signature:(int64_t,int64_t,constchar*)
Matched CPyCppyy Signature 2:(int64_t,int64_t,constchar*)
Succesful arg combo match= ('int64_t', 'int64_t', 'const char*') 

cppyy rocks 
15 + 20 = 35

note: the earlier outputs did not break out of the loop while testing ol = CppFunctionNumbaType(self._func.__overload__(signature), self._is_method), the above output breaks out once the CPyCppyy signature match is found.

This even adds support for multi argument template functions that did not work earlier, since after the initial CppFunctionNumbaType call with the TemplateProxy object, the call is the same with the CPPOverload object.

Function:

        cppyy.cppdef("""
            namespace NumbaSupportExample {
            template<typename T1>
            T1 add(T1 a, T1 b) { return a + b; }
            }""")

which runs perfectly. I am not entirely sure if I am making any silly mistakes while going down this rabbit hole but I was motivated by the fact that these functions(especially the template one) are running successfully. would love to know how to proceed ^_^

wlav commented 1 year ago

The mapping should be even more general: any implicit cast should be allowed. Baidyanath started this, but then other distractions popped up (conferences, a paper, and more work on InterOp). Pull requests are here:

https://github.com/wlav/CPyCppyy/pull/4 https://github.com/wlav/cppyy-backend/pull/5

More generally, the work done to provide cppyy on top of libInterOp instead of on top of Cling will simplify the above as it won't be based on string comparisons, but decl comparisons. I.e., the implementation provided by Baidyanath may be obsolete soon, but the general approach of finding compatible types (as opposed to iterating all possible types and typedefs) is the way to go.

NiKeYiGuN commented 1 year ago

Does cppyy support C++ template functions?

wlav commented 1 year ago

Does cppyy support C++ template functions?

Yes.