Skip to content
This repository was archived by the owner on Apr 10, 2024. It is now read-only.

Commit d8d55cf

Browse files
committed
Adding operator+[=] and operator/[=] for FloatingArray and IntegerArray.
This includes a design change that obviates the need for an ArrayView. Instead, every array has an internal offset. Shallow copy is achieved by copy constructor, though the current set of copy constructors don't yet support a slice. Deep copy is still achieved through the Copy virtual function. More detailed explanation of the changes: * Adding copy/move constructors for {Floating,Integer,Numeric}Array * Adding various method for marking/getting nulls (valid bits) in integer arrays * Changing data() and mutable_data() in NumericArray so that they return a pointer that starts at the array's offset * Addition of Addable/Divisable classes (similar to Boost operators) for easy support of operator[+/] * Implementing IntegerArray::operator/, IntegerArray::operator+= FloatingArray::operator/=, FloatingArray::operator+= * Adding meta programming class TypeList, which is used as scaffolding in array-test to simplify the declaration of every permutation of arithmetic operation for all types.
1 parent 29df124 commit d8d55cf

15 files changed

+911
-56
lines changed

CMakeLists.txt

+2-1
Original file line numberDiff line numberDiff line change
@@ -74,7 +74,7 @@ endif()
7474
# GCC cannot always verify whether strict aliasing rules are indeed followed due to
7575
# fundamental limitations in escape analysis, which can result in subtle bad code generation.
7676
# This has a small perf hit but worth it to avoid hard to debug crashes.
77-
set(CXX_COMMON_FLAGS "-std=c++11 -fno-strict-aliasing -msse4.2 -Wall -Wno-sign-compare -Wno-deprecated -pthread -D__STDC_FORMAT_MACROS")
77+
set(CXX_COMMON_FLAGS "-std=c++1y -fno-strict-aliasing -msse4.2 -Wall -Wno-sign-compare -Wno-deprecated -pthread -D__STDC_FORMAT_MACROS")
7878

7979
# compiler flags for different build types (run 'cmake -DCMAKE_BUILD_TYPE=<type> .')
8080
# For all builds:
@@ -642,6 +642,7 @@ set(PANDAS_TEST_LINK_LIBS ${PANDAS_MIN_TEST_LIBS})
642642
############################################################
643643

644644
add_subdirectory(src/pandas)
645+
add_subdirectory(src/pandas/meta)
645646
add_subdirectory(src/pandas/util)
646647

