APrioriInvestments / typed_python

An llvm-based framework for generating and calling into high-performance native code from Python.
Apache License 2.0
198 stars 8 forks source link

NamedTuple should have a `replacing` method #114

Closed braxtonmckee closed 5 years ago

braxtonmckee commented 5 years ago

Create a keyword argument method replacing so that t.replacing(k1=v1, k2=v2, ...) returns a copy of the tuple with member 'k1' replaced with value 'v1', etc. This makes code that just wants to get a copy of a tuple with a single field updated much cleaner to read.

braxtonmckee commented 5 years ago

In order to implement this, you'll need to override typeMethodsConcrete in the PyNamedTupleInstance class to generate the new functions.

szymonlipinski commented 5 years ago

I have a question:

As the function typeMethodsConcrete is static and gets no arguments, and I need to generate a function for each passed member of the NamedTuple, is there a way to get the list of the passed field names to the NamedTuple constructor from the static function?

braxtonmckee commented 5 years ago

So right now, typeMethods gets called with a Type that's the actual type. We're dropping that when we call typeMethodsConcrete. We should be passing that through everywhere, so that the type methods can vary with the type. You should also cast it to 'py_instance_type::modeled_type'. Then the concrete function in PyNamedTuple will get a NamedTuple* and you can use that.

On Tue, Jun 25, 2019 at 5:00 PM Szymon Lipiński notifications@github.com wrote:

I have a question:

As the function typeMethodsConcrete is static and gets no arguments, and I need to generate a function for each passed member of the NamedTuple, is there a way to get the list of the passed field names to the NamedTuple constructor from the static function?

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/APrioriInvestments/nativepython/issues/114?email_source=notifications&email_token=AB6OHBB3XXARBBR7J4R2E7DP4KBQLA5CNFSM4H3CUD3KYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODYRST7Y#issuecomment-505620991, or mute the thread https://github.com/notifications/unsubscribe-auth/AB6OHBFELSRV3FUOAAXAVYDP4KBQLANCNFSM4H3CUD3A .

szymonlipinski commented 5 years ago

I have made a new commit in a branch https://github.com/APrioriInvestments/nativepython/commit/165a45c2cc384950f0ccf3226918bf362de5c4f5. I'm not really sure how to implement all the parts to fit properly.

braxtonmckee commented 5 years ago

I made some comments. Hopefully they give you enough to proceed.

On Wed, Jun 26, 2019 at 6:32 AM Szymon Lipiński notifications@github.com wrote:

I have made a new commit in a branch 165a45c https://github.com/APrioriInvestments/nativepython/commit/165a45c2cc384950f0ccf3226918bf362de5c4f5. I'm not really sure how to implement all the parts to fit properly.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/APrioriInvestments/nativepython/issues/114?email_source=notifications&email_token=AB6OHBCSEAIB5WXFCRMC6FDP4NASXA5CNFSM4H3CUD3KYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODYTC4PY#issuecomment-505818687, or mute the thread https://github.com/notifications/unsubscribe-auth/AB6OHBCEIZPAVZX5KME3ZC3P4NASXANCNFSM4H3CUD3A .

szymonlipinski commented 5 years ago

Thank you for your comments. I have added some more changes https://github.com/APrioriInvestments/nativepython/compare/szymon_add_tuple_methods. The main problem now is that the arguments passed to both constructor functions are wrong, so it doesn't compile now.

braxtonmckee commented 5 years ago

I made some additional comments directly on the commit in question.

On Wed, Jun 26, 2019 at 4:32 PM Szymon Lipiński notifications@github.com wrote:

Thank you for your comments. I have added some more changes https://github.com/APrioriInvestments/nativepython/compare/szymon_add_tuple_methods. The main problem now is that the arguments passed to both constructor functions are wrong, so it doesn't compile now.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/APrioriInvestments/nativepython/issues/114?email_source=notifications&email_token=AB6OHBD5SWIHHZE5CK4VEMLP4PG4JA5CNFSM4H3CUD3KYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODYUXLJA#issuecomment-506033572, or mute the thread https://github.com/notifications/unsubscribe-auth/AB6OHBCR3CTPPY6CE2Y727DP4PG4JANCNFSM4H3CUD3A .

