Re: [Cython] About IndexNode and unicode[index]

2013-03-01 Thread Stefan Behnel
Zaur Shibzukhov, 01.03.2013 07:54:
 I think you could even pass in two flags, one for wraparound and one for
 boundscheck, and then just evaluate them appropriately in the existing "if"
 tests above. That should allow both features to be supported independently
 in a fast way.

>>> Intresting, could C compilers in optimization mode to eliminate unused
>>> evaluation path in nested if statements with constant conditional
>>> expressions?
>>
>> They'd be worthless if they didn't do that. (Even Cython does it, BTW.)
>>
> Then it can simplify writing utility code in order to support
> different optimization flags in other cases too.

Usually, yes. Look at the dict iteration code, for example, which makes
pretty heavy use of it.

This may not work in all cases, because the C compiler can decide to *not*
inline a function, for example, or may not be capable of cutting down the
code sufficiently in some specific cases.

I agree in general, but I wouldn't say that it's worth changing existing
(and working) code.

Stefan

___
cython-devel mailing list
cython-devel@python.org
http://mail.python.org/mailman/listinfo/cython-devel


Re: [Cython] About IndexNode and unicode[index]

2013-03-01 Thread Zaur Shibzukhov
>> Then it can simplify writing utility code in order to support
>> different optimization flags in other cases too.
>
> Usually, yes. Look at the dict iteration code, for example, which makes
> pretty heavy use of it.
>
> This may not work in all cases, because the C compiler can decide to *not*
> inline a function, for example, or may not be capable of cutting down the
> code sufficiently in some specific cases.
>
> I agree in general, but I wouldn't say that it's worth changing existing
> (and working) code.
>
Thus the strategy of specialization of code to handle special cases,
while maintaining the existing code, which works well in general, is
the preferred?
___
cython-devel mailing list
cython-devel@python.org
http://mail.python.org/mailman/listinfo/cython-devel


Re: [Cython] About IndexNode and unicode[index]

2013-03-01 Thread Robert Bradshaw
On Thu, Feb 28, 2013 at 10:54 PM, Zaur Shibzukhov  wrote:

 I think you could even pass in two flags, one for wraparound and one for
 boundscheck, and then just evaluate them appropriately in the existing "if"
 tests above. That should allow both features to be supported independently
 in a fast way.

>>> Intresting, could C compilers in optimization mode to eliminate unused
>>> evaluation path in nested if statements with constant conditional
>>> expressions?
>>
>> They'd be worthless if they didn't do that. (Even Cython does it, BTW.)
>>
> Then it can simplify writing utility code in order to support
> different optimization flags in other cases too.

The one thing you don't have much control over is whether the C
compiler will actually inline the function (CYTHON_INLINE is just a
hint). In particular, it may decide the function is too large to
inline before realizing how small it would become given the constant
arguments. I'm actually not sure how much of a problem this is in
practice...

- Robert
___
cython-devel mailing list
cython-devel@python.org
http://mail.python.org/mailman/listinfo/cython-devel


Re: [Cython] About IndexNode and unicode[index]

2013-03-01 Thread Zaur Shibzukhov
2013/3/1 Stefan Behnel :
> ZS, 28.02.2013 21:07:
>> 2013/2/28 Stefan Behnel:
 This allows to write unicode text parsing code almost at C speed
 mostly in python (+ .pxd defintions).
