Skip to content

BUG: Array copy does not zero out gaps in structured dtypes #10789

Closed
Closed
BUG: Array copy does not zero out gaps in structured dtypes#10789
Labels
57 - Close?Issues which may be closable unless discussion continuedcomponent: numpy.dtype

Description

@ikehall
ikehall
opened on Mar 23, 2018

Recently updated from numpy 1.11.0 to 1.14.0 and some of my code was immediately broken.

You can simulate the broken bits through the following

my_metadata_dtype = np.dtype({'names':['Thing1','Thing2'], 
                             'offsets':[22, 40], 
                             'formats':['>u2','>i4'], 
                             'itemsize'=256})
list_of_arrays = [np.arange(i, i+128, dtype='>i2') for i in range(5)]
array_of_metadata = np.array(list_of_arrays, dtype=my_metadata_dtype)
other_array_of_metadata = np.array(list_of_arrays)
other_array_of_metadata.dtype = my_metadata_dtype

One would expect that array_of_metadata and other_array_of_metadata should be the same thing.
They are not. I submit that the line creating array_of_metadata is buggy. I believe this produced the same as the lines that produce other_array_of_metadata in previous versions.

But that wasn't even the primary bug I was going to talk about. I discovered that one while hunting down this one.

Take our other_array_of_metadata:
Let's make a new array from a subset of this array (not just a view)

sub_array = np.array(other_array_of_metadata[2:4])
sub_array.dtype = '>i2'

You will see that our sub_array now has strange things in the spaces that were not exposed by our dtype. Not the nice aranges we had constructed before. In 1.11, this code would have preserved the data that was not exposed. Now it appears that the unexposed portions of sub-array are never initialized to anything. This matters because sometimes we want to only expose a part of our metadata to our code, but when we write the metadata back out to disk, want all of it preserved.

I can understand making a design choice to not initialize the unexposed portions of a dtype when constructing new elements from literals or from scratch. But when you already have an element of this dtype, ignoring the empty space seems like an inferior choice.

Activity

  1. ahaldane commented on Mar 23, 2018

    @ahaldane
    ahaldane
    on Mar 23, 2018
    Member

    I think this is the same as #10752, see the discussion there. We want to drop support of np.array(arange(10)) behaving like a view when using structured dtypes, since it has led to other bugs. It should behave like astype, like in other cases.

    Since change this seems to break multiple people's code though, so maybe we should consider supporting this "view-like" behavior a little longer, but I really think we should remove it eventually. Let me investigate if we can support np.array(arange(10)) style creation, with a deprecation warning, for 1.14.3.

  2. ahaldane commented on Mar 23, 2018

    @ahaldane
    ahaldane
    on Mar 23, 2018
    Member

    Also, just to explain our thinking :

    When you do array_of_metadata = np.array(list_of_arrays, dtype=my_metadata_dtype), in 1.14 we want to make this behave as if .astype(dtype) was called on every element of the input arrays. This is how most of numpy behaves, for instance np.array([np.arange(2), np.arange(2)], dtype='f8') casts all the int values to create a 2d float64 array with values 0. and 1.. In 1.13, structured types were the exception and reinterpreted the memeory, due to what can pretty fairly be called a bug, see this list of bugs in the issue linked in my last comment.

    On the other hand when you do other_array_of_metadata.dtype = my_metadata_dtype, that is equivalent to other_array_of_metadata.view(my_metadata_dtype). In numpy views reinterpret the underlying memory with a new dtype (ie, it does not cast), in contrast to .astype, which casts.

    In your second example with sub_array you are effectively usign astype in the first line, and then using a view in the second line.

    So I think you examples are what I, at least, think the future behavior should be. However, I am unhappy that we (I) caused people's code to break without warning.

  3. ikehall commented on Mar 23, 2018

    @ikehall
    ikehall
    on Mar 23, 2018
    Author

    I think I understand your logic for the decision that was made, however I would argue that the current behavior now is for np.array() to behave like assignment into an array rather than like a copy to an array.

    Perhaps a better solution is to move this view-like copying to a new function. Currently I am getting around this new behavior to get view-like-copying with the following.

    def array_copy(array_or_view):
    return np.frombuffer(array_or_view.tostring(), dtype=array_or_view.dtype)

    I'm not the biggest fan of this, but it works.

  4. ahaldane commented on Mar 23, 2018

    @ahaldane
    ahaldane
    on Mar 23, 2018
    Member

    Wait a second.. am I confusing something? I just tried your code in 1.11.0 (and 1.11.1) and I get the same behavior as in 1.14.2. Here's a trimmed version of your example:

    >>> import numpy as np
>>> np.__version__
'1.11.1'
>>> my_metadata_dtype = np.dtype({'names':['Thing1','Thing2'], 
...                              'offsets':[2, 12], 
...                              'formats':['>u2','>i4'], 
...                              'itemsize': 22})
>>> list_of_arrays = [np.arange(i, i+22, dtype='>i2') for i in range(2)]
>>> a1 = np.array(list_of_arrays, dtype=my_metadata_dtype)
>>> a2 = np.array(list_of_arrays)
>>> a2.dtype = my_metadata_dtype
>>> 
>>> a1
array([[(0, 0), (1, 1), (2, 2), (3, 3), (4, 4), (5, 5), (6, 6), (7, 7),
        (8, 8), (9, 9), (10, 10), (11, 11), (12, 12), (13, 13), (14, 14),
        (15, 15), (16, 16), (17, 17), (18, 18), (19, 19), (20, 20),
        (21, 21)],
       [(1, 1), (2, 2), (3, 3), (4, 4), (5, 5), (6, 6), (7, 7), (8, 8),
        (9, 9), (10, 10), (11, 11), (12, 12), (13, 13), (14, 14), (15, 15),
        (16, 16), (17, 17), (18, 18), (19, 19), (20, 20), (21, 21),
        (22, 22)]], 
      dtype={'names':['Thing1','Thing2'], 'formats':['>u2','>i4'], 'offsets':[2,12], 'itemsize':22})