szymonlipinski commented 5 years ago

Thank you. I have a couple of question while trying to understand the code:

  1. Is the eltPtr a pointer to an element in an internal list?
  2. In the Type class, the constructor function gets two arguments: (int64_t count, const ptr_func& ptrToChild) and then it loops from 0 to the count, passing to the second argument. How does it work?
  3. In your example, there is itemType->constructor(item_data, tupType->eltPtr(self->dataPtr(), index));, I'm wondering if the second part shouldn't be a function pointer, and I have no idea what the first argument should be then, as it looks like it should be a number, not a pointer (which in fact is a number, but iterating from 0 to a pointer value seems like not a good idea).
braxtonmckee commented 5 years ago

So in all cases, the 'itemPtr', 'dataPtr', etc. are pointers to the memory that contains an instance of some Type. Think of it like in C++ if you have a reference 'const T&' for some T, we store it as a T. For simplicity we just hold it as a char (element_ptr).

The type functions on the various classes manipulate these kinds of pointers. A type Type* t expects that a data pointer has t->bytecount() bytes of storage allocated at the relevant pointer. It can be in a state 'uninitialized' or 'initialized'. A type's base constructor function always takes a pointer to some uninitialized memory and default constructs an object there with the expectation that you will eventually call destructor on that same memory to tear it down.

eltPtr is a function that Types that are containers have. In this case, it takes a pointer to the data for the NamedTuple and produces a pointer to the data for the 'ix'th element of the named tuple.

The constructor function you're talking about (Type.hpp:279) is not the one I'm talking about. That's a convenience method that's designed to allow you to initialize many copies of the same type. For instance, if you had a type 'Type t' and you wanted to initialize 100 copies at memory locations with a base pointer `char pand a strides, you could write t->constructor(100, [&](int i) { return p + s * i; })and you would get an efficient implementation that doesn't need to do the type-dispatch for each step in the loop. But this is not the one you need to invoke here. Probably we should rename it toconstructMany` to avoid confusion.

The one you want is CompositeType.hpp:127', which looks like template void constructor(instance_ptr self, const sub_constructor& initializer)`. It accepts a pointer to the uninitialized storage for the NamedTuple and a callback function that will get called with a pointer and an index once for each item in the named tuple that has to get initialized. It's important that you have cast the Type to a NamedTuple or else the compiler won't be able to find the function.

On Thu, Jun 27, 2019 at 8:16 AM Szymon Lipiński notifications@github.com wrote:

Thank you. I have a couple of question while trying to understand the code:

  1. Is the eltPtr a pointer to an element in an internal list?
  2. In the Type class, the constructor function gets two arguments: (int64_t count, const ptr_func& ptrToChild) and then it loops from 0 to the count, passing to the second argument. How does it work?
  3. In your example, there is itemType->constructor(item_data, tupType->eltPtr(self->dataPtr(), index));, I'm wondering if the second part shouldn't be a function pointer, and I have no idea what the first argument should be then, as it looks like it should be a number, not a pointer (which in fact is a number, but iterating from 0 to a pointer value seems like not a good idea).

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/APrioriInvestments/nativepython/issues/114?email_source=notifications&email_token=AB6OHBDX3O4WUUA2ZSGKPK3P4SVQPA5CNFSM4H3CUD3KYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODYW5SVY#issuecomment-506321239, or mute the thread https://github.com/notifications/unsubscribe-auth/AB6OHBEZL5YTPJ3RIXZSL3TP4SVQPANCNFSM4H3CUD3A .

szymonlipinski commented 5 years ago

Thank you for the explanation. Could you check the last commit, especially these two lines:

szymonlipinski commented 5 years ago

https://github.com/APrioriInvestments/nativepython/commit/6202406c9936c2eecd55223e1f2d69d94e318dcb#diff-afd2454f2f0866ca45029783ab406a69R253

https://github.com/APrioriInvestments/nativepython/commit/6202406c9936c2eecd55223e1f2d69d94e318dcb#diff-afd2454f2f0866ca45029783ab406a69R290

szymonlipinski commented 5 years ago