>>>
>>> I suggest simply adding a constant flag argument to the existing function
>>> that states if checking should be done or not. Inlining will let the C
>>> compiler drop the corresponding code, which may or may nor make it a little
>>> faster.
>>
>> static inline Py_UCS4 unicode_char2(PyObject* ustring, Py_ssize_t i, int 
>> flag) {
>> Py_ssize_t length;
>> #if CYTHON_PEP393_ENABLED
>> if (PyUnicode_READY(ustring) < 0) return (Py_UCS4)-1;
>> #endif
>> if (flag) {
>> length = __Pyx_PyUnicode_GET_LENGTH(ustring);
>> if ((0 <= i) & (i < length)) {
>> return __Pyx_PyUnicode_READ_CHAR(ustring, i);
>> } else if ((-length <= i) & (i < 0)) {
>> return __Pyx_PyUnicode_READ_CHAR(ustring, i + length);
>> } else {
>> PyErr_SetString(PyExc_IndexError, "string index out of range");
>> return (Py_UCS4)-1;
>> }
>> } else {
>> return __Pyx_PyUnicode_READ_CHAR(ustring, i);
>> }
>> }
>
> I think you could even pass in two flags, one for wraparound and one for
> boundscheck, and then just evaluate them appropriately in the existing "if"
> tests above. That should allow both features to be supported independently
> in a fast way.
>
>
>> Here are timings:
>>
>> (py33) zbook:mytests $ python3.3 -m timeit -n 50 -r 5 -s "from
>> mytests.unicode_index import test_1" "test_1()"
>> 50 loops, best of 5: 152 msec per loop
>> (py33) zbook:mytests $ python3.3 -m timeit -n 50 -r 5 -s "from
>> mytests.unicode_index import test_2" "test_2()"
>> 50 loops, best of 5: 86.5 msec per loop
>> (py33) zbook:mytests $ python3.3 -m timeit -n 50 -r 5 -s "from
>> mytests.unicode_index import test_3" "test_3()"
>> 50 loops, best of 5: 86.5 msec per loop
>>
>> So your suggestion would be preferable.
>
> Nice. Yes, looks like it' worth it.
>
Could I help in order to include this in 19.0?

Zaur Shibzukhov
___
cython-devel mailing list
cython-devel@python.org
http://mail.python.org/mailman/listinfo/cython-devel


Re: [Cython] About IndexNode and unicode[index]

2013-03-01 Thread Stefan Behnel
Zaur Shibzukhov, 01.03.2013 10:46:
> Could I help in order to include this in 19.0?

I like pull requests. ;)

Stefan

___
cython-devel mailing list
cython-devel@python.org
http://mail.python.org/mailman/listinfo/cython-devel


Re: [Cython] About IndexNode and unicode[index]

2013-03-01 Thread Zaur Shibzukhov
2013/3/1 Stefan Behnel :
> Zaur Shibzukhov, 01.03.2013 10:46:
>> Could I help in order to include this in 19.0?
>
> I like pull requests. ;)
>
OK
___
cython-devel mailing list
cython-devel@python.org
http://mail.python.org/mailman/listinfo/cython-devel


Re: [Cython] Be more forgiving about memoryview strides

