c++c++17unordered-mapstd

std::reduce with std::unordered_map


I have an unordered_map of vectors and I'm trying to use std::reduce to get the sum of all values in all vectors in the map. My current functional code (which I want to replace) looks like this:

// input is std::unordered_map<std::vector<uint64_t>>
template<typename T>
uint64_t get_map_sum(T& my_map)
{
    uint64_t totalcount = 0;
    for (auto& p : my_map) 
    {
        for (const auto& q : p.second)
            totalcount += q;
    }
    return total_count;
}

I'd like to replace this with std::reduce to utilize the parallel execution; I thought this would be straight forward as I only needed to replace each loop with a call to std::reduce, but this doesn't appear to be working. My attempt is this:

#include <numeric>
#include <execution>
#include <vector>
#include <unordered_map>
#include <cstdint>
// reduces the vectors
template <typename Iter, typename T>
T get_vector_sum(Iter begin, Iter end, T initial = 0)
{
    return std::reduce(std::execution::par_unseq, begin, end, initial, 
           [&](auto cur, auto prev) { return cur + prev; });
}

// calls get_vector_sum for all vectors and then reduces vector sums
template<typename Iter>
uint64_t get_map_sum(Iter begin, Iter end)
{
    return std::reduce(std::execution::par_unseq, begin, end, 0ULL,
            [&](auto prev, auto cur)
            {
                return get_vector_sum<std::vector<uint64_t>::iterator, 
                       uint64_t>(cur.begin(), cur.end(), prev);
                //return get_vector_sum<std::vector<uint64_t>::iterator,
                //       uint64_t>(cur.second.begin(), cur.second.end(), prev);
            });
}

With the code above, I get an error message saying error C2039: 'begin': is not a member of 'std::pair' referring to the auto cur in the lambda inside get_map_sum. I initially used cur as a std::pair, but when I did that I got a different error saying error C2228: left of '.second' must have class/struct/union.

int main()
{
    std::unordered_map<uint64_t, std::vector<uint64_t>> in({ 
        {1, std::vector<uint64_t>{1,2,3,4,5} }, 
        {2, std::vector<uint64_t>{1,2,3,4,5}}, 
        {3, std::vector<uint64_t>{1,2,3,4,5}}});

    auto x = get_map_sum(in); // output 45
    auto y = get_map_sum(in.begin(), in.end()); // error

    return 0;
}

Is it possible to use std::reduce with maps like this and, if so, what changes do I need to make to get this working?


Solution

  • Note this requirements for binary_op of std::reduce:

    binary FunctionObject that will be applied in unspecified order to the result of dereferencing the input iterators, the results of other binary_op and init.

    This implies that the result of your lambda result and init needs to be of the same type as map's value type, i.e., std::pair<const uint64_t, std::vector<uint64_t>>.

    You would therefore need to perform the outer reduction over values of this type, which would involve construction of new vectors.


    I have also tried to create an exemplary code as follows:

    using M = std::unordered_map<uint64_t, std::vector<uint64_t>>;
    using V = M::value_type;
    
    M in({ {1, std::vector<uint64_t>{1,2,3,4,5}}, 
           {2, std::vector<uint64_t>{1,2,3,4,5}}, 
           {3, std::vector<uint64_t>{1,2,3,4,5}} });
    
    auto p = std::reduce(in.begin(), in.end(), V{}, 
        [](const V& a, const V& b) {
            auto ra = std::reduce(a.second.begin(), a.second.end(), 0UL,
                [](uint64_t i1, uint64_t i2){ return i1 + i2; });
            auto rb = std::reduce(b.second.begin(), b.second.end(), 0UL,
                [](uint64_t i1, uint64_t i2){ return i1 + i2; });
            return V{0, { ra + rb }};
    });
    

    But it does not compile with GCC due to seemingly missing std::reduce implementation and Clang complains about missing copy assignment operator for value type, which is not copy-assignable due to const key: https://wandbox.org/permlink/FBYAhCArtOHvwu8C.

    However, in cppreference, the requirements for the value type is only MoveConstructible, not Copy/MoveAssignable. So, there seems to be an incorrect implementation in libc++.


    In this exemplary code, I was able to make it working by defning V without const as follows:

    using V = std::pair<uint64_t, std::vector<uint64_t>>; 
    

    See https://wandbox.org/permlink/lF9VuJwISYXhpBJL.