Could you tell me how the code in this ticket is used later? It looks like it's wrapped by some python functions, and then imported to the python process as an extension. Am I right?

Why isn't there a normal Python tuple used?

braxtonmckee commented 5 years ago

Sure. A few points here. First, you can see a bunch of test uses of this code in types_test.py, which is where you should add tests for it yourself to verify you have done everything correctly. You should test the basic functionality, as well as checking that refcounts are correct by doing something like


N = NamedTuple(x=ListOf(int))
aList = ListOf(int)([1,2,3])

# _types is a member of the typed_python module
self.assertEqual(_types.refcount(aList), 1)
nt = N().replacing(x=aList)
self.assertEqual(nt.x, aList)
self.assertEqual(_types.refcount(aList), 2)
nt = None
self.assertEqual(_types.refcount(aList), 1)

to verify that the refcounts are all correct.

More generally, "why isn't there a normal Python tuple used":

The goal of typed_python is to give us the tools to express strong typing in our python programs. A "NamedTuple" in typedpython is deliberately more restrictive than a normal python tuple, which means it's easier to reason about. For instance, If you make the type ListOf(NamedTuple(x=int, y=str)) and you create an instance of that type, you know that the entire list itself has items of the correct type in it. There's no way to put a None in such a list (although if you want that behavior, you could write ListOf(OneOf(None, NamedTuple(x=int, y=str)))). If you have a bug where you try to put the wrong thing in the list, you get a bug when you insert the item (where the bug is), not where you take the item out of the list and use it.

The secondary goal of typed_python is to provide a set of datastructures with a strict memory layout that we can compile code against (this is the nativepython part of this project). A NamedTuple(x=int, y=float) is laid out exactly like std::pair<int, float>, and ListOf(NamedTuple(x=int, y=float)) is very similar to std::shared_ptr<std::vector<std::pair<int, float> > >. The code generated by nativepython can interact with these types directly, bypassing the python interpreter and using machine code, so that we can take portions of our programs and compile them, which can get 50x speedup per core, or more, and which bypasses the GIL, which in a multithreaded context can be an enormous advantage.

szymonlipinski commented 5 years ago

Thanks. Do you have an idea why this doesn't compile? https://github.com/APrioriInvestments/nativepython/blob/szymon_add_tuple_methods/typed_python/PyCompositeTypeInstance.cpp#L253

braxtonmckee commented 5 years ago

try itemType->copy_constructor

On Fri, Jun 28, 2019 at 1:09 PM Szymon Lipiński notifications@github.com wrote:

Thanks. Do you have an idea why this doesn't compile? https://github.com/APrioriInvestments/nativepython/blob/szymon_add_tuple_methods/typed_python/PyCompositeTypeInstance.cpp#L253

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/APrioriInvestments/nativepython/issues/114?email_source=notifications&email_token=AB6OHBAB6WZKK72UTJKFV63P4ZATPA5CNFSM4H3CUD3KYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODY2UVEA#issuecomment-506808976, or mute the thread https://github.com/notifications/unsubscribe-auth/AB6OHBBJL2WF2GRC5EO32CDP4ZATPANCNFSM4H3CUD3A .

szymonlipinski commented 5 years ago

There is a simple test for the function functionality.

However, with just METH_KEYWORDS, I get an error like this one https://bugs.python.org/issue11587.

With METH_VARARGS | METH_KEYWORDS I get

n2 = n1.replacing(a=2)
TypeError: Passed argument is not a dictionary.

What am I missing there?

braxtonmckee commented 5 years ago

I think you can't have just METH_KEYWORDS. It's an issue with the api. You should keep both VARARGS and KEYWORDS and throw an exception if there are any args.

On Fri, Jun 28, 2019 at 4:46 PM Szymon Lipiński notifications@github.com wrote:

There is a simple test for the function functionality.

However, with just METH_KEYWORDS, I get an error like this one https://bugs.python.org/issue11587.

With METH_VARARGS | METH_KEYWORDS I get

n2 = n1.replacing(a=2) TypeError: Passed argument is not a dictionary.