>>> a2
array([[(256, 100665088), (3072, 285217280)],
       [(512, 117442560), (3328, 301994752)]], 
      dtype={'names':['Thing1','Thing2'], 'formats':['>u2','>i4'], 'offsets':[2,12], 'itemsize':22})
>>>

    I get the same output in 1.14.2. What is your output?

  5. ikehall commented on Mar 23, 2018

    @ikehall
    ikehall
    on Mar 23, 2018
    Author

    Unfortunately I do not have an easy way of reverting back to 1.11.0, so I can't verify, but, for that part of the code (my first example), I was going on what I thought was the behavior back then...perhaps I was wrong. Sorry.

    It was the second example that broke my actual code, and trying to determine what the new behavior was led me to the first example which I apparently erroneously thought was changed.

  6. eric-wieser commented on Mar 23, 2018

    @eric-wieser
    eric-wieser
    on Mar 23, 2018
    Member

    I don't understand your array_copy function - it looks like it's just .copy() to me

  7. ikehall commented on Mar 23, 2018

    @ikehall
    ikehall
    on Mar 23, 2018
    Author

    The difference is that array_copy is faithful to the voids in my dtype whereas copy ignores them.

    my_metadata_dtype = np.dtype({'names':['Thing1','Thing2'], 
                             'offsets':[22, 40], 
                             'formats':['>u2','>i4'], 
                             'itemsize':256})
list_of_arrays = [np.arange(i, i+128, dtype='>i2') for i in range(5)]
other_array_of_metadata = np.array(list_of_arrays, dtype='>i2')
other_array_of_metadata.dtype = my_metadata_dtype

subarray_copy = other_array_of_metadata[2:4].copy()
subarray_array_copy = array_copy(other_array_of_metadata[2:4])

subarray_copy.dtype='>i2'
subarray_array_copy.dtype='>i2'

    Compare those last two arrays. subarray_array_copy still has the underlying arange with which it was created whereas subarray_copy does not.

  8. ikehall commented on Mar 23, 2018

    @ikehall
    ikehall
    on Mar 23, 2018
    Author

    After some more poking around though, my array_copy does not necessarily respect shape like I would want it to.

    Now it is:

    def array_copy(array_or_view):
    return np.frombuffer(array_or_view.tostring(), 
                         dtype=array_or_view.dtype).reshape(array_or_view.shape)
  9. mattip
    added
    57 - Close?Issues which may be closable unless discussion continued
    on Apr 18, 2018

  10. mattip commented on Aug 8, 2018

    @mattip
    mattip
    on Aug 8, 2018
    Member

    @ikehall is this still relevant? Could you include a valid code snippet?

  11. mattip
    added
    component: numpy.dtype
    on Aug 10, 2018

  12. mattip commented on Jan 16, 2019

    @mattip
    mattip
    on Jan 16, 2019
    Member

    If I understand correctly there are two issues here:

    • did we break old code with no warning
    • is the new behaviour correct

    In both cases, I think the ship has sailed. @ikehall is there anything more we can do here?

  13. ikehall commented on Jan 18, 2019

    @ikehall
    ikehall
    on Jan 18, 2019
    Author

    There was old code that broke. I can get (and really already have) onboard with dropping the support for view-like behavior with structured arrays by using the array_copy() method I posted above in place of array().

    A better solution might be to make .copy() behave like array_copy(), so that voids in a structured dtype are copied byte for byte into the new array rather than left uninitialized.

    ex:

    def array_copy(array_or_view):
    return np.frombuffer(array_or_view.tostring(), 
                         dtype=array_or_view.dtype).reshape(array_or_view.shape)

my_dtype = np.dtype({'names':['Thing1','Thing2'], 
                              'offsets':[22, 40], 
                              'formats':['>u2','>i4'], 
                              'itemsize':256})

a = np.zeros(2, my_dtype)
b = np.copy(a)
c = array_copy(a)
a.dtype = '>u2'
b.dtype = '>u2'
c.dtype = '>u2'

    You will note that b looks substantially different than a or c because the voids in the dtype are left uninitialized. This is really the issue that broke my old code. Previously I had just used array() in place of copy, and that had worked for me in the past. I fixed the broken old code with array_copy() and moved on. But I think it might be a good idea to make .copy() behave this way too. I can understand why using array() was not a great idea though.

  14. mattip
    changed the title [-]Something broken with structured arrays[/-] [+]BUG: Array copy does not zero out gaps in structured dtypes[/+] on Feb 14, 2019

  15. mattip commented on Feb 14, 2019

    @mattip
    mattip
    on Feb 14, 2019
    Member

    I changed the title to reflect the remaining issue: when using np.copy, gaps in the data due to offsets are not set to 0. I think this fits in with the general behavior of numpy. See for instance the warnings around the use of numpy.where.

    Perhaps an addition to the documentation of np.copy could make this clearer

  16. seberg commented on Feb 14, 2019

    @seberg
    seberg
    on Feb 14, 2019
    Member

    Just to crossref gh-12686 is somewhat related (possibly a duplicate even).

  17. seberg commented on Oct 4, 2021

    @seberg
    seberg
    on Oct 4, 2021
    Member

    I am closing this, as per Matti's last comment, this is probably the desired behaviour, and it is the behaviour now for many years so there is no point in thinking about a regression. Happy if open new issues about this or related problems though (or reopen this one)!

  18. seberg
    closed this as completedon Oct 4, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    57 - Close?Issues which may be closable unless discussion continuedcomponent: numpy.dtype

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions