Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Wrong striding used when assigning to multidimensional memory view slice #2941

Open
jmd-dk opened this issue Apr 30, 2019 · 2 comments

Comments

Projects
None yet
2 participants
@jmd-dk
Copy link

commented Apr 30, 2019

I find that assigning a single value to all elements of a multidimensional memory view slice fails, unless the slice is taken along the first dimension. Example:

import numpy as np

def bug():
    cdef int[:, ::1] a
    a = 2*np.ones((2, 2), dtype=np.intc)
    a[:, :1] = 1
    print(np.asarray(a))

When compiled and run, this prints the array [[1, 1], [2, 2]], though it should print [[1, 2], [1, 2]]. The correct behavior is seen if we remove the cdef statement.

The same error is seen for memory views of larger dimension. Assignment to slices of all but the first dimension fails in this way. If I leave out the contiguousity on the declaration (i.e. use int[:, :]) it works.

Surely this must be a rather critical bug? I couldn't find quite this usage documented, as here either all or just a single element is assigned to. If you are not supposed to do arbitrary slice assignments with memory views, I would think that the cythonization should error out or at least emit a warning.

Tested on Python 3.7.3 with Cython 0.29.7, Python 3.7.1 with Cython 0.29.2, Python 3.5.2 with Cython 0.23.4 and Python 2.7.12 with Cython 0.23.4, all on Linux Mint 18.3 and using gcc 5.4.0.

@jmd-dk

This comment has been minimized.

Copy link
Author

commented May 8, 2019

I realise now that the probable cause is that though a is C-contiguous, the slice a[:, :1] is not. If Cython treats it as so due to the cdef int[:, ::1] a, we have the observed trouble.

Is this the expected / wanted behavior? In general Cython cannot know the shape of such a slice at compile time, so shouldn't it just emit code that deals with the general case? I.e. effectively emit code that transforms a[:, :1] = 1 to something like

    for i in range(a.shape[0]):
        for j in range(1):
            a[i, j] = 1

?

@rjtobin

This comment has been minimized.

Copy link
Contributor

commented May 18, 2019

I made a PR (see above) that attempts to address this. It works for the minimal example you posted. If you have a more general use-case in mind, I'd appreciate if you could test the fix and provide any feedback.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.