How to add element by element of two STL vectors?

Question

The question is quite dumb, but I need to do it in a very efficient way - it will be performed over an over again in my code. I have a function that returns a vector, and I have to add the returned values to another vector, element by element. Quite simple:

vector<double> result;
vector<double> result_temp
for(int i=0; i< 10; i++) result_temp.push_back(i);

result += result_temp //I would like to do something like that.
for(int i =0; i< result_temp.size();i++)result[i] += result_temp[i]; //this give me segfault

The mathematical operation that I'm trying to do is

u[i] = u[i] + v[i] for all i

What can be done?

Thanks

EDIT: added a simple initialization, as that is not the point. How should result be initialized?

Answer 1

I'm with @James McNellis - this code seems correct, as long as result and result_temp are the same length.

Also - why did you declare result, but use the variable result_v - is that how the code is actually written? If so, that's an issue

Answer 2

If you are trying to append one vector to another, you can use something like the following. These are from one of my utilities libraries--two operator+= overloads for std::vector: one appends a single element to the vector, the other appends an entire vector:

template <typename T>
std::vector<T>& operator+=(std::vector<T>& a, const std::vector<T>& b)
{
    a.insert(a.end(), b.begin(), b.end());
    return a;
}

template <typename T>
std::vector<T>& operator+=(std::vector<T>& aVector, const T& aObject)
{
    aVector.push_back(aObject);
    return aVector;
}

If you are trying to perform a summation (that is, create a new vector containing the sums of the elements of two other vectors), you can use something like the following:

#include <algorithm>
#include <functional>

template <typename T>
std::vector<T> operator+(const std::vector<T>& a, const std::vector<T>& b)
{
    assert(a.size() == b.size());

    std::vector<T> result;
    result.reserve(a.size());

    std::transform(a.begin(), a.end(), b.begin(), 
                   std::back_inserter(result), std::plus<T>());
    return result;
}

You could similarly implement an operator+= overload.

Answer 3

You need to initialize result to all zeros first; just declaring the variable doesn't actually allocate any elements.

Try this:

vector<double> result(10); // default-initialize to 10 elements
vector<double> result_temp;
for(int i=0; i< 10; i++) 
    result_temp.push_back(i);

for(int i =0; i< result_temp.size();i++)
    result[i] += result_temp[i];

Answer 4

The code seems fine, but my first inclination would be to alter whatever code is filling the vector with values to add to the values in the first vector to take in a reference to the first vector and add directly to it rather than creating a new vector that gets returned. That's just inefficient.

If you can't alter the function in that way, perhaps you can alter it so that it takes a reference to a vector which it clears and then inserts the values into so that you aren't copying vectors around. That can get to be expensive if you're doing it much.

Another nitpick if you're trying to get this as fast as possible, you should use pre-increment with iterators rather than post-increment. The temporary that post-increment creates can't be optimized away when dealing with overloaded operators rather than built-in types. So, you keep creating and destroying a temporary every iteration of your loop. EDIT: As was pointed out in the comments, you're using indices here rather than iterators (I obviously wasn't paying enough attention), so this bit of advice doesn't really apply here. However, in cases where you are using iterators, it's still valid.

Other than that, if you're trying to add all of the elements of two vectors togther, what you have is probably about as efficient a solution as you're going to get. There are better ways if what you're concerned about is inserting the elements of one vector into another, but if you're just adding their values together, what you have looks good. I would expect that using any STL algorithms would be at best just as fast and likely slower due to extra function calls, but you'd probably have to profile it to be sure.

Answer 5

It sure looks like the problem is accessing values of result that don't exist. tzaman shows how to initialize result to 10 elements, each with value 0.

Now you need to call the transform function (from <algorithm>), applying the plus function object (from <functional>):

std::transform(result.begin(), result.end(), result_temp.begin(),
               result.begin(), std::plus<double>());

This iterates over result and result_temp, applies plus that adds doubles, and writes the sum back to result.

Answer 6

If your code is segfaulting then that's a correctness issue, not an efficiency issue.

To achieve "u[i] = u[i] + v[i] for all i", I would do basically what you did:

assert(u.size() == v.size()); // will fail with your initialization code, since
                              // your "result" has size 0, not size 10.
                              // perhaps do u.resize(v.size());
for (size_t i = 0; i < u.size(); ++i) {
    u[i] += v[i];
}

If you really care about performance of your program (that is, you've written a basic version and it's so slow that your program is failing some requirement, and you've proved that this is the code where much of the time is taken), then you could try:

• switching on lots of optimization in your compiler (actually, I usually do this by default even when there isn't a performance problem),
• using iterators instead of indexes (rarely makes much difference, but it's easy enough to compare the two),
• unrolling the loop a bit (can make a worthwhile speed difference, but that's quite sensitive to the particular case, and it encourages coding errors).
• looking at platform-specific SIMD instructions rather than C++. Then use embedded assembler or compiler intrinsics for those instructions.

Nevertheless, you have no business worrying about performance before your code is correct ;-). "Make it work, make it right, make it fast" is a reasonable motto, although often you don't need to go as far as step 3.

std::valarray actually has exactly the operator+= you want. Before you replace all your vectors with valarrays, be aware that doesn't necessarily mean it's any "more efficient" than a simple loop - I don't know how seriously implementers take valarray. You can always look at the source in your implementation. I also don't know why the multiple-data arithmetic functionality of valarray wasn't defined as part of vector, but there's usually a reason.