[Cython] change strides attribute
Hey, The current numpy contiguous flags are relatively restrictive. This means that for example an array of `shape == (1,3,1)` and `itemsize == 1` is C-Contiguous if `strides == (3,1,1)` and F-Contiguous if `strides == (1,1,3)`. It would simplify flags settings and avoid copies here and there and generally be what most seem to expect to ignore the `strides[i]` if `shape[i] == 1` (or all if the array has 0 size). Which means that for example `strides == (0,1,-32)` (so arbitrary but `strides[1]`) would be both C- and F-contiguous. However trying to change this, runs into problems with user code relying on `strides[-1] == itemsize` (C-Contiguous) or `strides[0] == itemsize` (F-Contiguous). But there seems to be no way to really deprecate or protect Cython generated C-Code against such a change, because even if the contiguous buffer requested by Cython has nice strides, a user writing `arr.strides` accesses the original array which may not have and there is no way to give compile time warnings by deprecating flags. So I was wondering (I don't know cython that well) if `arr.strides` could not use `buffer.strides` internally to allow redefinition of contiguous flags in numpy in some future without breaking existing Cython user code. Maybe it is not worth it to go through much trouble for these flags but it seems nicer to me for code simplicity and just generally to be more consistent to redefine them at some point. Regards, Sebastian ___ cython-devel mailing list cython-devel@python.org http://mail.python.org/mailman/listinfo/cython-devel
[Cython] Be more forgiving about memoryview strides
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! Regards, Sebastian PS: A similar thing exists with np.ndarray[...] interface if the user accesses array.strides. They get the arrays strides not the buffers. This is not quite related, but if it would be easy to use the buffer's strides in that case, it may make it easier if we want to change the flags in numpy in the long term, since one could clean up strides for forced contiguous buffer requests. ___ cython-devel mailing list cython-devel@python.org http://mail.python.org/mailman/listinfo/cython-devel
Re: [Cython] Be more forgiving about memoryview strides
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)
Re: [Cython] Be more forgiving about memoryview strides
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
Re: [Cython] Upcoming cython/numpy breakage with stride checking
On Mon, 2013-04-08 at 08:42 +0200, Dag Sverre Seljebotn wrote: > On 04/06/2013 04:19 PM, Nathaniel Smith wrote: > > Hi all, > > > > If you build current numpy master with > >NPY_RELAXED_STRIDES_CHECKING=1 python setup.py install > > then Cython code using ndarrays starts blowing up, e.g.: > > > > # foo.pyx > > def add_one(array): > > cdef double[::1] a = array > > a[0] += 1. > > return array > > > foo.add_one(np.ascontiguousarray(np.arange(10.)[::100])) > > Traceback (most recent call last): > >File "", line 1, in > >File "foo.pyx", line 2, in foo.add_one (foo.c:1210) > > cdef double[::1] a = array > > ValueError: Buffer and memoryview are not contiguous in the same dimension. > > > > The problem (as discussed before) is that Cython has an unnecessarily > > strict definition of "contiguous", so NPY_RELAXED_STRIDES_CHECKING=1 > > pretty much breaks all existing compiled Cython modules. > > > > Our plan is to make NPY_RELAXED_STRIDES_CHECKING=1 into the default > > sooner or later, and Cython is a major blocker on this plan. It may > > become the default as soon as the 1.8 pre-releases (with the > > expectation that we'll probably have to switch back again before the > > actual release, but still). > > > > References: > > > > Previous thread: > >http://thread.gmane.org/gmane.comp.python.cython.devel/14634 > > Detailed discussion of the difference between numpy/cython's current > > definition of "contiguity", and the correct definition: > >http://thread.gmane.org/gmane.comp.python.cython.devel/14634/focus=14640 > > The PR implementing NPY_RELAXED_STRIDES_CHECKING: > >https://github.com/numpy/numpy/pull/3162 > > Another test case: > >https://github.com/numpy/numpy/issues/2956 > > > > We're hoping that Cython will also switch soon to the more accurate > > check for contiguity. This shouldn't cause any backwards compatibility > > problems -- it just means Cython code would make strictly fewer > > copies, and error out due to lack of contiguity strictly less often, > > even with older numpys. And it seems like a necessary step for getting > > this untangled and minimizing user pain. What do you think? > > I agree that we should follow NumPy here, but can't see myself having > time to do the change in near future. I don't know about Mark? > > I think it is a fairly simple change though if any NumPyers would like > to do it, look at in Cython/Utility/MemoryView_C.c in the function > > _pyx_memviewslice_is_contig > > looks like it should just be to add a check for shape too. > > I guess you have changed your implementation of PEP 3118 too slightly on > the NumPy side? Is this undefined in the PEP or are you now not in > strict adherence to it? > Hi, maybe I will have a look at that, but not sure if I will manage. The PEP 3118 is a point, but it does not seem to cover this (probably we should try to get a clarification into 3118). I am still wondering whether for buffers that are requested contiguous numpy should set the strides again, since it cannot hurt. Would that make a difference for Cython? I expected Cython just got any buffer and then checked the strides instead of requesting the buffer contiguous and then double checking. Regards, Sebastian > Dag Sverre > ___ > cython-devel mailing list > cython-devel@python.org > http://mail.python.org/mailman/listinfo/cython-devel > ___ cython-devel mailing list cython-devel@python.org http://mail.python.org/mailman/listinfo/cython-devel
Re: [Cython] Upcoming cython/numpy breakage with stride checking
On Mon, 2013-04-08 at 12:31 +0200, Dag Sverre Seljebotn wrote: > On 04/08/2013 09:59 AM, Sebastian Berg wrote: > > On Mon, 2013-04-08 at 08:42 +0200, Dag Sverre Seljebotn wrote: > >> On 04/06/2013 04:19 PM, Nathaniel Smith wrote: > >>> Hi all, > >>> > >>> If you build current numpy master with > >>> NPY_RELAXED_STRIDES_CHECKING=1 python setup.py install > >>> then Cython code using ndarrays starts blowing up, e.g.: > >>> > >>> # foo.pyx > >>> def add_one(array): > >>> cdef double[::1] a = array > >>> a[0] += 1. > >>> return array > >>> > >>>>>> foo.add_one(np.ascontiguousarray(np.arange(10.)[::100])) > >>> Traceback (most recent call last): > >>> File "", line 1, in > >>> File "foo.pyx", line 2, in foo.add_one (foo.c:1210) > >>> cdef double[::1] a = array > >>> ValueError: Buffer and memoryview are not contiguous in the same > >>> dimension. > >>> > >>> The problem (as discussed before) is that Cython has an unnecessarily > >>> strict definition of "contiguous", so NPY_RELAXED_STRIDES_CHECKING=1 > >>> pretty much breaks all existing compiled Cython modules. > >>> > >> > >> I guess you have changed your implementation of PEP 3118 too slightly on > >> the NumPy side? Is this undefined in the PEP or are you now not in > >> strict adherence to it? > >> > > > > Hi, > > > > maybe I will have a look at that, but not sure if I will manage. The > > PEP 3118 is a point, but it does not seem to cover this (probably we > > should try to get a clarification into 3118). I am still wondering > > whether for buffers that are requested contiguous numpy should set the > > strides again, since it cannot hurt. Would that make a difference for > > Cython? I expected Cython just got any buffer and then checked the > > strides instead of requesting the buffer contiguous and then double > > checking. > > At least when I implemented > > cdef np.ndarray[double, mode='fortran'] arr > > that relies on PEP 3118 contiguous-flags and I did no checking myself. > Lots of Cython code does this instead of memoryviews (I still write my > own code that way). Yeah, though even if numpy "fixes" the buffer strides, `arr.strides` points at the original array I believe, leaving it to the user. But that just means users have to be a bit more careful with strides. I think I will have numpy "fix" the buffers stride anyway (when requested contiguous through the buffer protocol). I cannot think of any reason not to do it and it may work around bugs in extensions which may not even be aware of numpy's existence. > > The memory views OTOH does their own checking, but I also see plenty of > references to PyBUF_C_CONTIGUOUS etc. inside > Cython/Utility/MemoryView.pyx, so perhaps it does both. Mark would have > the definitive answer here. > After a quick check, it seems to me that those are there for exposing the memoryviews buffer and not for getting a buffer from another object. - Sebastian > Dag Sverre > ___ > cython-devel mailing list > cython-devel@python.org > http://mail.python.org/mailman/listinfo/cython-devel > ___ cython-devel mailing list cython-devel@python.org http://mail.python.org/mailman/listinfo/cython-devel
Re: [Cython] [PATCH] Refcount error when transposing memoryview attribute of an extension class
On Fri, 2013-04-12 at 17:37 +0100, mark florisson wrote: > > > On 12 April 2013 13:20, Matěj Laitl wrote: > On 8. 4. 2013 Matěj Laitl wrote: > > Hi cython-devel and Mark, > > I was getting > > > > > Fatal Python error: Acquisition count is 0 (line XYZ) > > > > when I was doing > > > > > cdef class MemViewContainer: > > > cdef double[:, :] A > > > > > > cdef a_method(self): > > > self.A = np.eye(2) > > > some_function(self.A.T) > > > some_function(self.A.T) > > > > I have found out that it is caused by the self.A.T > expression - Cython emits > > __PYX_XDEC_MEMVIEW() after the call, but no > __PYX_INC_MEMVIEW() before the > > call. This doesn't happen if the memoryview is a > function-local variable. > > > > Proper test case is in the pull request [1] along with an > ad-hoc patch that > > fixes the problem here, but needs review whether it is > actually correct. I'd > > be very grateful if a fix for this problem could get it into > Cython 0.19. > > > > Cython version: 0.19b1 3a6b9856187d7e490e08 > > > > [1] https://github.com/cython/cython/pull/201 > > > Bump. Anything else I can provide to help with getting this > fixed in 0.19? > > Ragards, > Matěj > > Thanks for the bump, I merged it. > > > Stefan, do you want to wait with 0.19 for the strides fix? We don't > know when 0.20 would be, I'd like to give it a go this weekend. Hey, if you have started on the strides fix or a PR, give me a ping and I will have a look at it if you like. If you still need tests, checking that these arrays work as both C and F-contiguous should be sufficient (of course numpy does not know about that yet, unless NPY_RELAXED_STRIDES_CHECKING is set): max_intp = np.iinfo(np.intp).max # obviously weird stride a = np.ones((1, 10, 1)) a.strides = (max_intp, a.itemsize, max_intp) and: a = np.ones((10, 0, 10)) a.strides = (max_intp,) * 3 Regards, Sebastian > ___ > cython-devel mailing list > cython-devel@python.org > http://mail.python.org/mailman/listinfo/cython-devel ___ cython-devel mailing list cython-devel@python.org http://mail.python.org/mailman/listinfo/cython-devel
Re: [Cython] [PATCH] Refcount error when transposing memoryview attribute of an extension class
On Wed, 2013-04-17 at 13:39 +0200, Stefan Behnel wrote: > Sebastian Berg, 17.04.2013 12:32: > > On Fri, 2013-04-12 at 17:37 +0100, mark florisson wrote: > >> Stefan, do you want to wait with 0.19 for the strides fix? We don't > >> know when 0.20 would be, I'd like to give it a go this weekend. > > > > if you have started on the strides fix or a PR, give me a ping and I > > will have a look at it if you like. > > It's already in the latest master, so please give it a try there. I was > planning to release an RC shortly. > Ah, awesome. I only checked the pull requests sporadically and it slipped past my notice. Small point, I think it does not always work for higher dimensional arrays that have no elements, because then it would error out before finding out that the array is empty. This should be very rare though, so no need to rush to fix it I think, so thanks again! - Sebastian > Stefan > > ___ > cython-devel mailing list > cython-devel@python.org > http://mail.python.org/mailman/listinfo/cython-devel > ___ cython-devel mailing list cython-devel@python.org http://mail.python.org/mailman/listinfo/cython-devel
Re: [Cython] "relaxed_strides" test broken with NumPy 1.8
On Sun, 2014-01-05 at 09:26 +0100, Stefan Behnel wrote: > Stefan Behnel, 05.01.2014 09:03: > > Nathaniel Smith, 05.01.2014 02:07: > >> On 4 Jan 2014 22:01, "Stefan Behnel" wrote: > >>> Stefan Behnel, 04.01.2014 22:51: > Stefan Behnel, 04.01.2014 22:47: > > Nathaniel Smith, 04.01.2014 18:36: > >> > >> if not np.ones((10, 1), order="C").flags.f_contiguous: > >> # numpy without relaxed stride support > >> raise SkipTest > > > > https://github.com/cython/cython/commit/e1982505564125714d2010391eecfb8de61626fa > > Hmm, but this doesn't seem to work for me in older NumPy versions, > although > the original test used to work there. Should we explicitly test for NumPy > 1.8+ as well? > >>> > >>> https://github.com/cython/cython/commit/a95d8f912c995300a13fc244ee71bc277668cb9a > >> > >> No, I'm missing something now; AFAIK there are only two numpy behaviors: > >> with relaxed strides and without relaxed strides, and version number should > >> be irrelevant beyond that. What's different between > >> 1.8-without-relaxed-strides and 1.7 that makes the test break? > > > > Mark would certainly know better than me. > > > > In any case, the test works with both NumPy 1.7 (tested with Py 2.x and > > 3.[12] on Jenkins) and NumPy 1.8 with relaxed strides support, but not with > > NumPy 1.8 without relaxed strides. The last two were tested in Py3.3 only, > > in case that matters. I also tested it locally now (in 3.3) and your > > snippet successfully distinguishes the two builds for me, but the test > > starts to fail when I disable relaxed strides in NumPy and works when it's > > enabled. > > I should add that this > > np.ones((10, 1), order="C").flags.f_contiguous > > returns False on NumPy 1.7 and only True on 1.8 with relaxed strides, thus > the additional version test for <1.8. > I didn't really follow this (only saw it now). Just to add... Numpy 1.7. does not have relaxed strides. However, in some cases 1.8. strides checks are more strict (and consistent) when not compiled with relaxed strides. So it is plausible that the tests simply run into one of those cases, maybe it even just doesn't matter if a test is unnecessarily skipped in 1.7... - Sebastian > Stefan > > ___ > cython-devel mailing list > cython-devel@python.org > https://mail.python.org/mailman/listinfo/cython-devel > ___ cython-devel mailing list cython-devel@python.org https://mail.python.org/mailman/listinfo/cython-devel