What am I missing there?

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/APrioriInvestments/nativepython/issues/114?email_source=notifications&email_token=AB6OHBDZP4G6Y6OVIX5MPXTP4Z2BVA5CNFSM4H3CUD3KYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODY3D65A#issuecomment-506871668, or mute the thread https://github.com/notifications/unsubscribe-auth/AB6OHBCRRG3WAG2ALV7ILCDP4Z2BVANCNFSM4H3CUD3A .

szymonlipinski commented 5 years ago

It looks like I have made this function with tests. The tests are testing the functionality, and generally it looks like all is fine.

Except for testing the refcounts. The problem is that if the last test runs separately, then it works. When it runs with all them, the result is a core dump.

Sat Jun 29 08:13:03 2019  |     failed in 0.0009438991546630859 seconds. See logs in nose.typed_python.types_test.NativeTypesTests.test_named_tuple_replacing_argument_errors.1606.log
Sat Jun 29 08:13:03 2019  |  Running test test_named_tuple_replacing_function (typed_python.types_test.NativeTypesTests)
Sat Jun 29 08:13:03 2019  |     passed in 0.00018024444580078125
Sat Jun 29 08:13:03 2019  |  Running test test_named_tuple_replacing_refcount (typed_python.types_test.NativeTypesTests)
[1]    1606 segmentation fault (core dumped)  ./test.py --filter typed_python.types_test.NativeTypesTests 
braxtonmckee commented 5 years ago

Ok probably something is getting freed and then re-incremented, so it looks OK but it’s actually acting on releases memory. Can you squash down to one commit and send me a PR and I’ll take a look

Sent from my iPhone

On Jun 29, 2019, at 8:21 AM, Szymon Lipiński notifications@github.com wrote:

It looks like I have made this function with tests. The tests are testing the functionality, and generally it looks like all is fine.

Except for testing the refcounts. The problem is that if the last test runs separately, then it works. When it runs with all them, the result is a core dump.

Sat Jun 29 08:13:03 2019 | failed in 0.0009438991546630859 seconds. See logs in nose.typed_python.types_test.NativeTypesTests.test_named_tuple_replacing_argument_errors.1606.log Sat Jun 29 08:13:03 2019 | Running test test_named_tuple_replacing_function (typed_python.types_test.NativeTypesTests) Sat Jun 29 08:13:03 2019 | passed in 0.00018024444580078125 Sat Jun 29 08:13:03 2019 | Running test test_named_tuple_replacing_refcount (typed_python.types_test.NativeTypesTests) [1] 1606 segmentation fault (core dumped) ./test.py --filter typed_python.types_test.NativeTypesTests — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub, or mute the thread.

szymonlipinski commented 5 years ago

It's here https://github.com/APrioriInvestments/nativepython/commit/00023fe7166c5618d0394def763cccf302e32c59.

I'm sure the problem is in my code. When I made a new branch with only the last test, then everything is fine. With all the tests running in the above commit, there is core dump. Travis thinks the same.

braxtonmckee commented 5 years ago

I hunk he problem is that you are not checking if the kwargs contains the key. So if you access it this way python throws an exception (returning null). You are expected to clear that exception but in this case you’ll just throw another one right after which can corrupt memory. If you clear the exception in the case the call returns null that will work but you probably ought to be checking if the doctor contains the string first as that’s much faster

Sent from my iPhone

On Jun 29, 2019, at 10:19 AM, Szymon Lipiński notifications@github.com wrote:

It's here 00023fe.

I'm sure the problem is in my code. When I made a new branch with only the last test, then everything is fine. With all the tests running in the above commit, there is core dump. Travis thinks the same.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub, or mute the thread.

szymonlipinski commented 5 years ago