2013-03-01 Thread Sebastian Berg
On Thu, 2013-02-28 at 23:25 -0800, Robert Bradshaw wrote:
> On Thu, Feb 28, 2013 at 11:12 AM, Nathaniel Smith  wrote:
> > On Thu, Feb 28, 2013 at 5:50 PM, Robert Bradshaw  wrote:
> >> On Thu, Feb 28, 2013 at 7:13 AM, Sebastian Berg
> >>  wrote:
> >>> Hey,
> >>>
> >>> Maybe someone here already saw it (I don't have a track account, or I
> >>> would just create a ticket), but it would be nice if Cython was more
> >>> forgiving about contiguous requirements on strides. In the future this
> >>> would make it easier for numpy to go forward with changing the
> >>> contiguous flags to be more reasonable for its purpose, and second also
> >>> to allow old (and maybe for the moment remaining) corner cases in numpy
> >>> to slip past (as well as possibly the same for other programs...). An
> >>> example is (see also https://github.com/numpy/numpy/issues/2956 and the
> >>> PR linked there for more details):
> >>>
> >>> def add_one(array):
> >>> cdef double[::1] a = array
> >>> a[0] += 1.
> >>> return array
> >>>
> >>> giving:
> >>>
> >> add_one(np.ascontiguousarray(np.arange(10.)[::100]))
> >>> ValueError: Buffer and memoryview are not contiguous in the same
> >>> dimension.
> >>>
> >>> This could easily be changed if MemoryViews check the strides as "can be
> >>> interpreted as contiguous". That means that if shape[i] == 1, then
> >>> strides[i] are arbitrary (you can just change them if you like). This is
> >>> also the case for 0-sized arrays, which are arguably always contiguous,
> >>> no matter their strides are!
> >>
> >> I was under the impression that the primary value for contiguous is
> >> that it a foo[::1] can be interpreted as a foo*. Letting strides be
> >> arbitrary completely breaks this, right?
> >
> > Nope. The natural definition of "C contiguous" is "the array entries
> > are arranged in memory in the same way they would be if they were a
> > multidimensional C array" (i.e., what you said.) But it turns out that
> > this is *not* the definition that numpy and cython use!
> >
> > The issue is that the above definition is a constraint on the actual
> > locations of items in memory, i.e., given a shape, it tells you that
> > for every index,
> >  (a)  sum(index * strides) == sum(index * cumprod(shape[::-1])[::-1] * 
> > itemsize)
> > Obviously this equality holds if
> >  (b)  strides == cumprod(shape[::-1])[::-1] * itemsize
> > (Or for F-contiguity, we have
> >  (b')  strides == cumprod(shape) * itemsize
> > )
> >
> > (a) is the natural definition of "C contiguous". (b) is the definition
> > of "C contiguous" used by numpy and cython. (b) implies (a). But (a)
> > does not imply (b), i.e., there are arrays that are C-contiguous which
> > numpy and cython think are discontiguous. (Also in numpy there are
> > some weird cases where numpy accidentally uses the correct definition,
> > I think, which is the point of Sebastian's example.)
> >
> > In particular, if shape[i] == 1, then the value of stride[i] really
> > should be irrelevant to judging contiguity, because the only thing you
> > can do with strides[i] is multiply it by index[i], and if shape[i] ==
> > 1 then index[i] is always 0. So an array of int8's with shape = (10,
> > 1), strides = (1, 73) is contiguous according to (a), but not
> > according to (b). Also if shape[i] is 0 for any i, then the entire
> > contents of the strides array becomes irrelevant to judging
> > contiguity; all zero-sized arrays are contiguous according to (a), but
> > not (b).
> 
> Thanks for clarifying.
> 
> Yes, I think it makes a lot of sense to loosen our definition for
> Cython. Internally, I think the only way we use this assumption is in
> not requiring that the first/final index be multiplied by the stride,
> which should be totally fine. But this merits closer inspection as
> there may be something else.

The only problem I saw was code that used strides[-1] instead of the
itemsize (e.g. using strides[i]/strides[-1] to then index the typed
buffer instead of using strides[i]/itemsize). But that should be easy to
check, numpy had two or so cases of that itself...

> 
> > (This is really annoying for numpy because given, say, a column vector
> > with shape (n, 1), it is impossible to be both C- and F-contiguous
> > according to the (b)-style definition. But people expect expect
> > various operations to preserve C versus F contiguity, so there are
> > heuristics in numpy that try to guess whether various result arrays
> > should pretend to be C- or F-contiguous, and we don't even have a
> > consistent idea of what it would mean for this code to be working
> > correctly, never mind test it and keep it working. OTOH if we just fix
> > numpy to use the (a) definition, then it turns out a bunch of
> > third-party code breaks, like, for example, cython.)
> 
> Can you give some examples?
> 

Not sure for what :). Maybe this is an example:

In [1]: a = np.asmatrix(np.arange(9).reshape(3,3).T)

In [2]: a.flags.f_contiguous
Out[2]: True

In [3]: a[:,0].flags

Re: [Cython] Be more forgiving about memoryview strides