647648
set(PANDAS_SRCS

pandas/native.pxd

+1-1
Original file line numberDiff line numberDiff line change
@@ -74,7 +74,7 @@ cdef extern from "pandas/api.h" namespace "pandas":
7474
c_bool Equals(const DataType& other)
7575
string ToString()
7676

77-
ctypedef shared_ptr[DataType] TypePtr
77+
ctypedef shared_ptr[const DataType] TypePtr
7878

7979
cdef cppclass Int8Type(DataType):
8080
pass

pandas/native.pyx

+1-1
Original file line numberDiff line numberDiff line change
@@ -277,7 +277,7 @@ cdef Array wrap_array(const lp.ArrayPtr& arr):
277277

278278
cdef PandasType wrap_type(const lp.TypePtr& sp_type):
279279
cdef:
280-
lp.DataType* type = sp_type.get()
280+
const lp.DataType* type = sp_type.get()
281281
PandasType result
282282

283283
if type.type() == lp.TypeId_CATEGORY:

src/pandas/array-test.cc

+123
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
#include "pandas/array.h"
1313
#include "pandas/common.h"
1414
#include "pandas/memory.h"
15+
#include "pandas/meta/typelist.h"
1516
#include "pandas/test-util.h"
1617
#include "pandas/type.h"
1718
#include "pandas/types/numeric.h"
@@ -45,6 +46,128 @@ TEST_F(TestArray, Attrs) {
4546
ASSERT_EQ(values_.size(), array_->length());
4647
}
4748

49+
template <typename LEFT_ARRAY_TYPE, typename RIGHT_ARRAY_TYPE, std::size_t LENGTH = 10>
50+
class OperatorTestData {
51+
public:
52+
OperatorTestData()
53+
: left_buffer_(std::make_shared<Buffer>(
54+
reinterpret_cast<const std::uint8_t*>(Initialize(left_data_)),
55+
LENGTH * sizeof(LEFT_C_TYPE))),
56+
right_buffer_(std::make_shared<Buffer>(
57+
reinterpret_cast<const std::uint8_t*>(Initialize(right_data_)),
58+
LENGTH * sizeof(RIGHT_C_TYPE))),
59+
left_array_(LENGTH, left_buffer_),
60+
right_array_(LENGTH, right_buffer_) {}
61+
62+
template <typename C_TYPE>
63+
static C_TYPE* Initialize(C_TYPE (&value)[LENGTH]) {
64+
for (auto ii = 0; ii < LENGTH; ++ii) {
65+
// Start at 1 so that we don't get FPE with operator/
66+
value[ii] = static_cast<C_TYPE>(ii + 1);
67+
}
68+
return value;
69+
}
70+
71+
using LEFT_C_TYPE = typename LEFT_ARRAY_TYPE::T;
72+
73+
using RIGHT_C_TYPE = typename RIGHT_ARRAY_TYPE::T;
74+
75+
LEFT_C_TYPE left_data_[LENGTH];
76+
77+
RIGHT_C_TYPE right_data_[LENGTH];
78+
79+
std::shared_ptr<Buffer> left_buffer_;
80+
81+
std::shared_ptr<Buffer> right_buffer_;
82+
83+
LEFT_ARRAY_TYPE left_array_;
84+
85+
RIGHT_ARRAY_TYPE right_array_;
86+
};
87+
88+
template <typename OPERATOR, typename INPLACE_OPERATOR, std::size_t LENGTH = 10>
89+
class TestInplaceOperator {
90+
public:
91+
TestInplaceOperator(OPERATOR& operation, INPLACE_OPERATOR& inplace_operation)
92+
: operation_(operation), inplace_operation_(inplace_operation) {}
93+
94+
template <typename PAIR>
95+
void operator()() {
96+
using FIRST = typename std::tuple_element<0, PAIR>::type;
97+
using SECOND = typename std::tuple_element<1, PAIR>::type;
98+
OperatorTestData<FIRST, SECOND> test_data;
99+
auto result = operation_(test_data.left_array_, test_data.right_array_);
100+
for (auto ii = 0; ii < test_data.left_array_.length(); ++ii) {
101+
ASSERT_EQ(result.data()[ii],
102+
operation_(test_data.left_data_[ii], test_data.right_data_[ii]));
103+
}
104+
inplace_operation_(test_data.left_array_, test_data.right_array_);
105+
for (auto ii = 0; ii < test_data.left_array_.length(); ++ii) {
106+
ASSERT_EQ(test_data.left_array_.data()[ii],
107+
operation_(test_data.left_data_[ii], test_data.right_data_[ii]));
108+
}
109+
for (auto ii = 0; ii < test_data.left_array_.length(); ++ii) {
110+
ASSERT_EQ(test_data.left_array_.data()[ii], result.data()[ii]);
111+
}
112+
}
113+
114+
private:
115+
OPERATOR& operation_;
116+
117+
INPLACE_OPERATOR inplace_operation_;
118+
};
119+
120+
template <typename OPERATOR, std::size_t LENGTH = 10>
121+
class TestOperator {
122+
public:
123+
TestOperator(OPERATOR& operation) : operation_(operation) {}
124+
125+
template <typename PAIR>
126+
void operator()() {
127+
using FIRST = typename std::tuple_element<0, PAIR>::type;
128+
using SECOND = typename std::tuple_element<1, PAIR>::type;
129+
OperatorTestData<FIRST, SECOND> test_data;
130+
auto result = operation_(test_data.left_array_, test_data.right_array_);
131+
for (auto ii = 0; ii < test_data.left_array_.length(); ++ii) {
132+
ASSERT_EQ(result.data()[ii],
133+
operation_(test_data.left_data_[ii], test_data.right_data_[ii]));
134+
}
135+
}
136+
137+
private:
138+
OPERATOR& operation_;
139+
};
140+
141+
using IntegerTypes = TypeList<IntegerArray<UInt8Type>, IntegerArray<UInt16Type>,
142+
IntegerArray<UInt32Type>, IntegerArray<UInt64Type>, IntegerArray<Int8Type>,
143+
IntegerArray<Int16Type>, IntegerArray<Int32Type>, IntegerArray<Int64Type>>;
144+
145+
using FloatingPointTypes = TypeList<FloatingArray<FloatType>, FloatingArray<DoubleType>>;
146+
147+
using NumericTypes = decltype(IntegerTypes() + FloatingPointTypes());
148+
149+
TEST(TestArrayOperators, Addition) {
150+
auto plus = [](auto const& left, auto const& right) { return left + right; };
151+
auto plus_inplace = [](auto& left, auto const& right) { left += right; };
152+
153+
static constexpr auto addition_tests =
154+
FloatingPointTypes().CartesianProduct(NumericTypes());
155+
TestInplaceOperator<decltype(plus), decltype(plus_inplace)> tester(plus, plus_inplace);
156+
addition_tests.Iterate(tester);
157+
}
158+
159+
TEST(TestArrayOperators, Division) {
160+
auto divide = [](auto const& left, auto const& right) { return left / right; };
161+
auto divide_inplace = [](auto& left, auto const& right) { left /= right; };
162+
163+
TestOperator<decltype(divide)> test(divide);
164+
IntegerTypes().CartesianProduct(IntegerTypes()).Iterate(test);
165+
166+
TestInplaceOperator<decltype(divide), decltype(divide_inplace)> inplace_test(
167+
divide, divide_inplace);
168+
FloatingPointTypes().CartesianProduct(NumericTypes()).Iterate(inplace_test);
169+
}
170+
48171
// ----------------------------------------------------------------------
49172
// Array view object
50173

src/pandas/array.cc

+1-4
Original file line numberDiff line numberDiff line change
@@ -11,11 +11,8 @@ namespace pandas {
1111
// ----------------------------------------------------------------------
1212
// Array
1313

14-
Array::Array(const std::shared_ptr<DataType>& type, int64_t length)
15-
: type_(type), length_(length) {}
16-
1714
Status Array::Copy(std::shared_ptr<Array>* out) const {
18-
return Copy(0, length_, out);
15+
return Copy(0, length(), out);
1916
}
2017

2118
// ----------------------------------------------------------------------

src/pandas/array.h

+16-9
Original file line numberDiff line numberDiff line change
@@ -20,9 +20,19 @@ class Array {
2020
public:
2121
virtual ~Array() {}
2222

23-
int64_t length() const { return length_; }
24-
std::shared_ptr<DataType> type() const { return type_; }
25-
DataType::TypeId type_id() const { return type_->type(); }
23+
virtual int64_t length() const = 0;
24+
// There are two methods to obtain the data type.
25+
// The signature without a shared_ptr allows sub-classes
26+
// to have a covariant return type, which eliminates the
27+
// need/danger of doing a static_cast when dealing with
28+
// a concrete sub-class. Ideally, the shared_ptr signature
29+
// would suffice, but the compiler cannot treat a shared_ptr
30+
// to a base class and a shared_ptr to a subclass as a
31+
// covariant return type.
32+
virtual TypePtr type() const = 0;
33+
virtual const DataType& type_reference() const = 0;
34+
35+
DataType::TypeId type_id() const { return type()->type(); }
2636

2737
// Copy a section of the array into a new output array
2838
virtual Status Copy(
@@ -42,13 +52,10 @@ class Array {
4252
virtual bool owns_data() const = 0;
4353

4454
protected:
45-
std::shared_ptr<DataType> type_;
46-
int64_t length_;
55+
Array() {}
4756

48-
Array(const std::shared_ptr<DataType>& type, int64_t length);
49-
50-
private:
51-
DISALLOW_COPY_AND_ASSIGN(Array);
57+
Array(const Array& other) = default;
58+
Array(Array&& other) = default;
5259
};
5360

5461
// An object that is a view on a section of another array (possibly the whole

src/pandas/meta/CMakeLists.txt

+10
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
# This file is a part of pandas. See LICENSE for details about reuse and
2+
# copyright holders
3+
4+
#######################################
5+
# Unit tests
6+
#######################################
7+
8+
set(PANDAS_TEST_LINK_LIBS pandas_test_util ${PANDAS_MIN_TEST_LIBS})
9+
10+
ADD_PANDAS_TEST(typelist-test)

0 commit comments

Comments
 (0)