Mailman 3 May 2011 - cython-devel

Re: [Cython] Specializing str methods?
by Stefan Behnel May 19, 2011

May 19, 2011

Hi, the right place to discuss this is the cython-devel mailing list. John Ehresman, 19.05.2011 18:17: > On 4/13/11 11:23 AM, Stefan Behnel wrote: >> You can add BuiltinMethod entries for the "str" type in Builtin.py and >> map them to a function name like "__Pyx_Str_PyString_WhatEver()". Then, >> define "utility_code" code blocks for each of the methods that #defines >> that name to the appropriate PyString_*() or PyUnicode_*() C-API >> function, depending on the Python version it runs with. There are a lot >> of examples for that in Builtin.py already. > > I've finally got back to this and have the start of an implementation. I've > attached a diff, though I'm not sure what the preferred workflow is. Should > I push to github and / or create a bug in trac? It's best to create a pull request on github, potentially accompanied by a request for review on the developer mailing list. We get a notification about pull requests, but it's still better to give us an idea in what state you think your patch is and what kind of review you need. The review can then happen by commenting on the changes directly in github. > I'll probably add more > methods, but wanted to get feedback before doing so. Certainly the right decision. > I also intend to contribute other patches in the future. Please do. :) > One thing that I didn't expect is that I originally declared the method as > BuiltinMethod("startswith", "TT", "O", "_Pyx_StrStartsWith", > utility_code=str_startswith_utility_code), > and expected the specialization to only be used when both self and arg > where a str, but instead cython always used the specialization and added a > check for arg being a str before the specialization was called. I ended up > writing a function that works for both unicode & bytes self and an object arg. You don't really have to care about the type of the argument when applying this optimisation. As long as it's clear that the object that provides the method is a "str" (although it's potentially None even then!), it's clear what the method will do when being called, even if we don't know the argument type at compile time. Just provide a fallback, as you did anyway. I'll comment on your patch below. diff --git a/Cython/Compiler/Builtin.py b/Cython/Compiler/Builtin.py +str_startswith_utility_code = UtilityCode( +proto=""" "proto" is for the forward declaration of the function signature. Except for very simple cases (short macros) the implementation goes into "impl". +static PyObject* _Pyx_StrStartsWith(PyObject* self, PyObject* arg) +{ + PyObject* ret; + + if ( PyBytes_CheckExact(self) && PyBytes_CheckExact(arg) ) { + if ( PyBytes_GET_SIZE(self) < PyBytes_GET_SIZE(arg) ) + ret = Py_False; + else if ( memcmp(PyBytes_AS_STRING(self), PyBytes_AS_STRING(arg), + PyBytes_GET_SIZE(arg)) == 0 ) + ret = Py_True; + else + ret = Py_False; + Py_INCREF(ret); + return ret; + } + + if ( PyUnicode_CheckExact(self) && PyUnicode_CheckExact(arg) ) { + if ( PyUnicode_GET_SIZE(self) < PyUnicode_GET_SIZE(arg) ) + ret = Py_False; + else if ( memcmp(PyUnicode_AS_UNICODE(self), PyUnicode_AS_UNICODE(arg), + PyUnicode_GET_DATA_SIZE(arg)) == 0 ) + ret = Py_True; + else + ret = Py_False; + Py_INCREF(ret); + return ret; + } + + return PyObject_CallMethod(self, "startswith", "O", arg); I'd split this up into two functions that can be applied to different types independently, and wrap it in a function that calls either one depending on PY_MAJOR_VERSION. After all, "str" is *known* to be "bytes" in Py2 and "unicode" in Py3. @@ -522,8 +557,12 @@ builtin_types_table = [ ]), ("bytes", "PyBytes_Type", []), - ("str", "PyString_Type", []), + ("str", "PyString_Type", [BuiltinMethod("startswith", "TO", "O", "_Pyx_StrStartsWith", + utility_code=str_startswith_utility_code), + ]), It's better to return a "bint" to avoid returning Python objects for True/False. The type character for that is "b". There's a "__Pyx_PyObject_IsTrue(obj)" macro that you can use to convert the fallback return value. It's usually also ok to copy error handling code from CPython, and to raise the exception directly without going into the fallback case. ("unicode", "PyUnicode_Type", [BuiltinMethod("join", "TO", "T", "PyUnicode_Join"), + BuiltinMethod("startswith", "TO", "O", "_Pyx_StrStartsWith", + utility_code=str_startswith_utility_code), ]), This is already optimised in Cython/Compiler/Optimize.py ("tailmatch"), basically because it allows different numbers of arguments that need to be dealt with. It may be worth going the same route for "str". The decision usually depends on how complex the code transformation is. The method table in Builtins is clearly limited. diff --git a/tests/run/stropts.pyx b/tests/run/stropts.pyx @@ -0,0 +1,84 @@ +cimport cython + +(a)cython.test_assert_path_exists( + '//AttributeNode[@member="_Pyx_StrStartsWith"]') +def str_startswith_str(): + """ + >>> str_startswith_str() + True + """ + + cdef str a + cdef str b + + a = '' + b = '' + return a.startswith(b) [...] I usually just implement one or a few test functions and use the doctest to pass in different values. Stefan