I'm not so sure, converting the code to this also makes core dump


    // return a copy with updated values
    return PyInstance::initialize(tupType, [&](instance_ptr newInstanceData) {
        //newInstanceData will point to the uninitialized memory we've allocated for the new tuple

        tupType->constructor(newInstanceData, [&](instance_ptr item_data, int index) {
            //item_data is a pointer to the uninitialized value in the 'index'th slot in the new tuple
            NamedTuple *itemType = (NamedTuple*) tupType->getTypes()[index];
            std::string itemName = tupType->getNames()[index];

            PyObject* key = PyUnicode_FromString(itemName.c_str());

            if (PyDict_Contains(kwargs, key)) {
                // value passed in kwargs
                PyObject* value = PyDict_GetItemString(kwargs, itemName.c_str());

                PyInstance::copyConstructFromPythonInstance(
                        itemType,
                        item_data,
                        value
                );
            } else {
                //we don't have a replacement, so copy the existing value over.
                itemType->copy_constructor(item_data, tupType->eltPtr(self->dataPtr(), index));
            };
        });
    });```
braxtonmckee commented 5 years ago

well now that I look at the docs for PyDict_GetItem it's clear that it doesn't set an exception. So that was just a red herring.

I see the issue and I just pushed a fix. The problem was that we are casting 'itemType' to NamedTuple. This forces the itemType->copy_constructor call to dispatch to the wrong code. itemType is not actually a NamedTuple - it can be any type, because it's the set of types of the individual pieces of the NamedTuple we're replacing on (in the test, its ListOf(int)).

Let's write a few more tests, and then the next step will be to integrate this into the compiler.

szymonlipinski commented 5 years ago

Thank you. I have been trying to find out how this should be integrated into the compiler, and I'm not sure I understand how all the stuff works. At first, I thought it would just work, as this new function is returned in the typeMethodsConcrete. When you wrote about additional integrating it, I'm not sure I get it.

Could you describe me how the main compiler components work?

szymonlipinski commented 5 years ago

And I have managed to make another segfault :)

    def test_making_segfault_1(self):
        a = 'a'
        self.assertEqual(_types.refcount(a), 1)
braxtonmckee commented 5 years ago

looks like the format string "first argument to refcount %S not a permitted Type", is wrong. see documentation for PyErr_Format. Can you create a test and fix? Thx.

On Tue, Jul 2, 2019 at 8:21 AM Szymon Lipiński notifications@github.com wrote:

And I have managed to make another segfault :)

def test_making_segfault_1(self):
    a = 'a'
    self.assertEqual(_types.refcount(a), 1)

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/APrioriInvestments/nativepython/issues/114?email_source=notifications&email_token=AB6OHBDO7WCNHZEXKG576DLP5NB23A5CNFSM4H3CUD3KYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODZBCTMI#issuecomment-507652529, or mute the thread https://github.com/notifications/unsubscribe-auth/AB6OHBGIJRRJAEJ4VLO2U2TP5NB23ANCNFSM4H3CUD3A .

braxtonmckee commented 5 years ago

On the compiler point, see commit 25bca3cc825961fe4e94fd4cb2203c2b3268c0f3 for an example of changes that add a feature to the compiler. when I'm back we can sit down and go over this in more detail. But basically the idea is that you have a set of types called 'wrappers' that provide code-generation for other types. For instance, in this case there's a class NamedTupleWrapper that provides facilities for the compiler to implement operations on a given variable of typed NamedTuple. When we're generating code, we keep track of each variable in a frame by storing a 'wrapper' type for each one, and then each action that the compiler needs to process (say, an operator, or an attribute) gets translated into a call on the associated wrapper type that passes in 'TypedExpression' instances for each argument to the operation.

In your case, to implement this you'll need to

def convert_method_call(self, context, instance, methodname, args, kwargs):
    if methodname == 'replace' and not args:
        return context.push(self, lambda newInstance:
self.initializeWithReplace(context, newInstance, instance, kwargs))

def initializeWithReplace(self, context, toInitialize, existingInstance,
kwargs):
        # do some check that kwargs only contains valid items and if not
generate code to throw an exception
        ...
        # construct the new value
        for i in range(len(self.subTypeWrappers)):
            if self.typeRepresentation.ElementNames[i] not in kwargs:
                # initialize item i from the existing tuple
                self.refAs(context, toInitialize,
i).convert_copy_initialize(self.refAs(context, existingInstance, i))
            else:
                converted =
kwargs[self.typeRepresentation.ElementNames[i]].convert_to_type(self.typeRepresentation.ElementTypes[i])
                if converted is None:
                    # conversion functions return None when the code they
generated _always_ throws an exception in which case there
                    # is no reason to continue producing code.
                    return None
               self.refAs(context, toInitialize,
i).convert_copy_initialize(converted)

On Tue, Jul 2, 2019 at 2:54 PM Braxton McKee braxton.mckee@gmail.com wrote:

looks like the format string "first argument to refcount %S not a permitted Type", is wrong. see documentation for PyErr_Format. Can you create a test and fix? Thx.

On Tue, Jul 2, 2019 at 8:21 AM Szymon Lipiński notifications@github.com wrote:

And I have managed to make another segfault :)

def test_making_segfault_1(self):
    a = 'a'
    self.assertEqual(_types.refcount(a), 1)

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/APrioriInvestments/nativepython/issues/114?email_source=notifications&email_token=AB6OHBDO7WCNHZEXKG576DLP5NB23A5CNFSM4H3CUD3KYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODZBCTMI#issuecomment-507652529, or mute the thread https://github.com/notifications/unsubscribe-auth/AB6OHBGIJRRJAEJ4VLO2U2TP5NB23ANCNFSM4H3CUD3A .

szymonlipinski commented 5 years ago

Here is the fix for the refcount function https://github.com/APrioriInvestments/nativepython/pull/146

szymonlipinski commented 5 years ago

Thanks for the explanation. I have written some basic code, however, I have a problem with the compilation in the test, and I cannot find any fix.

https://github.com/APrioriInvestments/nativepython/compare/szymon_add_named_tuple_replacing?expand=1

AttributeError: type object 'Int64' has no attribute 'typeRepresentation'
  File "/usr/lib/python3.6/unittest/case.py", line 59, in testPartExecutor
    yield
  File "/usr/lib/python3.6/unittest/case.py", line 605, in run
    testMethod()
  File "apriori/nativepython/nativepython/tests/tuple_compilation_test.py", line 81, in test_named_tuple_replacing_function
    def f1(x: NT, a: int) -> NT:
  File "apriori/nativepython/nativepython/tests/tuple_compilation_test.py", line 23, in Compiled
    return Runtime.singleton().compile(f)
  File "apriori/nativepython/nativepython/runtime.py", line 94, in compile
    self.compile(o, argument_types)
  File "apriori/nativepython/nativepython/runtime.py", line 71, in compile
    callTarget = self.converter.convert(f.functionObj, input_wrappers, f.returnType, assertIsRoot=True)
  File "apriori/nativepython/nativepython/python_to_native_converter.py", line 357, in convert
    self._resolveAllInflightFunctions()
  File "apriori/nativepython/nativepython/python_to_native_converter.py", line 318, in _resolveAllInflightFunctions
    while self._resolveInflightOnePass():
  File "apriori/nativepython/nativepython/python_to_native_converter.py", line 301, in _resolveInflightOnePass
    nativeFunction, actual_output_type = functionConverter.convertToNativeFunction()
  File "apriori/nativepython/nativepython/function_conversion_context.py", line 58, in convertToNativeFunction
    body_native_expr, controlFlowReturns = self.convert_function_body(self._statements)
  File "apriori/nativepython/nativepython/function_conversion_context.py", line 456, in convert_function_body
    return self.convert_statement_list_ast(statements, toplevel=True)
  File "apriori/nativepython/nativepython/function_conversion_context.py", line 461, in convert_statement_list_ast
    expr, controlFlowReturns = self.convert_statement_ast(s)
  File "apriori/nativepython/nativepython/function_conversion_context.py", line 311, in convert_statement_ast
    e = subcontext.convert_expression_ast(ast.value)
  File "apriori/nativepython/nativepython/expression_conversion_context.py", line 639, in convert_expression_ast
    return lhs.convert_call(args, kwargs)
  File "apriori/nativepython/nativepython/typed_expression.py", line 123, in convert_call
    return self.expr_type.convert_call(self.context, self, args, kwargs)
  File "apriori/nativepython/nativepython/type_wrappers/bound_compiled_method_wrapper.py", line 54, in convert_call
    kwargs
  File "apriori/nativepython/nativepython/type_wrappers/tuple_wrapper.py", line 112, in convert_method_call
    return context.push(self, lambda newInstance: self.initializeReplacing(context, newInstance, instance, kwargs))
  File "apriori/nativepython/nativepython/expression_conversion_context.py", line 212, in push
    expr = callback(resExpr)
  File "apriori/nativepython/nativepython/type_wrappers/tuple_wrapper.py", line 112, in <lambda>
    return context.push(self, lambda newInstance: self.initializeReplacing(context, newInstance, instance, kwargs))
  File "apriori/nativepython/nativepython/type_wrappers/tuple_wrapper.py", line 138, in initializeReplacing
    converted = kwargs[field_name].convert_to_type(field_type)
  File "apriori/nativepython/nativepython/typed_expression.py", line 129, in convert_to_type
    return self.expr_type.convert_to_type(self.context, self, target_type)
  File "apriori/nativepython/nativepython/type_wrappers/arithmetic_wrapper.py", line 106, in convert_to_type
    if target_type.typeRepresentation == Float64:

>> output captured in nose.nativepython.tests.tuple_compilation_test.TestTupleCompilation.test_named_tuple_replacing_function.13815.log:

('a', 'b')
(<class 'Int64'>, <class 'String'>)
{'a': <nativepython.typed_expression.TypedExpression object at 0x7f260328f5f8>}
TypedExpression(Int64,[ref])
braxtonmckee commented 5 years ago

try rebasing on top of my branch brax-dev. I've been doing some work that will fix that problem.

On Wed, Jul 3, 2019 at 5:06 PM Szymon Lipiński notifications@github.com wrote:

Thanks for the explanation. I have written some basic code, however, I have a problem with the compilation in the test, and I cannot find any fix.

https://github.com/APrioriInvestments/nativepython/compare/szymon_add_named_tuple_replacing?expand=1

AttributeError: type object 'Int64' has no attribute 'typeRepresentation' File "/usr/lib/python3.6/unittest/case.py", line 59, in testPartExecutor yield File "/usr/lib/python3.6/unittest/case.py", line 605, in run testMethod() File "apriori/nativepython/nativepython/tests/tuple_compilation_test.py", line 81, in test_named_tuple_replacing_function def f1(x: NT, a: int) -> NT: File "apriori/nativepython/nativepython/tests/tuple_compilation_test.py", line 23, in Compiled return Runtime.singleton().compile(f) File "apriori/nativepython/nativepython/runtime.py", line 94, in compile self.compile(o, argument_types) File "apriori/nativepython/nativepython/runtime.py", line 71, in compile callTarget = self.converter.convert(f.functionObj, input_wrappers, f.returnType, assertIsRoot=True) File "apriori/nativepython/nativepython/python_to_native_converter.py", line 357, in convert self._resolveAllInflightFunctions() File "apriori/nativepython/nativepython/python_to_native_converter.py", line 318, in _resolveAllInflightFunctions while self._resolveInflightOnePass(): File "apriori/nativepython/nativepython/python_to_native_converter.py", line 301, in _resolveInflightOnePass nativeFunction, actual_output_type = functionConverter.convertToNativeFunction() File "apriori/nativepython/nativepython/function_conversion_context.py", line 58, in convertToNativeFunction body_native_expr, controlFlowReturns = self.convert_function_body(self._statements) File "apriori/nativepython/nativepython/function_conversion_context.py", line 456, in convert_function_body return self.convert_statement_list_ast(statements, toplevel=True) File "apriori/nativepython/nativepython/function_conversion_context.py", line 461, in convert_statement_list_ast expr, controlFlowReturns = self.convert_statement_ast(s) File "apriori/nativepython/nativepython/function_conversion_context.py", line 311, in convert_statement_ast e = subcontext.convert_expression_ast(ast.value) File "apriori/nativepython/nativepython/expression_conversion_context.py", line 639, in convert_expression_ast return lhs.convert_call(args, kwargs) File "apriori/nativepython/nativepython/typed_expression.py", line 123, in convert_call return self.expr_type.convert_call(self.context, self, args, kwargs) File "apriori/nativepython/nativepython/type_wrappers/bound_compiled_method_wrapper.py", line 54, in convert_call kwargs File "apriori/nativepython/nativepython/type_wrappers/tuple_wrapper.py", line 112, in convert_method_call return context.push(self, lambda newInstance: self.initializeReplacing(context, newInstance, instance, kwargs)) File "apriori/nativepython/nativepython/expression_conversion_context.py", line 212, in push expr = callback(resExpr) File "apriori/nativepython/nativepython/type_wrappers/tuple_wrapper.py", line 112, in return context.push(self, lambda newInstance: self.initializeReplacing(context, newInstance, instance, kwargs)) File "apriori/nativepython/nativepython/type_wrappers/tuple_wrapper.py", line 138, in initializeReplacing converted = kwargs[field_name].convert_to_type(field_type) File "apriori/nativepython/nativepython/typed_expression.py", line 129, in convert_to_type return self.expr_type.convert_to_type(self.context, self, target_type) File "apriori/nativepython/nativepython/type_wrappers/arithmetic_wrapper.py", line 106, in convert_to_type if target_type.typeRepresentation == Float64:

output captured in nose.nativepython.tests.tuple_compilation_test.TestTupleCompilation.test_named_tuple_replacing_function.13815.log:

('a', 'b') (<class 'Int64'>, <class 'String'>) {'a': <nativepython.typed_expression.TypedExpression object at 0x7f260328f5f8>} TypedExpression(Int64,[ref])

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/APrioriInvestments/nativepython/issues/114?email_source=notifications&email_token=AB6OHBFHCDCG7BJVOYJG7DTP5UIGRA5CNFSM4H3CUD3KYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODZFVW7Y#issuecomment-508255103, or mute the thread https://github.com/notifications/unsubscribe-auth/AB6OHBCGRUMB4CNABXQGI73P5UIGRANCNFSM4H3CUD3A .

szymonlipinski commented 5 years ago

Thank you. It seems like after the rabase maybe I have the integration done, the tests are passing. The main problem now is in some other place in the code https://travis-ci.com/APrioriInvestments/nativepython/builds/117948812#L1703 I'm trying to fix it.

szymonlipinski commented 5 years ago

It looks like this always returns None.

The function has been called with the arguments from the Server::_removeOld

fieldId = self._currentTypeMap().fieldIdFor("core", "Connection", " exists")
TypeMap:
    def fieldIdFor(self, schema, typename, fieldname):
        key = FieldDefinition(schema=schema, typename=typename, fieldname=fieldname)
        return self.fieldDefToId.get(key)

However, when I placed this inside:

print('---')
print(self.fieldDefToId.get(key))
print(key == FieldDefinition(schema=schema, typename=typename, fieldname=fieldname))
print(self.fieldDefToId)
print('---')

then I got this:

---
None
True
{(schema="core", typename="Connection", fieldname=" exists"): 0, (schema="test_schema", typename="Counter", fieldname=" exists"): 1, [I've removed the rest]`
---

I cannot figure out why the kyes() method called on the Dict object returns whole dictionary, not just the keys.

braxtonmckee commented 5 years ago

Try rebasing on latest. I had a bug associated with hashing that broke some tests. Should be clean on ‘dev’

Sent from my iPhone

On Jul 5, 2019, at 4:49 AM, Szymon Lipiński notifications@github.com wrote:

It looks like this always returns None.

The function has been called with the arguments from the Server::_removeOld

fieldId = self._currentTypeMap().fieldIdFor("core", "Connection", " exists") TypeMap: def fieldIdFor(self, schema, typename, fieldname): key = FieldDefinition(schema=schema, typename=typename, fieldname=fieldname) return self.fieldDefToId.get(key) However, when I placed this inside:

print('---') print(self.fieldDefToId.get(key)) print(key == FieldDefinition(schema=schema, typename=typename, fieldname=fieldname)) print(self.fieldDefToId) print('---') then I got this:


None True {(schema="core", typename="Connection", fieldname=" exists"): 0, (schema="test_schema", typename="Counter", fieldname=" exists"): 1, [I've removed the rest]`

I cannot figure out why the kyes() method called on the Dict object returns whole dictionary, not just the keys.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub, or mute the thread.