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.

Sign up for GitHub

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

Jump to bottom

Closes #3757 ak.array gives unexpected results on a transposed numpy multi dimensional array #3761

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
+14 −0
Open

Closes #3757 ak.array gives unexpected results on a transposed numpy multi dimensional array #3761

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions arkouda/pdarraycreation.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -291,6 +291,8 @@ def array(
raise RuntimeError(
"Array exceeds allowed transfer size. Increase ak.client.maxTransferBytes to allow"
)
# Make a copy to avoid error #3757
a = a.copy()
Comment on lines +294 to +295
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we know why this fixes the error? I haven't thought about it much but if we don't have to create a copy, that's prob better right?

Copy link
Member

@stress-tess stress-tess Sep 18, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I mean, I guess it's not that a big of a deal bc if it was passed into ak.array was relatively small

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yah it's exactly what I thought. when numpy does a transpose, it doesn't actually move any data around, it just takes the same base array in memory and switches it from column_major to row_major or vice versa (I forget which one C used vs Fortran)

>>> nda.flags
  C_CONTIGUOUS : True
  F_CONTIGUOUS : False
  OWNDATA : True
  WRITEABLE : True
  ALIGNED : True
  WRITEBACKIFCOPY : False

>>> npa = np.transpose(nda)

>>> npa.flags
  C_CONTIGUOUS : False
  F_CONTIGUOUS : True
  OWNDATA : False
  WRITEABLE : True
  ALIGNED : True
  WRITEBACKIFCOPY : False

>>> npa.copy().flags
  C_CONTIGUOUS : True
  F_CONTIGUOUS : False
  OWNDATA : True
  WRITEABLE : True
  ALIGNED : True
  WRITEBACKIFCOPY : False

When we do a deep copy, it actually allocates new memory and interprets it correctly. We don't have support for different orders with the current impl. We used to have it with arrayview but that had it's own mess of problems. I guess we could try to check if the OWNDATA flag is false and then deep copy but idk if it's worth it

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think I'll just reference this comment in the issue for future reference and approve for now. I'll leave this unresolved so other reviewers can see it bc I think it kinda interesting. And i'd like to get other opinions

# Pack binary array data into a bytes object with a command header
# including the dtype and size. If the server has a different byteorder
# than our numpy array we need to swap to match since the server expects
Expand Down
12 changes: 12 additions & 0 deletions tests/pdarray_creation_test.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
import pytest

import arkouda as ak
from arkouda.testing import assert_arkouda_array_equal

INT_SCALARS = list(ak.dtypes.int_scalars.__args__)
NUMERIC_SCALARS = list(ak.dtypes.numeric_scalars.__args__)
Expand Down Expand Up @@ -104,6 +105,17 @@ def test_array_creation_misc(self):
with pytest.raises(TypeError):
ak.array(list(list(0)))

@pytest.mark.skip_if_max_rank_less_than(2)
def test_array_creation_transpose_bug_reproducer(self):

import numpy as np

rows = 5
cols = 5
nda = np.random.randint(1, 10, (rows, cols))

assert_arkouda_array_equal(ak.transpose(ak.array(nda)), ak.array(np.transpose(nda)))

def test_infer_shape_from_size(self):
from arkouda.util import _infer_shape_from_size

Expand Down
Loading