2 1

Git workflow, branches, pull requests
by Dag Sverre Seljebotn May 13, 2011

May 13, 2011

There was just a messup in git history: Mark's OpenMP pull request got merged twice; all commits show up two times. It doesn't matter, since the two openmp branches with the same changes merged OK, but we shouldn't make this a habit. For instance, the openMP commits also show up as part of vitja's pull request, which is confusing. In Mercurial speak: The openmp branch was used like you would use a Mercurial "patch queue" in one case, and as a branch in another case. In git they are the same technically and you rely on conventions to make sure you don't treat a "queue" as a "branch". OPTION A) Either i) only branch from master, or ii) make sure you agree with whoever you're branching from that this is a "branch", not a "patch queue", so that it isn't rebased under your feet. We could also, say, prepend all patch queues with an underscore (its private). OPTION B) Stop rebasing. I'd have a very hard time doing that myself, but nobody are pulling from dagss/cython these days anyway. Opinions? FYI, The workflow me and Mark is currently using is: a) Fork off a feature branch from master (with master I'll always refer to cython/master) b) When one gets in sync with master, do NOT merge master, but rather rebase on top of it: git pull --rebase origin master c) Continue rebasing, and eventually . The advantage of this approach is that ugly merges disappear from history, since commits are rewritten. And the history graph looks very nice and is easy to follow. BUT, if the result is duplication, we should avoid this practice, and rather always merge. Dag Sverre

7 19

Buffer type specification (CEP 514) in @cython.locals() in .pxd
by Matěj Laitl May 11, 2011

May 11, 2011

Hi list, it has been unusually quiet here so it's time to speak up. How hard would it be to implement support for specifying buffer dtype (and friends) in @cython.locals() in a .pxd file? Following code (in a .pxd file) cimport numpy @cython.locals(local1 = numpy.ndarray[double]) cdef double f() currently yields "test.pxd:4:32: Not a type" error. The problem is that one currently cannot make use of buffer access optimisation for local variables in pure python mode. I use pure mode extensively in my project [1]. [1] https://github.com/strohel/PyBayes I understand that it has no sense in .py files as type type is not usually subscriptable. On the same note, is there a plan to implement fast buffer access for class variables? ;) (this is obviously not that crucial) Regards and keep up the good work! Matěj Laitl

1 0

buffers and flow-control
by Vitja Makarov May 9, 2011

May 9, 2011

I've never been using buffers so my question is: Should uninitialized buffer access raise UnboundLocalVariable error? Like this: def foo(): cdef object<int> bar print bar -- vitja.

2 1

Re: [Cython] [SciPy-User] Scikits.sparse build issue
by Dag Sverre Seljebotn May 5, 2011

May 5, 2011

On 05/05/2011 08:42 PM, Nathaniel Smith wrote: > On Thu, May 5, 2011 at 3:03 AM, Anand Patil > <anand.prabhakar.patil(a)gmail.com> wrote: >> >> On May 4, 8:16 pm, Nathaniel Smith<n...(a)pobox.com> wrote: >>> On Tue, May 3, 2011 at 10:10 AM, Nathaniel Smith<n...(a)pobox.com> wrote: >>>> On Tue, May 3, 2011 at 5:51 AM, Anand Patil >>>> <anand.prabhakar.pa...(a)gmail.com> wrote: >>>>> scikits/sparse/cholmod.c: In function >>>>> ‘__pyx_f_7scikits_6sparse_7cholmod__py_sparse’: >>>>> scikits/sparse/cholmod.c:1713: error: storage size of ‘__pyx_t_10’ >>>>> isn’t known >>> >>>>> I've never used Cython and am having a hard time figuring this out. >>> >>>> Could you send me the file 'scikits/sparse/cholmod.c'? This means that >>>> there's some C type that was forward-declared, but never actually >>>> defined, and then we tried to instantiate an instance of it. But I'll >>>> need to see the generated code to figure out which type '__pyx_t_10' >>>> is supposed to be. >>> >>> Huh, this appears to be some bad interaction between numpy and cython, >>> rather than anything to do with my code. The offending variable comes >>> from doing 'cimport numpy as np' and then referring to >>> 'np.NPY_F_CONTIGUOUS' -- this is being translated to: >>> enum requirements __pyx_t_10; >>> __pyx_t_10 = NPY_F_CONTIGUOUS; >>> and then gcc is complaining that 'enum requirements' is an undefined type. >>> >>> What version of Numpy and Cython do you have installed? >> >> Cython 0.14.1, Numpy 1.5.1. Which versions do you have? > > It looks like with Cython 0.12.1, which is what I was using before, it > happens not to generate a temporary variable in this case, but Cython > 0.14.1 generates the temporary variable. > > I've just committed a workaround to the scikits.sparse repository: > https://code.google.com/p/scikits-sparse/source/detail?r=ad106e9c2c2d55f202… > (I believe it works -- it does compile -- but technically I can't > guarantee it since for me the tests are now failing with an "illegal > instruction" error inside BLAS. But I think this must be an unrelated > Ubuntu screwup. Yay software.) > > And I'll see about poking Cython upstream to get this fixed... Awh. Thanks! https://github.com/cython/cython/commit/a6ec50077990a9767695896076a8b573a5b… Dag Sverre

2 1

Hudson pyregr testing takes too long
by Stefan Behnel May 5, 2011

May 5, 2011

Hi, I just noticed that the CPython pyregr tests have jumped up from ~14 minutes for a run to ~4 hours when we added generator support. https://sage.math.washington.edu:8091/hudson/view/cython-devel/job/cython-d… I currently have no idea why that is (well, it's likely because we compile more tests now, but Vitja's branch ran the tests in ~30 minutes). It would be great if someone could find the time to analyse this problem. The current run time makes it basically impossible to keep these tests enabled. Stefan

2 12

Fused Types
by mark florisson May 4, 2011

May 4, 2011

Hey, I've been working a bit on fused types (http://wiki.cython.org/enhancements/fusedtypes), and I've got it to generate code for every permutation of specific types. Currently it only works for cdef functions. Now in SimpleCallNode I'm trying to use PyrexTypes.best_match() to find the function with the best signature, but it doesn't seem to take care of coercions, e.g. it calls 'assignable_from' on the dst_type (e.g. char *), but for BuiltinObjectType (a str) this returns false. Why is this the case, don't calls to overloaded C++ methods need to dispatch properly here also? Other issues are public and api declarations. So should we support that at all? We could define a macro that calls a function that does the dispatch. So e.g. for this ctypedef cython.fused_type(typeA, typeB) dtype cdef func(dtype x): ... we would get two generated functions, say, __pyx_typeA_func and __pyx_typeB_func. So we could have a macro get_func(dtype) or something that then substitutes __pyx_get_func(#dtype), where __pyx_get_func returns the pointer to the right function based on the type names. I'm not sure we should support it, right now I just put the mangled names in the header. At least the cdef functions will be sharable between Cython implementation files. I also noticed that for cdef functions with optional argument it gets a struct as argument, but this struct doesn't seem to end up in the header file when the function is declared public. I believe that also the typedefs for ctypedef-ed things in the .pyx file don't make it there when used to type a cdef function's arguments. Should that be fixed? Cheers, Mark

7 82

jenkins problems
by Vitja Makarov May 4, 2011

May 4, 2011

Hi! Jenkins doesn't work for me. It seems that it can't do pull and is running tests again obsolete sources. May be because of forced push. There are only 6 errors here: https://sage.math.washington.edu:8091/hudson/view/cython-vitek/job/cython-v… -- vitja.

3 9

prange CEP updated
by Dag Sverre Seljebotn May 4, 2011

May 4, 2011

I've done a pretty major revision to the prange CEP, bringing in a lot of the feedback. Thread-private variables are now split in two cases: i) The safe cases, which really require very little technical knowledge -> automatically inferred ii) As an advanced feature, unsafe cases that requires some knowledge of threading -> must be explicitly declared I think this split simplifies things a great deal. I'm rather excited over this now; this could turn out to be a really user-friendly and safe feature that would not only allow us to support OpenMP-like threading, but be more convenient to use in a range of common cases. http://wiki.cython.org/enhancements/prange <http://wiki.cython.org/enhancements/prange#preview> Dag Sverre

3 63

Dead code removal
by Vitja Makarov May 3, 2011

May 3, 2011

Hi! I can move unreachable code removal into its own branch and then make a pull request. But it seems to me that dead code removal should be done in control flow analysis so there are few options: 1. remove unreachable code in control flow transformation 2. set is_terminator flag in control flow and then run removal transformation later For instance, RemoveUnreachableCode doesn't handle this case: try: raise Error finally: pass print 'unreachable' This case could be easily handled if I set is_terminator flag on the whole TryFinallyStatNode, but I'm not sure that this is the only unhandled case. -- vitja.

1 0