2013-03-01 Thread Robert Bradshaw
On Fri, Mar 1, 2013 at 7:56 AM, Sebastian Berg
 wrote:
> On Thu, 2013-02-28 at 23:25 -0800, Robert Bradshaw wrote:
>> On Thu, Feb 28, 2013 at 11:12 AM, Nathaniel Smith  wrote:
>> > On Thu, Feb 28, 2013 at 5:50 PM, Robert Bradshaw  
>> > wrote:
>> >> On Thu, Feb 28, 2013 at 7:13 AM, Sebastian Berg
>> >>  wrote:
>> >>> Hey,
>> >>>
>> >>> Maybe someone here already saw it (I don't have a track account, or I
>> >>> would just create a ticket), but it would be nice if Cython was more
>> >>> forgiving about contiguous requirements on strides. In the future this
>> >>> would make it easier for numpy to go forward with changing the
>> >>> contiguous flags to be more reasonable for its purpose, and second also
>> >>> to allow old (and maybe for the moment remaining) corner cases in numpy
>> >>> to slip past (as well as possibly the same for other programs...). An
>> >>> example is (see also https://github.com/numpy/numpy/issues/2956 and the
>> >>> PR linked there for more details):
>> >>>
>> >>> def add_one(array):
>> >>> cdef double[::1] a = array
>> >>> a[0] += 1.
>> >>> return array
>> >>>
>> >>> giving:
>> >>>
>> >> add_one(np.ascontiguousarray(np.arange(10.)[::100]))
>> >>> ValueError: Buffer and memoryview are not contiguous in the same
>> >>> dimension.
>> >>>
>> >>> This could easily be changed if MemoryViews check the strides as "can be
>> >>> interpreted as contiguous". That means that if shape[i] == 1, then
>> >>> strides[i] are arbitrary (you can just change them if you like). This is
>> >>> also the case for 0-sized arrays, which are arguably always contiguous,
>> >>> no matter their strides are!
>> >>
>> >> I was under the impression that the primary value for contiguous is
>> >> that it a foo[::1] can be interpreted as a foo*. Letting strides be
>> >> arbitrary completely breaks this, right?
>> >
>> > Nope. The natural definition of "C contiguous" is "the array entries
>> > are arranged in memory in the same way they would be if they were a
>> > multidimensional C array" (i.e., what you said.) But it turns out that
>> > this is *not* the definition that numpy and cython use!
>> >
>> > The issue is that the above definition is a constraint on the actual
>> > locations of items in memory, i.e., given a shape, it tells you that
>> > for every index,
>> >  (a)  sum(index * strides) == sum(index * cumprod(shape[::-1])[::-1] * 
>> > itemsize)
>> > Obviously this equality holds if
>> >  (b)  strides == cumprod(shape[::-1])[::-1] * itemsize
>> > (Or for F-contiguity, we have
>> >  (b')  strides == cumprod(shape) * itemsize
>> > )
>> >
>> > (a) is the natural definition of "C contiguous". (b) is the definition
>> > of "C contiguous" used by numpy and cython. (b) implies (a). But (a)
>> > does not imply (b), i.e., there are arrays that are C-contiguous which
>> > numpy and cython think are discontiguous. (Also in numpy there are
>> > some weird cases where numpy accidentally uses the correct definition,
>> > I think, which is the point of Sebastian's example.)
>> >
>> > In particular, if shape[i] == 1, then the value of stride[i] really
>> > should be irrelevant to judging contiguity, because the only thing you
>> > can do with strides[i] is multiply it by index[i], and if shape[i] ==
>> > 1 then index[i] is always 0. So an array of int8's with shape = (10,
>> > 1), strides = (1, 73) is contiguous according to (a), but not
>> > according to (b). Also if shape[i] is 0 for any i, then the entire
>> > contents of the strides array becomes irrelevant to judging
>> > contiguity; all zero-sized arrays are contiguous according to (a), but
>> > not (b).
>>
>> Thanks for clarifying.
>>
>> Yes, I think it makes a lot of sense to loosen our definition for
>> Cython. Internally, I think the only way we use this assumption is in
>> not requiring that the first/final index be multiplied by the stride,
>> which should be totally fine. But this merits closer inspection as
>> there may be something else.
>
> The only problem I saw was code that used strides[-1] instead of the
> itemsize (e.g. using strides[i]/strides[-1] to then index the typed
> buffer instead of using strides[i]/itemsize). But that should be easy to
> check, numpy had two or so cases of that itself...

I'd be surprised if we do that, but the only way to be sure would be
to look at the code.

>> > (This is really annoying for numpy because given, say, a column vector
>> > with shape (n, 1), it is impossible to be both C- and F-contiguous
>> > according to the (b)-style definition. But people expect expect
>> > various operations to preserve C versus F contiguity, so there are
>> > heuristics in numpy that try to guess whether various result arrays
>> > should pretend to be C- or F-contiguous, and we don't even have a
>> > consistent idea of what it would mean for this code to be working
>> > correctly, never mind test it and keep it working. OTOH if we just fix
>> > numpy to use the (a) definition, then it turns out a bunch of
>> > third-party

Re: [Cython] Be more forgiving about memoryview strides

2013-03-01 Thread Sebastian Berg
On Fri, 2013-03-01 at 12:17 -0800, Robert Bradshaw wrote:
> On Fri, Mar 1, 2013 at 7:56 AM, Sebastian Berg
>  wrote:
> > On Thu, 2013-02-28 at 23:25 -0800, Robert Bradshaw wrote:
> >> On Thu, Feb 28, 2013 at 11:12 AM, Nathaniel Smith  wrote:
> >> > On Thu, Feb 28, 2013 at 5:50 PM, Robert Bradshaw  
> >> > wrote:
> >> >> On Thu, Feb 28, 2013 at 7:13 AM, Sebastian Berg
> >> >>  wrote:
> >> >>> Hey,
> >> >>>
> >> >>> Maybe someone here already saw it (I don't have a track account, or I
> >> >>> would just create a ticket), but it would be nice if Cython was more
> >> >>> forgiving about contiguous requirements on strides. In the future this
> >> >>> would make it easier for numpy to go forward with changing the
> >> >>> contiguous flags to be more reasonable for its purpose, and second also
> >> >>> to allow old (and maybe for the moment remaining) corner cases in numpy
> >> >>> to slip past (as well as possibly the same for other programs...). An
> >> >>> example is (see also https://github.com/numpy/numpy/issues/2956 and the
> >> >>> PR linked there for more details):
> >> >>>
> >> >>> def add_one(array):
> >> >>> cdef double[::1] a = array
> >> >>> a[0] += 1.
> >> >>> return array
> >> >>>
> >> >>> giving:
> >> >>>
> >> >> add_one(np.ascontiguousarray(np.arange(10.)[::100]))
> >> >>> ValueError: Buffer and memoryview are not contiguous in the same
> >> >>> dimension.
> >> >>>
> >> >>> This could easily be changed if MemoryViews check the strides as "can 
> >> >>> be
> >> >>> interpreted as contiguous". That means that if shape[i] == 1, then
> >> >>> strides[i] are arbitrary (you can just change them if you like). This 
> >> >>> is
> >> >>> also the case for 0-sized arrays, which are arguably always contiguous,
> >> >>> no matter their strides are!
> >> >>
> >> >> I was under the impression that the primary value for contiguous is
> >> >> that it a foo[::1] can be interpreted as a foo*. Letting strides be
> >> >> arbitrary completely breaks this, right?
> >> >
> >> > Nope. The natural definition of "C contiguous" is "the array entries
> >> > are arranged in memory in the same way they would be if they were a
> >> > multidimensional C array" (i.e., what you said.) But it turns out that
> >> > this is *not* the definition that numpy and cython use!
> >> >
> >> > The issue is that the above definition is a constraint on the actual
> >> > locations of items in memory, i.e., given a shape, it tells you that
> >> > for every index,
> >> >  (a)  sum(index * strides) == sum(index * cumprod(shape[::-1])[::-1] * 
> >> > itemsize)
> >> > Obviously this equality holds if
> >> >  (b)  strides == cumprod(shape[::-1])[::-1] * itemsize
> >> > (Or for F-contiguity, we have
> >> >  (b')  strides == cumprod(shape) * itemsize
> >> > )
> >> >
> >> > (a) is the natural definition of "C contiguous". (b) is the definition
> >> > of "C contiguous" used by numpy and cython. (b) implies (a). But (a)
> >> > does not imply (b), i.e., there are arrays that are C-contiguous which
> >> > numpy and cython think are discontiguous. (Also in numpy there are
> >> > some weird cases where numpy accidentally uses the correct definition,
> >> > I think, which is the point of Sebastian's example.)
> >> >
> >> > In particular, if shape[i] == 1, then the value of stride[i] really
> >> > should be irrelevant to judging contiguity, because the only thing you
> >> > can do with strides[i] is multiply it by index[i], and if shape[i] ==
> >> > 1 then index[i] is always 0. So an array of int8's with shape = (10,
> >> > 1), strides = (1, 73) is contiguous according to (a), but not
> >> > according to (b). Also if shape[i] is 0 for any i, then the entire
> >> > contents of the strides array becomes irrelevant to judging
> >> > contiguity; all zero-sized arrays are contiguous according to (a), but
> >> > not (b).
> >>
> >> Thanks for clarifying.
> >>
> >> Yes, I think it makes a lot of sense to loosen our definition for
> >> Cython. Internally, I think the only way we use this assumption is in
> >> not requiring that the first/final index be multiplied by the stride,
> >> which should be totally fine. But this merits closer inspection as
> >> there may be something else.
> >
> > The only problem I saw was code that used strides[-1] instead of the
> > itemsize (e.g. using strides[i]/strides[-1] to then index the typed
> > buffer instead of using strides[i]/itemsize). But that should be easy to
> > check, numpy had two or so cases of that itself...
> 
> I'd be surprised if we do that, but the only way to be sure would be
> to look at the code.
> 
> >> > (This is really annoying for numpy because given, say, a column vector
> >> > with shape (n, 1), it is impossible to be both C- and F-contiguous
> >> > according to the (b)-style definition. But people expect expect
> >> > various operations to preserve C versus F contiguity, so there are
> >> > heuristics in numpy that try to guess whether various result arrays
> >> > should pretend to be C- or

[Cython] Two minor bugs

2013-03-01 Thread Nikita Nemkin

Hi,

I'm new to this list and to Cython internals.

Reporting two recently found bugs:

1. Explicit  cast fails unexpectedly:

   ctypedef char* LPSTR
   cdef LPSTR c_str = b"ascii"
   c_str  # Failure: Python objects cannot be cast from  
pointers of primitive types


   The problem is CTypedefType not delegating can_coerce_to_pyobject() to  
the original type.
   (because BaseType.can_coerce_to_pyobject takes precedence over  
__getattr__).

   Patch+test case and attached.

   Interestingly, implicit casts use a different code path and are not  
affected.


   There is potential for similar bugs in the future, because __getattr__
   delegation is inherently brittle in the presence of the base class  
(BaseType).


2. This recently added code does not compile with MSVC:
   
https://github.com/cython/cython/blob/master/Cython/Utility/TypeConversion.c#L140-142
   Interleaving declarations and statements is not allowed in C90...


Best Regards,
Nikita Nemkin

0001-Fixed-explicit-coercion-of-ctypedef-ed-C-types.patch
Description: Binary data
___
cython-devel mailing list
cython-devel@python.org
http://mail.python.org/mailman/listinfo/cython-devel