From 2f625787c4921f7a622723e0822b9b3721faee95 Mon Sep 17 00:00:00 2001 From: Sarah Gilmore Date: Wed, 4 Oct 2023 10:54:41 -0400 Subject: [PATCH 01/18] Add buffer.h header file --- .../cpp/arrow/matlab/buffer/proxy/buffer.h | 45 +++++++++++++++++++ 1 file changed, 45 insertions(+) create mode 100644 matlab/src/cpp/arrow/matlab/buffer/proxy/buffer.h diff --git a/matlab/src/cpp/arrow/matlab/buffer/proxy/buffer.h b/matlab/src/cpp/arrow/matlab/buffer/proxy/buffer.h new file mode 100644 index 0000000000000..3576e00a1038c --- /dev/null +++ b/matlab/src/cpp/arrow/matlab/buffer/proxy/buffer.h @@ -0,0 +1,45 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +#pragma once + +#include "arrow/arbufferray.h" + +#include "libmexclass/proxy/Proxy.h" + +namespace arrow::matlab::buffer::proxy { + +class Buffer : public libmexclass::proxy::Proxy { + public: + Buffer(std::shared_ptr buffer); + + ~Buffer() {} + + std::shared_ptr unwrap(); + + protected: + + void getNumBytes(libmexclass::proxy::method::Context& context); + + void toMATLAB(libmexclass::proxy::method::Context& context); + + void isEqual(libmexclass::proxy::method::Context& context); + + std::shared_ptr buffer; +}; + +} From 7c9ccd821767ddecd6c0ee151e3eadf2e6950700 Mon Sep 17 00:00:00 2001 From: Sarah Gilmore Date: Wed, 4 Oct 2023 10:55:47 -0400 Subject: [PATCH 02/18] Add Make static function to Buffer --- matlab/src/cpp/arrow/matlab/buffer/proxy/buffer.h | 3 +++ 1 file changed, 3 insertions(+) diff --git a/matlab/src/cpp/arrow/matlab/buffer/proxy/buffer.h b/matlab/src/cpp/arrow/matlab/buffer/proxy/buffer.h index 3576e00a1038c..3fb52422f72d8 100644 --- a/matlab/src/cpp/arrow/matlab/buffer/proxy/buffer.h +++ b/matlab/src/cpp/arrow/matlab/buffer/proxy/buffer.h @@ -31,6 +31,9 @@ class Buffer : public libmexclass::proxy::Proxy { std::shared_ptr unwrap(); + static libmexclass::proxy::MakeResult make(const libmexclass::proxy::FunctionArguments& constructor_arguments); + + protected: void getNumBytes(libmexclass::proxy::method::Context& context); From 5c70be655bd500a49416291fc189f484b5d75c34 Mon Sep 17 00:00:00 2001 From: Sarah Gilmore Date: Wed, 4 Oct 2023 11:46:55 -0400 Subject: [PATCH 03/18] Add implementaion for Buffer proxy C++ class --- .../cpp/arrow/matlab/buffer/proxy/buffer.cc | 92 +++++++++++++++++++ 1 file changed, 92 insertions(+) create mode 100644 matlab/src/cpp/arrow/matlab/buffer/proxy/buffer.cc diff --git a/matlab/src/cpp/arrow/matlab/buffer/proxy/buffer.cc b/matlab/src/cpp/arrow/matlab/buffer/proxy/buffer.cc new file mode 100644 index 0000000000000..e8e87bf53ac57 --- /dev/null +++ b/matlab/src/cpp/arrow/matlab/buffer/proxy/buffer.cc @@ -0,0 +1,92 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + + +#include "arrow/matlab/buffer/proxy/buffer.h" +#include "arrow/matlab/buffer/matlab_buffer.h" +#include "arrow/matlab/error/error.h" +#include "libmexclass/proxy/ProxyManager.h" + +namespace arrow::matlab::buffer::proxy { + + Buffer::Buffer(std::shared_ptr buffer) : buffer{std::move(buffer)} { + + REGISTER_METHOD(Buffer, getNumBytes); + REGISTER_METHOD(Buffer, isEqual); + REGISTER_METHOD(Buffer, toMATLAB); + } + + libmexclass::proxy::MakeResult Buffer::make(const libmexclass::proxy::FunctionArguments& constructor_arguments) { + namespace mda = ::matlab::data; + using BufferProxy = proxy::Buffer; + + mda::StructArray opts = constructor_arguments[0]; + const mda::TypedArray values_mda = opts[0]["Values"]; + auto buffer = std::make_shared(values_mda); + return std::make_shared(std::move(buffer)); + } + + void Buffer::getNumBytes(libmexclass::proxy::method::Context& context) { + namespace mda = ::matlab::data; + mda::ArrayFactory factory; + auto num_bytes_mda = factory.createScalar(buffer->size()); + context.outputs[0] = num_bytes_mda; + } + + void Buffer::isEqual(libmexclass::proxy::method::Context& context) { + namespace mda = ::matlab::data; + + bool is_equal = true; + const mda::TypedArray buffer_proxy_ids = context.inputs[0]; + for (const auto& buffer_proxy_id : buffer_proxy_ids) { + // Retrieve the Buffer proxy from the ProxyManager + auto proxy = libmexclass::proxy::ProxyManager::getProxy(buffer_proxy_id); + auto buffer_proxy = std::static_pointer_cast(proxy); + auto buffer_to_compare = buffer_proxy->unwrap(); + + if (!buffer->Equals(*buffer_to_compare)) { + is_equal = false; + break; + } + } + mda::ArrayFactory factory; + context.outputs[0] = factory.createScalar(is_equal); + } + + void Buffer::toMATLAB(libmexclass::proxy::method::Context& context) { + namespace mda = ::matlab::data; + + // If buffer is not backed by a cpu memory manager, calling its data() method + // may cause a crash. To avoid this, call ViewOrCopy, which tries to create a + // no-copy view of the buffer on the given memory manager device. If not possible, + // ViewOrCopy makes a copy of the buffer's contents. + + MATLAB_ASSIGN_OR_ERROR_WITH_CONTEXT( + auto cpu_buffer, + arrow::Buffer::ViewOrCopy(buffer, arrow::default_cpu_memory_manager(), + context, "invalid:buffer"); + + const auto* data_begin = cpu_buffer->data(); + const auto num_bytes = cpu_buffer->size(); + // data_begin is a uint8_t*, so num_bytes is equal to the number of elements + const auto* data_end = data_begin + num_bytes; + + mda::ArrayFactory factory; + context.outputs[0] = factory.createArray({num_bytes, 1}, data_begin, data_end); + } + +} \ No newline at end of file From ec94f69a81dc2927442a7b84035f717c0ccf977c Mon Sep 17 00:00:00 2001 From: Sarah Gilmore Date: Wed, 4 Oct 2023 11:59:48 -0400 Subject: [PATCH 04/18] Add buffer.cc to the CMake source list --- matlab/src/cpp/arrow/matlab/buffer/proxy/buffer.cc | 14 ++++++++++---- matlab/src/cpp/arrow/matlab/buffer/proxy/buffer.h | 2 +- matlab/tools/cmake/BuildMatlabArrowInterface.cmake | 4 +++- 3 files changed, 14 insertions(+), 6 deletions(-) diff --git a/matlab/src/cpp/arrow/matlab/buffer/proxy/buffer.cc b/matlab/src/cpp/arrow/matlab/buffer/proxy/buffer.cc index e8e87bf53ac57..e692d5a72c287 100644 --- a/matlab/src/cpp/arrow/matlab/buffer/proxy/buffer.cc +++ b/matlab/src/cpp/arrow/matlab/buffer/proxy/buffer.cc @@ -33,13 +33,18 @@ namespace arrow::matlab::buffer::proxy { libmexclass::proxy::MakeResult Buffer::make(const libmexclass::proxy::FunctionArguments& constructor_arguments) { namespace mda = ::matlab::data; using BufferProxy = proxy::Buffer; + using MatlabBuffer = arrow::matlab::buffer::MatlabBuffer; mda::StructArray opts = constructor_arguments[0]; const mda::TypedArray values_mda = opts[0]["Values"]; - auto buffer = std::make_shared(values_mda); + auto buffer = std::make_shared(values_mda); return std::make_shared(std::move(buffer)); } + std::shared_ptr Buffer::unwrap() { + return buffer; + } + void Buffer::getNumBytes(libmexclass::proxy::method::Context& context) { namespace mda = ::matlab::data; mda::ArrayFactory factory; @@ -77,8 +82,9 @@ namespace arrow::matlab::buffer::proxy { MATLAB_ASSIGN_OR_ERROR_WITH_CONTEXT( auto cpu_buffer, - arrow::Buffer::ViewOrCopy(buffer, arrow::default_cpu_memory_manager(), - context, "invalid:buffer"); + arrow::Buffer::ViewOrCopy(buffer, arrow::default_cpu_memory_manager()), + context, "invalid:buffer" + ); const auto* data_begin = cpu_buffer->data(); const auto num_bytes = cpu_buffer->size(); @@ -86,7 +92,7 @@ namespace arrow::matlab::buffer::proxy { const auto* data_end = data_begin + num_bytes; mda::ArrayFactory factory; - context.outputs[0] = factory.createArray({num_bytes, 1}, data_begin, data_end); + context.outputs[0] = factory.createArray({static_cast(num_bytes), 1}, data_begin, data_end); } } \ No newline at end of file diff --git a/matlab/src/cpp/arrow/matlab/buffer/proxy/buffer.h b/matlab/src/cpp/arrow/matlab/buffer/proxy/buffer.h index 3fb52422f72d8..efee6fa615d40 100644 --- a/matlab/src/cpp/arrow/matlab/buffer/proxy/buffer.h +++ b/matlab/src/cpp/arrow/matlab/buffer/proxy/buffer.h @@ -17,7 +17,7 @@ #pragma once -#include "arrow/arbufferray.h" +#include "arrow/buffer.h" #include "libmexclass/proxy/Proxy.h" diff --git a/matlab/tools/cmake/BuildMatlabArrowInterface.cmake b/matlab/tools/cmake/BuildMatlabArrowInterface.cmake index 149a688b27e15..50daab65324f4 100644 --- a/matlab/tools/cmake/BuildMatlabArrowInterface.cmake +++ b/matlab/tools/cmake/BuildMatlabArrowInterface.cmake @@ -73,7 +73,9 @@ set(MATLAB_ARROW_LIBMEXCLASS_CLIENT_PROXY_SOURCES "${CMAKE_SOURCE_DIR}/src/cpp/a "${CMAKE_SOURCE_DIR}/src/cpp/arrow/matlab/io/feather/proxy/reader.cc" "${CMAKE_SOURCE_DIR}/src/cpp/arrow/matlab/io/csv/proxy/table_writer.cc" "${CMAKE_SOURCE_DIR}/src/cpp/arrow/matlab/io/csv/proxy/table_reader.cc" - "${CMAKE_SOURCE_DIR}/src/cpp/arrow/matlab/index/validate.cc") + "${CMAKE_SOURCE_DIR}/src/cpp/arrow/matlab/index/validate.cc" + "${CMAKE_SOURCE_DIR}/src/cpp/arrow/matlab/buffer/proxy/buffer.cc") + set(MATLAB_ARROW_LIBMEXCLASS_CLIENT_PROXY_FACTORY_INCLUDE_DIR "${CMAKE_SOURCE_DIR}/src/cpp/arrow/matlab/proxy") set(MATLAB_ARROW_LIBMEXCLASS_CLIENT_PROXY_FACTORY_SOURCES "${CMAKE_SOURCE_DIR}/src/cpp/arrow/matlab/proxy/factory.cc") From ce33cef1067012bc5855799e0673f36f9e510ea3 Mon Sep 17 00:00:00 2001 From: Sarah Gilmore Date: Wed, 4 Oct 2023 12:01:22 -0400 Subject: [PATCH 05/18] Register Buffer proxy --- matlab/src/cpp/arrow/matlab/proxy/factory.cc | 2 ++ 1 file changed, 2 insertions(+) diff --git a/matlab/src/cpp/arrow/matlab/proxy/factory.cc b/matlab/src/cpp/arrow/matlab/proxy/factory.cc index 62ed84fedcf6a..d81e651ae0bf4 100644 --- a/matlab/src/cpp/arrow/matlab/proxy/factory.cc +++ b/matlab/src/cpp/arrow/matlab/proxy/factory.cc @@ -40,12 +40,14 @@ #include "arrow/matlab/io/feather/proxy/reader.h" #include "arrow/matlab/io/csv/proxy/table_writer.h" #include "arrow/matlab/io/csv/proxy/table_reader.h" +#include "arrow/matlab/buffer/proxy/buffer.h" #include "factory.h" namespace arrow::matlab::proxy { libmexclass::proxy::MakeResult Factory::make_proxy(const ClassName& class_name, const FunctionArguments& constructor_arguments) { + REGISTER_PROXY(arrow.buffer.Buffer , arrow::matlab::buffer::proxy::Buffer); REGISTER_PROXY(arrow.array.proxy.Float32Array , arrow::matlab::array::proxy::NumericArray); REGISTER_PROXY(arrow.array.proxy.Float64Array , arrow::matlab::array::proxy::NumericArray); REGISTER_PROXY(arrow.array.proxy.UInt8Array , arrow::matlab::array::proxy::NumericArray); From f8156eeb322773ad3f35b17cfbc70653fb6be245 Mon Sep 17 00:00:00 2001 From: Sarah Gilmore Date: Wed, 4 Oct 2023 14:05:55 -0400 Subject: [PATCH 06/18] Add arrow.buffer.Buffer MATLAB class --- matlab/src/cpp/arrow/matlab/proxy/factory.cc | 2 +- matlab/src/matlab/+arrow/+buffer/Buffer.m | 81 ++++++++++++++++++++ 2 files changed, 82 insertions(+), 1 deletion(-) create mode 100644 matlab/src/matlab/+arrow/+buffer/Buffer.m diff --git a/matlab/src/cpp/arrow/matlab/proxy/factory.cc b/matlab/src/cpp/arrow/matlab/proxy/factory.cc index d81e651ae0bf4..a22344a214032 100644 --- a/matlab/src/cpp/arrow/matlab/proxy/factory.cc +++ b/matlab/src/cpp/arrow/matlab/proxy/factory.cc @@ -47,7 +47,7 @@ namespace arrow::matlab::proxy { libmexclass::proxy::MakeResult Factory::make_proxy(const ClassName& class_name, const FunctionArguments& constructor_arguments) { - REGISTER_PROXY(arrow.buffer.Buffer , arrow::matlab::buffer::proxy::Buffer); + REGISTER_PROXY(arrow.buffer.proxy.Buffer , arrow::matlab::buffer::proxy::Buffer); REGISTER_PROXY(arrow.array.proxy.Float32Array , arrow::matlab::array::proxy::NumericArray); REGISTER_PROXY(arrow.array.proxy.Float64Array , arrow::matlab::array::proxy::NumericArray); REGISTER_PROXY(arrow.array.proxy.UInt8Array , arrow::matlab::array::proxy::NumericArray); diff --git a/matlab/src/matlab/+arrow/+buffer/Buffer.m b/matlab/src/matlab/+arrow/+buffer/Buffer.m new file mode 100644 index 0000000000000..4e39a1ed5bd31 --- /dev/null +++ b/matlab/src/matlab/+arrow/+buffer/Buffer.m @@ -0,0 +1,81 @@ +%BUFFER Represents a block of contiguous memory with a specified size. + +% Licensed to the Apache Software Foundation (ASF) under one or more +% contributor license agreements. See the NOTICE file distributed with +% this work for additional information regarding copyright ownership. +% The ASF licenses this file to you under the Apache License, Version +% 2.0 (the "License"); you may not use this file except in compliance +% with the License. You may obtain a copy of the License at +% +% http://www.apache.org/licenses/LICENSE-2.0 +% +% Unless required by applicable law or agreed to in writing, software +% distributed under the License is distributed on an "AS IS" BASIS, +% WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or +% implied. See the License for the specific language governing +% permissions and limitations under the License. + +classdef Buffer < matlab.mixin.Scalar + + properties (Dependent, GetAccess=public, SetAccess=private) + NumBytes + end + + properties (Hidden, GetAccess=public, SetAccess=private) + Proxy + end + + methods + function obj = Buffer(proxy) + arguments + proxy(1, 1) libmexclass.proxy.Proxy {validate(proxy, "arrow.buffer.proxy.Buffer")} + end + import arrow.internal.proxy.validate + obj.Proxy = proxy; + end + + function numBytes = get.NumBytes(obj) + numBytes = obj.Proxy.getNumBytes(); + end + + function data = toMATLAB(obj) + data = obj.Proxy.toMATLAB(); + end + + function tf = isequal(obj, varargin) + narginchk(2, inf); + tf = false; + + bufferProxyIDs = zeros([1 numel(varargin)], "uint64"); + for ii = 2:numel(varargin) + maybeBuffer = varargin{ii}; + if ~isa(maybeBuffer, "arrow.buffer.Buffer") + % If maybeBuffer is not an instance of + % arrow.buffer.Buffer, return false early. + return; + end + % Otherwise, extract the proxy ID associated with + % maybeBuffer. + bufferProxyIDs(ii) = maybeBuffer.Proxy.ID; + end + + % Invoke the isEqual method on the Buffer Proxy class. + tf = obj.Proxy.isEqual(bufferProxyIDs); + end + end + + methods (Static, Hidden) + function buffer = fromMATLAB(data) + arguments + data(:, 1) {mustBeNumeric} + end + + % Re-interpret bit pattern as uint8s without changing the + % underlying data. + data = typecast(data, "uint8"); + args = struct(Values=data); + proxy = arrow.internal.proxy.create("arrow.buffer.proxy.Buffer", args); + buffer = arrow.buffer.Buffer(proxy); + end + end +end \ No newline at end of file From 0be93be17d71828145fc7aa158ee376db3a60b25 Mon Sep 17 00:00:00 2001 From: Sarah Gilmore Date: Wed, 4 Oct 2023 14:41:33 -0400 Subject: [PATCH 07/18] Add unit tests for arrow.buffer.Buffer --- matlab/test/arrow/buffer/tBuffer.m | 141 +++++++++++++++++++++++++++++ 1 file changed, 141 insertions(+) create mode 100644 matlab/test/arrow/buffer/tBuffer.m diff --git a/matlab/test/arrow/buffer/tBuffer.m b/matlab/test/arrow/buffer/tBuffer.m new file mode 100644 index 0000000000000..c4c16fd2e9088 --- /dev/null +++ b/matlab/test/arrow/buffer/tBuffer.m @@ -0,0 +1,141 @@ +%TBUFFER Unit tests for arrow.buffer.Buffer + +% Licensed to the Apache Software Foundation (ASF) under one or more +% contributor license agreements. See the NOTICE file distributed with +% this work for additional information regarding copyright ownership. +% The ASF licenses this file to you under the Apache License, Version +% 2.0 (the "License"); you may not use this file except in compliance +% with the License. You may obtain a copy of the License at +% +% http://www.apache.org/licenses/LICENSE-2.0 +% +% Unless required by applicable law or agreed to in writing, software +% distributed under the License is distributed on an "AS IS" BASIS, +% WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or +% implied. See the License for the specific language governing +% permissions and limitations under the License. + +classdef tBuffer < matlab.unittest.TestCase + + methods (Test) + function Smoke(testCase) + import arrow.buffer.Buffer + + source = uint8([1 2 3]); + buffer = Buffer.fromMATLAB(source); + testCase.verifyInstanceOf(buffer, "arrow.buffer.Buffer"); + end + + function fromMATLABUint8Scalar(testCase) + % Verifies fromMATLAB returns the expected arrow.buffer.Buffer + % instance when given a scalar uint8 value. + + import arrow.buffer.Buffer + + source = uint8(20); + buffer = Buffer.fromMATLAB(source); + testCase.verifyEqual(buffer.NumBytes, int64(1)); + values = toMATLAB(buffer); + testCase.verifyEqual(values, source); + end + + function fromMATLABFromEmptyUint8(testCase) + % Verifies fromMATLAB returns the expected arrow.buffer.Buffer + % value when given an empty uint8 array. + + import arrow.buffer.Buffer + + % Create buffer from 0x0 uint8 array + source = uint8.empty(0, 0); + buffer = Buffer.fromMATLAB(source); + testCase.verifyEqual(buffer.NumBytes, int64(0)); + values = toMATLAB(buffer); + testCase.verifyEqual(values, uint8.empty(0, 1)); + + % Create buffer from 0x1 uint8 array + source = uint8.empty(0, 1); + buffer = Buffer.fromMATLAB(source); + testCase.verifyEqual(buffer.NumBytes, int64(0)); + values = toMATLAB(buffer); + testCase.verifyEqual(values, uint8.empty(0, 1)); + + % Create buffer from 1x0 uint8 array + source = uint8.empty(0, 1); + buffer = Buffer.fromMATLAB(source); + testCase.verifyEqual(buffer.NumBytes, int64(0)); + values = toMATLAB(buffer); + testCase.verifyEqual(values, uint8.empty(0, 1)); + end + + function fromMATLABUint8Vector(testCase) + % Verifies fromMATLAB returns the expected arrow.buffer.Buffer + % value when given an uint8 vector. + + import arrow.buffer.Buffer + + % Create buffer from 1x3 uint8 array + source = uint8([3 9 27]); + buffer = Buffer.fromMATLAB(source); + testCase.verifyEqual(buffer.NumBytes, int64(3)); + values = toMATLAB(buffer); + testCase.verifyEqual(values, uint8([3 9 27]')); + + % Create buffer from 3x1 uint8 array + source = uint8([3 9 27]'); + buffer = Buffer.fromMATLAB(source); + testCase.verifyEqual(buffer.NumBytes, int64(3)); + values = toMATLAB(buffer); + testCase.verifyEqual(values, uint8([3 9 27]')); + end + + function fromMATLABDoubleVector(testCase) + % Verifies fromMATLAB returns the expected arrow.buffer.Buffer + % value when given an double vector. + + import arrow.buffer.Buffer + + % Create buffer from 1x3 double array + source = [3 9 27]; + buffer = Buffer.fromMATLAB(source); + testCase.verifyEqual(buffer.NumBytes, int64(24)); + values = toMATLAB(buffer); + expected = typecast(source', "uint8"); + testCase.verifyEqual(values, expected); + testCase.verifyEqual(typecast(values, "double"), source'); + + % Create buffer from 3x1 double array + source = [3 9 27]'; + buffer = Buffer.fromMATLAB(source); + testCase.verifyEqual(buffer.NumBytes, int64(24)); + values = toMATLAB(buffer); + expected = typecast(source, "uint8"); + testCase.verifyEqual(values, expected); + testCase.verifyEqual(typecast(values, "double"), source); + end + + function fromMATLABNonNumericError(testCase) + % Verify fromMATLAB throws an error if given a non-numeric + % array as input. + import arrow.buffer.Buffer + + fcn = @() Buffer.fromMATLAB(true); + testCase.verifyError(fcn, "MATLAB:validators:mustBeNumeric"); + + fcn = @() Buffer.fromMATLAB("A"); + testCase.verifyError(fcn, "MATLAB:validators:mustBeNumeric"); + + fcn = @() Buffer.fromMATLAB(datetime(2023, 1, 1)); + testCase.verifyError(fcn, "MATLAB:validators:mustBeNumeric"); + end + + function NumBytesNoSetter(testCase) + % Verifies the NumBytes property is not settable. + + import arrow.buffer.Buffer + + buffer = Buffer.fromMATLAB([1 2 3]); + fcn = @() setfield(buffer, "NumBytes", int64(1)); + testCase.verifyError(fcn, "MATLAB:class:SetProhibited"); + end + end +end \ No newline at end of file From d7fcafdec2af5f267ba6af44f4ffc8f435eae9c8 Mon Sep 17 00:00:00 2001 From: Sarah Gilmore Date: Wed, 4 Oct 2023 14:47:42 -0400 Subject: [PATCH 08/18] Update ID of error thrown when ViewOrCopy fails --- matlab/src/cpp/arrow/matlab/buffer/proxy/buffer.cc | 11 +++++------ matlab/src/cpp/arrow/matlab/error/error.h | 1 + 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/matlab/src/cpp/arrow/matlab/buffer/proxy/buffer.cc b/matlab/src/cpp/arrow/matlab/buffer/proxy/buffer.cc index e692d5a72c287..f6e8e2eb1d0db 100644 --- a/matlab/src/cpp/arrow/matlab/buffer/proxy/buffer.cc +++ b/matlab/src/cpp/arrow/matlab/buffer/proxy/buffer.cc @@ -75,15 +75,14 @@ namespace arrow::matlab::buffer::proxy { void Buffer::toMATLAB(libmexclass::proxy::method::Context& context) { namespace mda = ::matlab::data; - // If buffer is not backed by a cpu memory manager, calling its data() method - // may cause a crash. To avoid this, call ViewOrCopy, which tries to create a - // no-copy view of the buffer on the given memory manager device. If not possible, - // ViewOrCopy makes a copy of the buffer's contents. - + // If buffer->is_cpu() returns false, invoking buffer->data() may cause a crash. + // Avoid this potential crash by first invoking ViewOrCopy(buffer, memory_manager_device). + // This function tries to create a no-copy view of the buffer on the given memory + // manager device. If not possible, then ViewOrCopy copies the buffer's contents. MATLAB_ASSIGN_OR_ERROR_WITH_CONTEXT( auto cpu_buffer, arrow::Buffer::ViewOrCopy(buffer, arrow::default_cpu_memory_manager()), - context, "invalid:buffer" + context, error::BUFFER_VIEW_OR_COPY_FAILED ); const auto* data_begin = cpu_buffer->data(); diff --git a/matlab/src/cpp/arrow/matlab/error/error.h b/matlab/src/cpp/arrow/matlab/error/error.h index 347bc25b5f3a6..b8376c0a06ecb 100644 --- a/matlab/src/cpp/arrow/matlab/error/error.h +++ b/matlab/src/cpp/arrow/matlab/error/error.h @@ -198,4 +198,5 @@ namespace arrow::matlab::error { static const char* STRUCT_ARRAY_MAKE_FAILED = "arrow:array:StructArrayMakeFailed"; static const char* INDEX_EMPTY_CONTAINER = "arrow:index:EmptyContainer"; static const char* INDEX_OUT_OF_RANGE = "arrow:index:OutOfRange"; + static const char* BUFFER_VIEW_OR_COPY_FAILED = "arrow:buffer:ViewOrCopyFailed"; } From 29edeb154cc79ec1e0dc413ba4b0ce6930f31da2 Mon Sep 17 00:00:00 2001 From: Sarah Gilmore Date: Wed, 4 Oct 2023 15:46:15 -0400 Subject: [PATCH 09/18] Add isequal unit tests for arrow.buffer.Buffer --- matlab/src/matlab/+arrow/+buffer/Buffer.m | 2 +- matlab/test/arrow/buffer/tBuffer.m | 47 +++++++++++++++++++++++ 2 files changed, 48 insertions(+), 1 deletion(-) diff --git a/matlab/src/matlab/+arrow/+buffer/Buffer.m b/matlab/src/matlab/+arrow/+buffer/Buffer.m index 4e39a1ed5bd31..c1cad114202e2 100644 --- a/matlab/src/matlab/+arrow/+buffer/Buffer.m +++ b/matlab/src/matlab/+arrow/+buffer/Buffer.m @@ -47,7 +47,7 @@ tf = false; bufferProxyIDs = zeros([1 numel(varargin)], "uint64"); - for ii = 2:numel(varargin) + for ii = 1:numel(varargin) maybeBuffer = varargin{ii}; if ~isa(maybeBuffer, "arrow.buffer.Buffer") % If maybeBuffer is not an instance of diff --git a/matlab/test/arrow/buffer/tBuffer.m b/matlab/test/arrow/buffer/tBuffer.m index c4c16fd2e9088..326ea33cc3085 100644 --- a/matlab/test/arrow/buffer/tBuffer.m +++ b/matlab/test/arrow/buffer/tBuffer.m @@ -137,5 +137,52 @@ function NumBytesNoSetter(testCase) fcn = @() setfield(buffer, "NumBytes", int64(1)); testCase.verifyError(fcn, "MATLAB:class:SetProhibited"); end + + function IsEqualTrue(testCase) + % Verifies two buffers are considered equal if + % 1. They have the same size (NumBytes) + % 2. They contain the same bytes + + import arrow.buffer.Buffer + + % Compare two non-empty buffers + buffer1 = Buffer.fromMATLAB([1 2 3]); + buffer2 = Buffer.fromMATLAB([1 2 3]); + testCase.verifyTrue(isequal(buffer1, buffer2)); + + % Compare two buffers, one of which was created from a double + % array and the other created from a uint8 array. However, both + % arrays have the same bit pattern. + data = zeros([1 24], "uint8"); + data([7 8 16 23 24]) = [240 63 64 8 64]; + buffer3 = Buffer.fromMATLAB(data); + testCase.verifyTrue(isequal(buffer1, buffer3)); + + % Compare two empty buffers + buffer4 = Buffer.fromMATLAB([]); + buffer5 = Buffer.fromMATLAB([]); + testCase.verifyTrue(isequal(buffer4, buffer5)); + + % Compare more than two buffers + testCase.verifyTrue(isequal(buffer1, buffer2, buffer3)); + end + + function IsEqualFalse(testCase) + % Verifies two buffers are considered equal if + % 1. They have the same size (NumBytes) + % 2. They contain the same bytes + + import arrow.buffer.Buffer + + buffer1 = Buffer.fromMATLAB([1 2 3]); + buffer2 = Buffer.fromMATLAB([1 3 2]); + buffer3 = Buffer.fromMATLAB([1 2 3 4]); + testCase.verifyFalse(isequal(buffer1, buffer2)); + testCase.verifyFalse(isequal(buffer1, buffer3)); + testCase.verifyFalse(isequal(buffer1, buffer2, buffer3)); + + % Compare a buffer to a string + testCase.verifyFalse(isequal(buffer1, "A")); + end end end \ No newline at end of file From 37c83ed1b62613ee9ca882349f369017adb264a8 Mon Sep 17 00:00:00 2001 From: Sarah Gilmore Date: Wed, 4 Oct 2023 15:51:43 -0400 Subject: [PATCH 10/18] Make arrow.buffer.Buffer.fromMATLAB error if given a sparse or complex array --- matlab/src/matlab/+arrow/+buffer/Buffer.m | 2 +- matlab/test/arrow/buffer/tBuffer.m | 18 ++++++++++++++++++ 2 files changed, 19 insertions(+), 1 deletion(-) diff --git a/matlab/src/matlab/+arrow/+buffer/Buffer.m b/matlab/src/matlab/+arrow/+buffer/Buffer.m index c1cad114202e2..6c46276810299 100644 --- a/matlab/src/matlab/+arrow/+buffer/Buffer.m +++ b/matlab/src/matlab/+arrow/+buffer/Buffer.m @@ -67,7 +67,7 @@ methods (Static, Hidden) function buffer = fromMATLAB(data) arguments - data(:, 1) {mustBeNumeric} + data(:, 1) {mustBeNumeric, mustBeNonsparse, mustBeReal} end % Re-interpret bit pattern as uint8s without changing the diff --git a/matlab/test/arrow/buffer/tBuffer.m b/matlab/test/arrow/buffer/tBuffer.m index 326ea33cc3085..5a6bf7b3ec729 100644 --- a/matlab/test/arrow/buffer/tBuffer.m +++ b/matlab/test/arrow/buffer/tBuffer.m @@ -128,6 +128,24 @@ function fromMATLABNonNumericError(testCase) testCase.verifyError(fcn, "MATLAB:validators:mustBeNumeric"); end + function fromMATLABNonRealError(testCase) + % Verify fromMATLAB throws an error if given a complex + % numeric array as input. + import arrow.buffer.Buffer + + fcn = @() Buffer.fromMATLAB(10 + 3i); + testCase.verifyError(fcn, "MATLAB:validators:mustBeReal"); + end + + function fromMATLABNonSparseError(testCase) + % Verify fromMATLAB throws an error if given a sparse + % numeric array as input. + import arrow.buffer.Buffer + + fcn = @() Buffer.fromMATLAB(sparse(ones([5 1]))); + testCase.verifyError(fcn, "MATLAB:validators:mustBeNonsparse"); + end + function NumBytesNoSetter(testCase) % Verifies the NumBytes property is not settable. From b6d63984debcd7d58d5051116e66f8a7e599970f Mon Sep 17 00:00:00 2001 From: Sarah Gilmore Date: Thu, 5 Oct 2023 12:26:40 -0400 Subject: [PATCH 11/18] Move REGISTER_PROXY(arrow.buffer.Buffer, ...) below the registration of proxies in the arrow.array.* namespace --- matlab/src/cpp/arrow/matlab/proxy/factory.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/matlab/src/cpp/arrow/matlab/proxy/factory.cc b/matlab/src/cpp/arrow/matlab/proxy/factory.cc index a22344a214032..3091d248f8ca1 100644 --- a/matlab/src/cpp/arrow/matlab/proxy/factory.cc +++ b/matlab/src/cpp/arrow/matlab/proxy/factory.cc @@ -47,7 +47,6 @@ namespace arrow::matlab::proxy { libmexclass::proxy::MakeResult Factory::make_proxy(const ClassName& class_name, const FunctionArguments& constructor_arguments) { - REGISTER_PROXY(arrow.buffer.proxy.Buffer , arrow::matlab::buffer::proxy::Buffer); REGISTER_PROXY(arrow.array.proxy.Float32Array , arrow::matlab::array::proxy::NumericArray); REGISTER_PROXY(arrow.array.proxy.Float64Array , arrow::matlab::array::proxy::NumericArray); REGISTER_PROXY(arrow.array.proxy.UInt8Array , arrow::matlab::array::proxy::NumericArray); @@ -67,6 +66,7 @@ libmexclass::proxy::MakeResult Factory::make_proxy(const ClassName& class_name, REGISTER_PROXY(arrow.array.proxy.Date32Array , arrow::matlab::array::proxy::NumericArray); REGISTER_PROXY(arrow.array.proxy.Date64Array , arrow::matlab::array::proxy::NumericArray); REGISTER_PROXY(arrow.array.proxy.ChunkedArray , arrow::matlab::array::proxy::ChunkedArray); + REGISTER_PROXY(arrow.buffer.proxy.Buffer , arrow::matlab::buffer::proxy::Buffer); REGISTER_PROXY(arrow.tabular.proxy.RecordBatch , arrow::matlab::tabular::proxy::RecordBatch); REGISTER_PROXY(arrow.tabular.proxy.Table , arrow::matlab::tabular::proxy::Table); REGISTER_PROXY(arrow.tabular.proxy.Schema , arrow::matlab::tabular::proxy::Schema); From 9b335aeadd0ac378aa293bec3bfea11c1b085cc9 Mon Sep 17 00:00:00 2001 From: Sarah Gilmore Date: Tue, 10 Oct 2023 09:37:47 -0400 Subject: [PATCH 12/18] Remove extra newline --- matlab/src/cpp/arrow/matlab/buffer/proxy/buffer.cc | 1 - 1 file changed, 1 deletion(-) diff --git a/matlab/src/cpp/arrow/matlab/buffer/proxy/buffer.cc b/matlab/src/cpp/arrow/matlab/buffer/proxy/buffer.cc index f6e8e2eb1d0db..633b82d9fa127 100644 --- a/matlab/src/cpp/arrow/matlab/buffer/proxy/buffer.cc +++ b/matlab/src/cpp/arrow/matlab/buffer/proxy/buffer.cc @@ -24,7 +24,6 @@ namespace arrow::matlab::buffer::proxy { Buffer::Buffer(std::shared_ptr buffer) : buffer{std::move(buffer)} { - REGISTER_METHOD(Buffer, getNumBytes); REGISTER_METHOD(Buffer, isEqual); REGISTER_METHOD(Buffer, toMATLAB); From 9d2a9d6f10ef61a1e74a2a1821b22ae47c044e51 Mon Sep 17 00:00:00 2001 From: Sarah Gilmore Date: Tue, 10 Oct 2023 09:38:53 -0400 Subject: [PATCH 13/18] Remove extra newlines --- matlab/src/cpp/arrow/matlab/buffer/proxy/buffer.h | 2 -- 1 file changed, 2 deletions(-) diff --git a/matlab/src/cpp/arrow/matlab/buffer/proxy/buffer.h b/matlab/src/cpp/arrow/matlab/buffer/proxy/buffer.h index efee6fa615d40..bb6dc5e4c5d2c 100644 --- a/matlab/src/cpp/arrow/matlab/buffer/proxy/buffer.h +++ b/matlab/src/cpp/arrow/matlab/buffer/proxy/buffer.h @@ -33,9 +33,7 @@ class Buffer : public libmexclass::proxy::Proxy { static libmexclass::proxy::MakeResult make(const libmexclass::proxy::FunctionArguments& constructor_arguments); - protected: - void getNumBytes(libmexclass::proxy::method::Context& context); void toMATLAB(libmexclass::proxy::method::Context& context); From a24d9b7e1a976d12bda76d2d6839a00966461cb2 Mon Sep 17 00:00:00 2001 From: Sarah Gilmore Date: Tue, 10 Oct 2023 09:40:59 -0400 Subject: [PATCH 14/18] Uint -> UInt --- matlab/test/arrow/buffer/tBuffer.m | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/matlab/test/arrow/buffer/tBuffer.m b/matlab/test/arrow/buffer/tBuffer.m index 5a6bf7b3ec729..e2ef2f290dd68 100644 --- a/matlab/test/arrow/buffer/tBuffer.m +++ b/matlab/test/arrow/buffer/tBuffer.m @@ -26,7 +26,7 @@ function Smoke(testCase) testCase.verifyInstanceOf(buffer, "arrow.buffer.Buffer"); end - function fromMATLABUint8Scalar(testCase) + function fromMATLABUInt8Scalar(testCase) % Verifies fromMATLAB returns the expected arrow.buffer.Buffer % instance when given a scalar uint8 value. From 499643e807f7d7e07aa7abc688b5512dccee9608 Mon Sep 17 00:00:00 2001 From: Sarah Gilmore Date: Tue, 10 Oct 2023 09:42:10 -0400 Subject: [PATCH 15/18] Fix typo: uint8.empty(0, 1) -> uint8.empty(1, 0) --- matlab/test/arrow/buffer/tBuffer.m | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/matlab/test/arrow/buffer/tBuffer.m b/matlab/test/arrow/buffer/tBuffer.m index e2ef2f290dd68..60c74367c25fa 100644 --- a/matlab/test/arrow/buffer/tBuffer.m +++ b/matlab/test/arrow/buffer/tBuffer.m @@ -60,7 +60,7 @@ function fromMATLABFromEmptyUint8(testCase) testCase.verifyEqual(values, uint8.empty(0, 1)); % Create buffer from 1x0 uint8 array - source = uint8.empty(0, 1); + source = uint8.empty(1, 0); buffer = Buffer.fromMATLAB(source); testCase.verifyEqual(buffer.NumBytes, int64(0)); values = toMATLAB(buffer); From 6ebc4cff55a2f2126297eec70d5e6ba9d54f0cfb Mon Sep 17 00:00:00 2001 From: Sarah Gilmore Date: Tue, 10 Oct 2023 09:43:06 -0400 Subject: [PATCH 16/18] Fix typos: an -> a --- matlab/test/arrow/buffer/tBuffer.m | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/matlab/test/arrow/buffer/tBuffer.m b/matlab/test/arrow/buffer/tBuffer.m index 60c74367c25fa..ff49c580843e1 100644 --- a/matlab/test/arrow/buffer/tBuffer.m +++ b/matlab/test/arrow/buffer/tBuffer.m @@ -69,7 +69,7 @@ function fromMATLABFromEmptyUint8(testCase) function fromMATLABUint8Vector(testCase) % Verifies fromMATLAB returns the expected arrow.buffer.Buffer - % value when given an uint8 vector. + % value when given a uint8 vector. import arrow.buffer.Buffer @@ -90,7 +90,7 @@ function fromMATLABUint8Vector(testCase) function fromMATLABDoubleVector(testCase) % Verifies fromMATLAB returns the expected arrow.buffer.Buffer - % value when given an double vector. + % value when given a double vector. import arrow.buffer.Buffer From 80e7cd8616a1765eabad6fde6375188f4e19f60d Mon Sep 17 00:00:00 2001 From: Sarah Gilmore Date: Tue, 10 Oct 2023 09:45:31 -0400 Subject: [PATCH 17/18] Fix comments --- matlab/test/arrow/buffer/tBuffer.m | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/matlab/test/arrow/buffer/tBuffer.m b/matlab/test/arrow/buffer/tBuffer.m index ff49c580843e1..5e3f748adfa7b 100644 --- a/matlab/test/arrow/buffer/tBuffer.m +++ b/matlab/test/arrow/buffer/tBuffer.m @@ -97,6 +97,9 @@ function fromMATLABDoubleVector(testCase) % Create buffer from 1x3 double array source = [3 9 27]; buffer = Buffer.fromMATLAB(source); + % Since the input vector is a 1x3 double (i.e. 64-bit floating + % point value) array - each of the 3 elements takes up 8 bytes + % (3 elements * 8 bytes per element = 24 bytes). testCase.verifyEqual(buffer.NumBytes, int64(24)); values = toMATLAB(buffer); expected = typecast(source', "uint8"); @@ -186,9 +189,9 @@ function IsEqualTrue(testCase) end function IsEqualFalse(testCase) - % Verifies two buffers are considered equal if - % 1. They have the same size (NumBytes) - % 2. They contain the same bytes + % Verifies two buffers are not considered equal if + % 1. They do not have the same size (NumBytes) OR + % 2. They do not contain the same bytes import arrow.buffer.Buffer From cbcd0f42be9d77b4d210a7ecf5a5c671733cdf6e Mon Sep 17 00:00:00 2001 From: Sarah Gilmore Date: Tue, 10 Oct 2023 10:00:39 -0400 Subject: [PATCH 18/18] Fix typos in test case names: Uint8 -> UInt8 --- matlab/test/arrow/buffer/tBuffer.m | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/matlab/test/arrow/buffer/tBuffer.m b/matlab/test/arrow/buffer/tBuffer.m index 5e3f748adfa7b..d58af2fc9d22d 100644 --- a/matlab/test/arrow/buffer/tBuffer.m +++ b/matlab/test/arrow/buffer/tBuffer.m @@ -39,7 +39,7 @@ function fromMATLABUInt8Scalar(testCase) testCase.verifyEqual(values, source); end - function fromMATLABFromEmptyUint8(testCase) + function fromMATLABFromEmptyUInt8(testCase) % Verifies fromMATLAB returns the expected arrow.buffer.Buffer % value when given an empty uint8 array. @@ -67,7 +67,7 @@ function fromMATLABFromEmptyUint8(testCase) testCase.verifyEqual(values, uint8.empty(0, 1)); end - function fromMATLABUint8Vector(testCase) + function fromMATLABUInt8Vector(testCase) % Verifies fromMATLAB returns the expected arrow.buffer.Buffer % value when given a uint8 vector.