🏆 Live Soccer TV: ray project/ray/pull/

scottjlee · 2023-02-01T00:12:38Z

Signed-off-by: Scott Lee sjl@anyscale.com

Why are these changes needed?

Add support for creating ArrowTensorArrays and ArrowVariableShapedTensorArray with string typed columns. The previous PR #31817 had CI test failures which were not run at PR-review time. This PR replicates the functionality of the previous PR, and additionally addresses the test failures (which only occur for Arrow 8.0+).

Related issue number

Based off of PR #31817, with additional implementation on top. I have marked new sections of code with GitHub comments.

Checks

I've signed off every commit(by using the -s flag, i.e., git commit -s) in this PR.
I've run scripts/format.sh to lint the changes in this PR.
I've included any doc changes needed for https://docs.ray.io/en/master/.
I've made sure the tests are passing. Note that there might be a few flaky tests, see the recent failures at https://flakey-tests.ray.io/
Testing Strategy
- Unit tests
- Release tests
- This PR is not tested :(

Signed-off-by: Scott Lee <sjl@anyscale.com>

scottjlee · 2023-02-01T00:38:26Z

+            raw_values = data.values
+            offset = raw_values.offset
+            if pa.types.is_boolean(raw_values.type):
+                # Arrow boolean array buffers are bit-packed, with 8 entries per byte,
+                # and are accessed via bit offsets.
+                buffer_item_width = raw_values.type.bit_width
+            else:
+                # We assume all other array types are accessed via byte array
+                # offsets.
+                buffer_item_width = raw_values.type.bit_width // 8
+            data_buffer = raw_values.buffers()[1]
+            data_offset = buffer_item_width * offset
+
+            if pa.types.is_boolean(raw_values.type):
+                # Special handling for boolean arrays, since Arrow
+                # bit-packs boolean arrays while NumPy does not.
+                # Cast as uint8 array and let NumPy unpack into a boolean view.
+                # Offset into uint8 array, where each element is
+                # a bucket for 8 booleans.
+                byte_bucket_offset = data_offset // 8
+                # Offset for a specific boolean, within a uint8 array element.
+                bool_offset = data_offset % 8
+                # The number of uint8 array elements (buckets) that our slice spans.
+                # Note that, due to the offset for a specific boolean,
+                # the slice can span byte boundaries even if it contains
+                # less than 8 booleans.
+                num_boolean_byte_buckets = 1 + ((bool_offset + np.prod(shape) - 1) // 8)
+                # Construct the uint8 array view on the buffer.
+                arr = np.ndarray(
+                    (num_boolean_byte_buckets,),
+                    dtype=np.uint8,
+                    buffer=data_buffer,
+                    offset=byte_bucket_offset,
+                )
+                # Unpack into a byte per boolean, using LSB bit-packed ordering.
+                arr = np.unpackbits(arr, bitorder="little")
+                # Interpret buffer as boolean array.
+                return np.ndarray(shape, dtype=np.bool_, buffer=arr, offset=bool_offset)
+
+            # Special handling of ragged string tensors
+            if pa.types.is_fixed_size_binary(raw_values.type):
+                offset = raw_values.offset
+                buffer_item_width = raw_values.type.bit_width // 8
+                ext_dtype = np.dtype(
+                    f"<U{raw_values.type.byte_width // NUM_BYTES_PER_UNICODE_CHAR}"
+                )
+                data_offset = buffer_item_width * offset
+                data_buffer = raw_values.buffers()[1]
+                return np.ndarray(
+                    shape, dtype=ext_dtype, buffer=data_buffer, offset=data_offset
+                )
+            ext_dtype = raw_values.type.to_pandas_dtype()
+            return np.ndarray(
+                shape, dtype=ext_dtype, buffer=data_buffer, offset=data_offset
+            )


Part 2 of new code from this PR (as opposed to changes from previous PR)

scottjlee · 2023-02-01T00:38:43Z

-            # TODO(Clark): Construct ndarray view directly on tensor element buffer to
-            # ensure reliable zero-copy semantics.
-            flat_ndarray = scalar.value.values.to_numpy(zero_copy_only=False)
-            return flat_ndarray.reshape(self.shape)
+            shape = scalar.type.shape
+            raw_values = scalar.value.values
+            offset = raw_values.offset
+            if pa.types.is_boolean(raw_values.type):
+                # Arrow boolean array buffers are bit-packed, with 8 entries per byte,
+                # and are accessed via bit offsets.
+                buffer_item_width = raw_values.type.bit_width
+            else:
+                # We assume all other array types are accessed via byte array
+                # offsets.
+                buffer_item_width = raw_values.type.bit_width // 8
+            data_buffer = raw_values.buffers()[1]
+            data_offset = buffer_item_width * offset
+            if pa.types.is_boolean(raw_values.type):
+                # Special handling for boolean arrays, since Arrow bit-packs
+                # boolean arrays while NumPy does not.
+                # Cast as uint8 array and let NumPy unpack into a boolean view.
+                # Offset into uint8 array, where each element is
+                # a bucket for 8 booleans.
+                byte_bucket_offset = offset // 8
+                # Offset for a specific boolean, within a uint8 array element.
+                bool_offset = offset % 8
+                # The number of uint8 array elements (buckets) that our slice spans.
+                # Note that, due to the offset for a specific boolean,
+                # the slice can span byte boundaries even if it contains
+                # less than 8 booleans.
+                num_boolean_byte_buckets = 1 + ((bool_offset + np.prod(shape) - 1) // 8)
+                # Construct the uint8 array view on the buffer.
+                arr = np.ndarray(
+                    (num_boolean_byte_buckets,),
+                    dtype=np.uint8,
+                    buffer=data_buffer,
+                    offset=byte_bucket_offset,
+                )
+                # Unpack into a byte per boolean, using LSB bit-packed ordering.
+                arr = np.unpackbits(arr, bitorder="little")
+                # Interpret buffer as boolean array.
+                return np.ndarray(shape, dtype=np.bool_, buffer=arr, offset=bool_offset)
+            # Special handling of ragged string tensors
+            if pa.types.is_fixed_size_binary(raw_values.type):
+                offset = raw_values.offset
+                buffer_item_width = raw_values.type.bit_width // 8
+                ext_dtype = np.dtype(
+                    f"<U{raw_values.type.byte_width // NUM_BYTES_PER_UNICODE_CHAR}"
+                )
+                data_offset = buffer_item_width * offset
+                data_buffer = raw_values.buffers()[1]
+                return np.ndarray(
+                    shape, dtype=ext_dtype, buffer=data_buffer, offset=data_offset
+                )
+            ext_dtype = raw_values.type.to_pandas_dtype()
+            return np.ndarray(
+                shape, dtype=ext_dtype, buffer=data_buffer, offset=data_offset
+            )


Part 1 of new code from this PR (as opposed to changes from previous PR)

scottjlee · 2023-02-01T00:47:48Z

        "int64": np.int64,
        "int64_list": object,
-        "float": np.float,
+        "float": np.float_,


This came up when I was testing locally with Arrow 8.0+. np.float is deprecated starting numpy 1.20.0, so I have replaced it with np.float_.

clarkzinzow · 2023-02-01T02:46:08Z

+                offset = raw_values.offset
+                buffer_item_width = raw_values.type.bit_width // 8


Duplicated definitions of above

Suggested change

offset = raw_values.offset

buffer_item_width = raw_values.type.bit_width // 8

clarkzinzow · 2023-02-01T02:46:24Z

+                data_offset = buffer_item_width * offset
+                data_buffer = raw_values.buffers()[1]


Same here

Suggested change

data_offset = buffer_item_width * offset

data_buffer = raw_values.buffers()[1]

clarkzinzow · 2023-02-01T02:47:15Z

+                return np.ndarray(
+                    shape, dtype=ext_dtype, buffer=data_buffer, offset=data_offset
+                )


Actually, could we fallthrough to the below np.ndarray() constructor after overriding ext_dtype? Should just need to move the below ext_dtype to the top of this function to let the string override take place.

Suggested change

return np.ndarray(

shape, dtype=ext_dtype, buffer=data_buffer, offset=data_offset

)

clarkzinzow · 2023-02-01T02:47:40Z

+            if pa.types.is_fixed_size_binary(raw_values.type):
+                offset = raw_values.offset
+                buffer_item_width = raw_values.type.bit_width // 8
+                ext_dtype = np.dtype(
+                    f"<U{raw_values.type.byte_width // NUM_BYTES_PER_UNICODE_CHAR}"
+                )
+                data_offset = buffer_item_width * offset
+                data_buffer = raw_values.buffers()[1]
+                return np.ndarray(
+                    shape, dtype=ext_dtype, buffer=data_buffer, offset=data_offset
+                )


Same here

Suggested change

if pa.types.is_fixed_size_binary(raw_values.type):

offset = raw_values.offset

buffer_item_width = raw_values.type.bit_width // 8

ext_dtype = np.dtype(

f"<U{raw_values.type.byte_width // NUM_BYTES_PER_UNICODE_CHAR}"

)

data_offset = buffer_item_width * offset

data_buffer = raw_values.buffers()[1]

return np.ndarray(

shape, dtype=ext_dtype, buffer=data_buffer, offset=data_offset

)

if pa.types.is_fixed_size_binary(raw_values.type):

ext_dtype = np.dtype(

f"<U{raw_values.type.byte_width // NUM_BYTES_PER_UNICODE_CHAR}"

)

Signed-off-by: Scott Lee <sjl@anyscale.com>

clarkzinzow · 2023-02-01T03:09:31Z

+                )
+            return np.ndarray(
+                shape, dtype=ext_dtype, buffer=data_buffer, offset=data_offset
+            )


Btw, it appears that this and ArrowVariableShapedTensorType._extension_scalar_to_ndarray() are the same after the definitions of shape and raw_values, could we consolidate most of this logic into a single utility function that lives at the bottom of this module?

I generalized ArrowTensorType._extension_scalar_to_ndarray as well (which had a few variables that were slightly different). LMK what you think

Signed-off-by: Scott Lee <sjl@anyscale.com>

clarkzinzow

Awesome, great consolidation of that common ndarray building code! It looks like we could also subsume most of ArrowTensorArray._to_numpy() into that helper as well:

ray/python/ray/air/util/tensor_extensions/arrow.py

Lines 392 to 447 in f21a9e6

    
           if pa.types.is_boolean(value_type): 
        
               # Arrow boolean array buffers are bit-packed, with 8 entries per byte, 
        
               # and are accessed via bit offsets. 
        
               buffer_item_width = value_type.bit_width 
        
           else: 
        
               # We assume all other array types are accessed via byte array 
        
               # offsets. 
        
               buffer_item_width = value_type.bit_width // 8 
        
           # Number of items per inner ndarray. 
        
           num_items_per_element = np.prod(shape) if shape else 1 
        
           # Base offset into data buffer, e.g. due to zero-copy slice. 
        
           buffer_offset = self.offset * num_items_per_element 
        
           # Offset of array data in buffer. 
        
           offset = buffer_item_width * buffer_offset 
        
           if index is not None: 
        
               # Getting a single tensor element of the array. 
        
               offset_buffer = buffers[1] 
        
               offset_array = np.ndarray( 
        
                   (len(self),), buffer=offset_buffer, dtype=self.OFFSET_DTYPE 
        
               ) 
        
               # Offset into array to reach logical index. 
        
               index_offset = offset_array[index] 
        
               # Add the index offset to the base offset. 
        
               offset += buffer_item_width * index_offset 
        
           else: 
        
               # Getting the entire array of tensors. 
        
               shape = (len(self),) + shape 
        
           if pa.types.is_boolean(value_type): 
        
               # Special handling for boolean arrays, since Arrow bit-packs boolean arrays 
        
               # while NumPy does not. 
        
               # Cast as uint8 array and let NumPy unpack into a boolean view. 
        
               # Offset into uint8 array, where each element is a bucket for 8 booleans. 
        
               byte_bucket_offset = offset // 8 
        
               # Offset for a specific boolean, within a uint8 array element. 
        
               bool_offset = offset % 8 
        
               # The number of uint8 array elements (buckets) that our slice spans. 
        
               # Note that, due to the offset for a specific boolean, the slice can span 
        
               # byte boundaries even if it contains less than 8 booleans. 
        
               num_boolean_byte_buckets = 1 + ((bool_offset + np.prod(shape) - 1) // 8) 
        
               # Construct the uint8 array view on the buffer. 
        
               arr = np.ndarray( 
        
                   (num_boolean_byte_buckets,), 
        
                   dtype=np.uint8, 
        
                   buffer=data_buffer, 
        
                   offset=byte_bucket_offset, 
        
               ) 
        
               # Unpack into a byte per boolean, using LSB bit-packed ordering. 
        
               arr = np.unpackbits(arr, bitorder="little") 
        
               # Interpret buffer as boolean array. 
        
               return np.ndarray(shape, dtype=np.bool_, buffer=arr, offset=bool_offset) 
        
           # Special handling of binary/string types. Assumes unicode string tensor columns 
        
           if pa.types.is_fixed_size_binary(value_type): 
        
               ext_dtype = np.dtype( 
        
                   f"<U{value_type.byte_width // NUM_BYTES_PER_UNICODE_CHAR}" 
        
               ) 
        
           return np.ndarray(shape, dtype=ext_dtype, buffer=data_buffer, offset=offset)

scottjlee · 2023-02-02T00:07:47Z

Awesome, great consolidation of that common ndarray building code! It looks like we could also subsume most of ArrowTensorArray._to_numpy() into that helper as well:

ray/python/ray/air/util/tensor_extensions/arrow.py

Lines 392 to 447 in f21a9e6

if pa.types.is_boolean(value_type):

# Arrow boolean array buffers are bit-packed, with 8 entries per byte,

# and are accessed via bit offsets.

buffer_item_width = value_type.bit_width

else:

# We assume all other array types are accessed via byte array

# offsets.

buffer_item_width = value_type.bit_width // 8

# Number of items per inner ndarray.

num_items_per_element = np.prod(shape) if shape else 1

# Base offset into data buffer, e.g. due to zero-copy slice.

buffer_offset = self.offset * num_items_per_element

# Offset of array data in buffer.

offset = buffer_item_width * buffer_offset

if index is not None:

# Getting a single tensor element of the array.

offset_buffer = buffers[1]

offset_array = np.ndarray(

(len(self),), buffer=offset_buffer, dtype=self.OFFSET_DTYPE

)

# Offset into array to reach logical index.

index_offset = offset_array[index]

# Add the index offset to the base offset.

offset += buffer_item_width * index_offset

else:

# Getting the entire array of tensors.

shape = (len(self),) + shape

if pa.types.is_boolean(value_type):

# Special handling for boolean arrays, since Arrow bit-packs boolean arrays

# while NumPy does not.

# Cast as uint8 array and let NumPy unpack into a boolean view.

# Offset into uint8 array, where each element is a bucket for 8 booleans.

byte_bucket_offset = offset // 8

# Offset for a specific boolean, within a uint8 array element.

bool_offset = offset % 8

# The number of uint8 array elements (buckets) that our slice spans.

# Note that, due to the offset for a specific boolean, the slice can span

# byte boundaries even if it contains less than 8 booleans.

num_boolean_byte_buckets = 1 + ((bool_offset + np.prod(shape) - 1) // 8)

# Construct the uint8 array view on the buffer.

arr = np.ndarray(

(num_boolean_byte_buckets,),

dtype=np.uint8,

buffer=data_buffer,

offset=byte_bucket_offset,

)

# Unpack into a byte per boolean, using LSB bit-packed ordering.

arr = np.unpackbits(arr, bitorder="little")

# Interpret buffer as boolean array.

return np.ndarray(shape, dtype=np.bool_, buffer=arr, offset=bool_offset)

# Special handling of binary/string types. Assumes unicode string tensor columns

if pa.types.is_fixed_size_binary(value_type):

ext_dtype = np.dtype(

f"<U{value_type.byte_width // NUM_BYTES_PER_UNICODE_CHAR}"

)

return np.ndarray(shape, dtype=ext_dtype, buffer=data_buffer, offset=offset)

I was looking into this as well, but I think that method has some extra logic that's not present in the other methods, for handling the offsets for a single element:

ray/python/ray/air/util/tensor_extensions/arrow.py

Lines 406 to 415 in f21a9e6

    
           if index is not None: 
        
               # Getting a single tensor element of the array. 
        
               offset_buffer = buffers[1] 
        
               offset_array = np.ndarray( 
        
                   (len(self),), buffer=offset_buffer, dtype=self.OFFSET_DTYPE 
        
               ) 
        
               # Offset into array to reach logical index. 
        
               index_offset = offset_array[index] 
        
               # Add the index offset to the base offset. 
        
               offset += buffer_item_width * index_offset

I could further generalize the helper method to work for this, but I think it could be overgeneralized with this new addition. Thoughts? @clarkzinzow

clarkzinzow · 2023-02-02T02:52:25Z

@scottjlee Ah good point, I missed that on the skim! In that case, this looks good to merge!

…` and `ArrowVariableShapedTensorArray` (ray-project#32143) Add support for creating ArrowTensorArrays and ArrowVariableShapedTensorArray with string typed columns. The previous PR ray-project#31817 had CI test failures which were not run at PR-review time. This PR replicates the functionality of the previous PR, and additionally addresses the test failures (which only occur for Arrow 8.0+). Signed-off-by: Scott Lee <sjl@anyscale.com> Signed-off-by: Edward Oakes <ed.nmi.oakes@gmail.com>

Scott Lee added 2 commits January 31, 2023 16:06

add string type conversion logic in _extension_scalar_to_ndarray

d5cb25d

Signed-off-by: Scott Lee <sjl@anyscale.com>

format

11cf3c8

Signed-off-by: Scott Lee <sjl@anyscale.com>

scottjlee requested review from c21, clarkzinzow, ericl, jianoaix, jjyao and scv119 as code owners February 1, 2023 00:12

Scott Lee added 3 commits January 31, 2023 16:20

Merge branch 'master' into fix-string-tensor-tests

0d4650b

Signed-off-by: Scott Lee <sjl@anyscale.com>

clean up

6259267

Signed-off-by: Scott Lee <sjl@anyscale.com>

remove constant

bffcd3c

Signed-off-by: Scott Lee <sjl@anyscale.com>

scottjlee commented Feb 1, 2023

View reviewed changes

scottjlee assigned c21, clarkzinzow, amogkam, bveeramani and jianoaix Feb 1, 2023

clarkzinzow reviewed Feb 1, 2023

View reviewed changes

address comments

435f8fb

Signed-off-by: Scott Lee <sjl@anyscale.com>

scottjlee requested a review from clarkzinzow February 1, 2023 03:05

clarkzinzow reviewed Feb 1, 2023

View reviewed changes

refactor common ndarray generation logic

f21a9e6

Signed-off-by: Scott Lee <sjl@anyscale.com>

scottjlee requested a review from clarkzinzow February 1, 2023 04:05

clarkzinzow approved these changes Feb 1, 2023

View reviewed changes

clarkzinzow merged commit 74266a2 into ray-project:master Feb 2, 2023

Navigation Menu

Uh oh!

[Datasets] Add support for string tensor columns in `ArrowTensorArray` and `ArrowVariableShapedTensorArray` - #32143

[Datasets] Add support for string tensor columns in `ArrowTensorArray` and `ArrowVariableShapedTensorArray`#32143
clarkzinzow merged 7 commits into
ray-project:masterfrom
scottjlee:fix-string-tensor-tests

scottjlee commented Feb 1, 2023 •

edited

Loading

Uh oh!

scottjlee Feb 1, 2023 •

edited

Loading

Uh oh!

scottjlee Feb 1, 2023

Uh oh!

scottjlee Feb 1, 2023

Uh oh!

clarkzinzow Feb 1, 2023

Uh oh!

clarkzinzow Feb 1, 2023

Uh oh!

clarkzinzow Feb 1, 2023

Uh oh!

clarkzinzow Feb 1, 2023

Uh oh!

clarkzinzow Feb 1, 2023

Uh oh!

scottjlee Feb 1, 2023

Uh oh!

clarkzinzow left a comment

Uh oh!

scottjlee commented Feb 2, 2023

Uh oh!

clarkzinzow commented Feb 2, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

		offset = raw_values.offset
		buffer_item_width = raw_values.type.bit_width // 8

		data_offset = buffer_item_width * offset
		data_buffer = raw_values.buffers()[1]

	return np.ndarray(
	shape, dtype=ext_dtype, buffer=data_buffer, offset=data_offset
	)

	if pa.types.is_boolean(value_type):
	# Arrow boolean array buffers are bit-packed, with 8 entries per byte,
	# and are accessed via bit offsets.
	buffer_item_width = value_type.bit_width
	else:
	# We assume all other array types are accessed via byte array
	# offsets.
	buffer_item_width = value_type.bit_width // 8
	# Number of items per inner ndarray.
	num_items_per_element = np.prod(shape) if shape else 1
	# Base offset into data buffer, e.g. due to zero-copy slice.
	buffer_offset = self.offset * num_items_per_element
	# Offset of array data in buffer.
	offset = buffer_item_width * buffer_offset
	if index is not None:
	# Getting a single tensor element of the array.
	offset_buffer = buffers[1]
	offset_array = np.ndarray(
	(len(self),), buffer=offset_buffer, dtype=self.OFFSET_DTYPE
	)
	# Offset into array to reach logical index.
	index_offset = offset_array[index]
	# Add the index offset to the base offset.
	offset += buffer_item_width * index_offset
	else:
	# Getting the entire array of tensors.
	shape = (len(self),) + shape
	if pa.types.is_boolean(value_type):
	# Special handling for boolean arrays, since Arrow bit-packs boolean arrays
	# while NumPy does not.
	# Cast as uint8 array and let NumPy unpack into a boolean view.
	# Offset into uint8 array, where each element is a bucket for 8 booleans.
	byte_bucket_offset = offset // 8
	# Offset for a specific boolean, within a uint8 array element.
	bool_offset = offset % 8
	# The number of uint8 array elements (buckets) that our slice spans.
	# Note that, due to the offset for a specific boolean, the slice can span
	# byte boundaries even if it contains less than 8 booleans.
	num_boolean_byte_buckets = 1 + ((bool_offset + np.prod(shape) - 1) // 8)
	# Construct the uint8 array view on the buffer.
	arr = np.ndarray(
	(num_boolean_byte_buckets,),
	dtype=np.uint8,
	buffer=data_buffer,
	offset=byte_bucket_offset,
	)
	# Unpack into a byte per boolean, using LSB bit-packed ordering.
	arr = np.unpackbits(arr, bitorder="little")
	# Interpret buffer as boolean array.
	return np.ndarray(shape, dtype=np.bool_, buffer=arr, offset=bool_offset)
	# Special handling of binary/string types. Assumes unicode string tensor columns
	if pa.types.is_fixed_size_binary(value_type):
	ext_dtype = np.dtype(
	f"<U{value_type.byte_width // NUM_BYTES_PER_UNICODE_CHAR}"
	)
	return np.ndarray(shape, dtype=ext_dtype, buffer=data_buffer, offset=offset)

Uh oh!

Conversation

scottjlee commented Feb 1, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Why are these changes needed?

Related issue number

Checks

Uh oh!

scottjlee Feb 1, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

clarkzinzow left a comment

Choose a reason for hiding this comment

Uh oh!

scottjlee commented Feb 2, 2023

Uh oh!

clarkzinzow commented Feb 2, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

scottjlee commented Feb 1, 2023 •

edited

Loading

scottjlee Feb 1, 2023 •

edited

Loading