Adding operator+[=] and operator/[=] for FloatingArray and IntegerArray.

[joshuastorck]

• closes #xxxx
• tests added / passed
• passes git diff upstream/master | flake8 --diff
• whatsnew entry

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[+/]
• Unit test scaffolding for testing permutations of left/right hand
  side types on arithmetic operators
• Implementing IntegerArray::operator/, IntegerArray::operator+=
  FloatingArray::operator/=, FloatingArray::operator+=

[shoyer]

This check may or may not be a good idea from a performance perspective. Are you sure the conditional check is cheaper than just doing the arithmetic always? Even when there are only a few null values?

[wesm]

I would say to do the arithmetic always and use byte/word-wise & with the respective bitmaps (if they are byte-aligned, otherwise do it bitwise) to propagate nulls separately

[joshuastorck]

That would have been my preference. It just wasn't clear from the existing code if floating point arrays intentionally didn't have bitmaps.

[wesm]

Offset needs to be used as a bit offset. Can use CopyBitmap here (which hasn't been properly implemented yet, but the signature is okay)

[wesm]

Guess we may want some macros (or other templates) to be able to say "Do this thing for the cartesian product of certain types"

[wesm]

We could also use type traits to associate a class template (e.g. IntegerArray<Int32Type>) with just the type Int32Type

[wesm]

Google C++ guide discourages mutable references, preferring bool* changed

[wesm]

I would say to do the arithmetic always and use byte/word-wise & with the respective bitmaps (if they are byte-aligned, otherwise do it bitwise) to propagate nulls separately

[wesm]

Thinking out loud here. I have been thinking about a slightly different approach with implementing operators in generality, by instead unpacking the containers into simpler data structures, like:

# SFINAE
template <typename T, typename ENABLED = void>
struct TypeData {};

template <typename T>
struct TypeData<T, std::enable_if<std::is_integer<T>, T>::type> {
  T* data;
  uint8_t* valid_bits;
  int64_t length;
  int64_t offset;
}

and then have arithmetic methods

template <typename LEFT_TYPE, typename RIGHT_TYPE, typename OUT_TYPE>
void add(TypeData<LEFT_TYPE> left, TypeData<RIGHT_TYPE> right, TypeData<OUT_TYPE> out) {
  ...
  for (int64_t i = 0; i < left.length; ++i) {
    out.data[i] = left.data[i] + right.data[i];
  } 
  ...
}

In the case of inplace-add, in the operator dispatcher, you would have::

TypeData<LEFT_TYPE> left(...);
TypeData<RIGHT_TYPE> right(...);
add(left, right, left);

You would have to deal with whether to allocate the validity/null bitmap prior to invoking this function, but I think it's probably better to handle the mutability checks / memory allocation / etc. separate from the analytics. Then you can box the output as you wish.

Of course, you need to have a forward declaration someplace

extern template <typename LEFT_TYPE, typename RIGHT_TYPE, typename OUT_TYPE>
void add(TypeData<LEFT_TYPE> left, TypeData<RIGHT_TYPE> right, TypeData<OUT_TYPE> out);

and then instantiate the matrix of types you're choosing to support. Some analytical kernels might need to allocate memory based on what's in the data, and so we could do something like

void kernel_name(OperatorContext* ctx, TypeData<ARG1_TYPE> arg1, ...);

I'd also like to maximize code sharing amongst related operator implementations.

I'm going to take a crack at implementing part of Take under this regime (see https://github.com/pandas-dev/pandas/blob/master/pandas/src/algos_take_helper.pxi). Will keep thinking about this.

Also, per #46 we are contemplating going bitmaps for all null handling which would make the code for integers and floats identical (outside some type promotion details maybe)

[joshuastorck]

If you assume that all numeric types will have a null bitmap, then I think NumericArray in the current code would probably hold something equivalent to what you are suggesting with TypeData.

I'm not following your SFINAE code, but I think you are driving at a design that looks a bit more like this:

template <typename KERNEL,
          typename LEFT_TYPE, 
          typename RIGHT_TYPE, 
          typename OUT_TYPE,  
          typename... KERNEL_ARGS>
void RunKernel(TypeData<LEFT_TYPE> const& left, 
               TypeData<RIGHT_TYPE> const& right,
               TypeData<OUT_TYPE>& out,                    
               KERNEL_ARGS&&... kernel_args)
{
    KERNEL kernel(std::forward<KERNEL_ARGS>(kernel_args)...);
    kernel(left, right, out);
}

class GenericKernel
{

public:

    template <typename LEFT_TYPE, typename RIGHT_TYPE, typename OUT_TYPE, typename FUNCTOR>
    void Run(const TypeData<LEFT_TYPE>& left, 
             FUNCTOR& functor,
             const TypeData<RIGHT_TYPE>& right,
             TypeData<OUT_TYPE>& out)
    {
        // Do a bitmap OR for nulls first and then do ffs
        // and branch 
        for (auto ii = 0; ii < left.length; ++ii)
        {
            functor(left.data[ii], right.data[ii], out.data[ii]);
        }
    }    

};

// This would get initialized once at startup and would 
// have information about the instruction set available
// on the machine (AVX vs. AVX2)
class SIMDConfiguration
{
};

template <typename LEFT_TYPE, typename RIGHT_TYPE>
class SIMDSinglePrecisionPossible;

template <typename LEFT_TYPE, typename RIGHT_TYPE>
class SIMDSinglePrecisionPossible : public std::false_type
{
};

template <>
class SIMDSinglePrecisionPossible<float, float> : public std::true_type
{
};

template <typename LEFT_TYPE, typename RIGHT_TYPE>
class SIMDDoublePrecisionPossible;

template <typename LEFT_TYPE, typename RIGHT_TYPE>
class SIMDDoublePrecisionPossible : public std::false_type
{
};

template <>
class SIMDDoublePrecisionPossible<double, double> : public std::true_type
{
};

template <typename LEFT_TYPE, typename RIGHT_TYPE>
class SIMDImpossible 
{
public:
    static constexpr auto value = !(SIMDDoublePrecisionPossible<LEFT_TYPE, RIGHT_TYPE>::value ||
                                    SIMDSinglePrecisionPossible<LEFT_TYPE, RIGHT_TYPE>::value);
};


class AddKernel : public GenericKernel
{

public:

    AddKernel(SIMDConfiguration& config) : config_(config)
    {
    }

    template <typename LEFT_TYPE,
              typename RIGHT_TYPE,
              typename OUT_TYPE>
    void operator()(const TypeData<LEFT_TYPE>& left, 
                    const TypeData<RIGHT_TYPE>& right,
                    TypeData<OUT_TYPE>& out)
    {
        RunOptimized<LEFT_TYPE, RIGHT_TYPE>(left, right, out);
    }

    template <typename LEFT_TYPE, typename RIGHT_TYPE, typename OUT_TYPE>
    typename std::enable_if<SIMDSinglePrecisionPossible<LEFT_TYPE, RIGHT_TYPE>::value>::type
    RunOptimized(const TypeData<LEFT_TYPE>& left, 
                 const TypeData<RIGHT_TYPE>& right,
                 TypeData<OUT_TYPE>& out)
    {
        // Use _mm_add_ps or _mm256_add_pd depending on 
        // config_, fallback to Run
    }

    template <typename LEFT_TYPE, typename RIGHT_TYPE, typename OUT_TYPE>
    typename std::enable_if<SIMDDoublePrecisionPossible<LEFT_TYPE, RIGHT_TYPE>::value>::type
    RunOptimized(const TypeData<LEFT_TYPE>& left, 
                 const TypeData<RIGHT_TYPE>& right,
                 TypeData<OUT_TYPE>& out)
    {
        // Use _mm_add_pd or _mm256_add_pd depending on 
        // config_, fallback to Run
    }

    template <typename LEFT_TYPE, typename RIGHT_TYPE, typename OUT_TYPE>
    typename std::enable_if<SIMDImpossible<LEFT_TYPE, RIGHT_TYPE>::value>::type
    RunOptimized(const TypeData<LEFT_TYPE>& left, 
                 const TypeData<RIGHT_TYPE>& right,
                 TypeData<OUT_TYPE>& out)
    {
        static auto add = [](auto const& left, auto const& right, auto& out)
            {
            out = left + right;
            };
        Run(left, add, right, out);
    }


private:

    SIMDConfiguration& config_;

};

int main(int argc, char* argv[])
{
    // Just to see if it compiles
    TypeData<float> a;
    TypeData<int> b;
    TypeData<int> c;
    SIMDConfiguration config;
    RunKernel<AddKernel>(a, b, c, config);
}

I would expect FloatingArray and IntegerArray to control what operations are allowed using SFINAE, in the same fashion that I've currently implemented it. The main difference would be that those functions would just invoke RunKernel and would setup the correct return type based on the desired rules for promotion of types.

[wesm]

In pure Python and NumPy at least, integer division always yields double

[shoyer]

We could support integer division with //, but that's certainly a lower priority.

[joshuastorck]

Just to be clear, this is just choosing the right size of floating point type based on the inputs.

[xhochy]

This will increase the minimal required GCC version to 5?

[wesm]

I think c++1y is required by clang, we can do c++14 on gcc 4.9 I believe

[xhochy]

The thing that comes to my mind is that the typical toolchains for packages that work on multiple Linux distros (i.e. manylinux1 wheels and the redhat toolchain in conda) all depend on code compiling with GCC 4.8.

[wesm]

Redhat devtoolset-3 has gcc 4.9.x, so I imagine by the time pandas2 is ready for primetime this upgrade will have happened. There will be other packages that need C++14, might as well push the issue

[xhochy]

Ok, then let's hope for a manylinux2 standard then and enjoy the new C++14 features ;) . Looking forward to std::make_unique.

[wesm]

There's a bunch of bits here that can be extracted and merged as useful utilities. I want to wait on pulling in operator implementations until the operator interface has been thought through some more: https://docs.google.com/document/d/1YmsV48iO6YNSxCIC84Xig5z-i9g4g7rzYTmCgUAxKRk/edit#heading=h.pzfqkdqgusm7

I'd like to take a crack at implementing some operators in a way that can evaluated either in single- or multi-threaded mode (without excessive code duplication -- so the code that runs things in parallel won't be too knowledgeable about the details of the operator except how invoke their virtual functions

[wesm]

I cherry-picked some of this as a7f2a82

I'm going to do a little prototyping on operator evaluation generally, so will return to this hopefully soon