Question

I wrote some maths functions for use within my program, they are going to get very heavy use. I would like to offer the code up to see if a) there are logic improvements to made and b) if there is a better way of doing any of this. Its a header file which is included where needed.

I'm not compiling for c++11 so please bare that in mind. - I'm also aware that the rootDouble for negative numbers is not mathematically correct.

I think the first thing which may come up is converting the vector inputs to pass by reference, comments around that are welcome.

In terms of me accepting an answer, i'd like to know what and how these functions could be improved for speed.

++ I've posted this rather quickly, hope I didn't leave any embarrassing errors inside!

#ifndef MATHSFUNCTIONS_H_
#define MATHSFUNCTIONS_H_
#include <algorithm>
#include <vector>
#include <numeric>
#include <cmath>


class MathsFunctions {
public:
    MathsFunctions();
    virtual ~MathsFunctions();

    inline static double squareDouble(double input) {
        return input * input;
    }

    inline static double rootDouble(double input) {
        if (input == 0.0) {
            return 0.0;
        } else if ( input < 0.0) {
            input = flipDouble(input);
            input = sqrt(input);
            return flipDouble(input);
        }
        return sqrt(input);
    }

    inline static double flipDouble(double input) {
        return input * -1;
    }

    inline static double rangeInVec(std::vector<double> inputs) {
        return maxInVec(inputs) - minInVec(inputs);
    }

    inline static double stdDevInVec(std::vector<double> inputs) {
        if (inputs.size() < 2) {return 0.0;}
        double mean = meanInVec(inputs);
        double sq_sum = std::inner_product(inputs.begin(), inputs.end(), inputs.begin(), 0.0);
        return std::sqrt(sq_sum / inputs.size() - mean * mean);

    }

    inline static double meanInVec(std::vector<double> inputs) {
        double sum = std::accumulate(inputs.begin(), inputs.end(), 0.0);
        return sum / inputs.size();
    }

    inline static double sumOfVec(std::vector<double> inputs) {
        double total = 0.0;
        for (unsigned int var = 0; var < inputs.size(); ++var) {
            total += inputs[var];
        }
        return total;
    }

    inline static double maxInVec(std::vector<double> inputs) {
        bool first = true;
        double max;
        for (unsigned int var = 0; var < inputs.size(); ++var) {
            if (first) {
                max = inputs[var];
                first = false;
            } else {
                if (inputs[var] > max) {
                    max = inputs[var];
                }
            }
        }
        return max;
    }

    inline static double minInVec(std::vector<double> inputs) {
        bool first = true;
        double min;
        for (unsigned int var = 0; var < inputs.size(); ++var) {
            if (first) {
                min = inputs[var];
                first = false;
            } else {
                if (inputs[var] < min) {
                    min = inputs[var];
                }
            }
        }
        return min;
    }

    inline static std::vector<double> weightValueVector(std::vector<double> inputs,std::vector<double> weights) {
        std::vector<double> results;
        for (unsigned x = 0; x < inputs.size(); ++x) {
            results.push_back(inputs[x] * weights[x]);
        }
        return results;
    }

};

#endif /* MATHSFUNCTIONS_H_ */
Was it helpful?

Solution

  1. Perhaps the biggest gain in both speed and development time would be to use an existing linear algebra library instead of re-inventing the wheel, see

  2. Use BLAS and LAPACK that is tuned to your machine architecture. In particular, the vector instructions available on your machine and the cache sizes have a BIG impact on the performance.

  3. If your vectors are small enough, you may get a significant performance gain by allocating them on the stack. Now, you are allocating them on the heap.

  4. By using template metaprogramming techniques you can eliminate many of the temporaries and unnecessary loops at compile time. To repeat the Wikipedia example here:

    Say you have Vec x = alpha*(u - v); where alpha is a scalar and u and v are Vecs.

    If you implement it in the fashion you are doing it, it will cost you at least 2 temporary vectors (one for u-v and one for the multiplication with alpha) and 2 passing through the memory (2 or 3 loops: one for u-v, one for the multiplication with alpha and one more for the assignment if it is not optimized away).

    If you do template metaprogramming, Vec x = alpha*(u - v); will boil down to a single loop with no temporaries and that is the best you can get. The gain becomes even bigger with more complicated expressions.

    At the moment you don't have these operations but I guess it is only a matter of time that you will need them (weightValueVector() is an indication).

Of course, if you use a linear algebra library, you don't have to know / worry about any of these but you can concentrate on your application instead and get blazing fast code.

OTHER TIPS

Well, I spotted several improvements,

  1. std::vector should be passed by const reference or reference.

  2. you need to check inputs.size() for some of those functions.

  3. it would be useful and faster to offer weightValueVector for taking a reference as parameter and modify its values in place.

On top of answers already provided:
in maxInVec function:
- zero intialize double max
- i would recommend size_t var over unsigned int var = 0
You can also use reserve for your vector when you know the size in advance, to avoid reallocating memory when doing several push_back

Licensed under: CC-BY-SA with attribution
Not affiliated with StackOverflow
scroll top