Compute mean, variance and standard deviation of CSV number file

Question

I've written my first C++ application this week and I wanted to see if there's anything that aren't up to standards or better ways to do what I'm currently doing.

For the goal of the exercise, let's assume I can't use any other mathematical function other than the square root.

The application reads a file where numbers are on separate lines and computes the mean, variance and standard deviation of all the numbers. Afterwards I print the whole numbers list and the stats that were computed.

#include <iostream>
#include <fstream>
#include <vector>
#include <sstream>
#import <cmath>

using namespace std;

std::vector<int> readFile(const std::string &filePath) {
    ifstream in_file;
    in_file.open(filePath);
    std::vector<int> numbers;

    std::string line;

    while(std::getline(in_file,line,'\r')){
        numbers.push_back(std::stoi(line));
    }

    return numbers;
}

float computeMean(std::vector<int> numbers)
{
    if(numbers.empty()) return 0;

    float total = 0;
    for (int number : numbers) {
        total += number;
    }

    return (total / numbers.size());
}

float computeVariance(float mean, std::vector<int> numbers)
{
    float result = 0;
    for(int number : numbers)
    {
        result += (number - mean)*(number - mean);
    }

    return result / (numbers.size() - 1);
}

int main() {
    std::cout << "Please enter the file path :" << std::endl;
    std::string filePath;
    std::cin >> filePath;
    std::vector<int> numbers = readFile(filePath);
    float mean = computeMean(numbers);
    float variance = computeVariance(mean, numbers);
    float standardDeviation = sqrt(variance);

    std::cout << std::to_string(numbers.size()) + " numbers : ";
    std::string data;
    for(int number : numbers) {
        data += std::to_string(number) + ", ";
    }
    data = data.substr(0, data.length()-2);
    std::cout << data << std::endl;

    std::cout << "Mean : " << std::to_string(mean) << std::endl;
    std::cout << "Variance : " << std::to_string(variance) << std::endl;
    std::cout << "Standard Deviation : " << std::to_string(standardDeviation) << std::endl;
    return 0;
}

Answer 1

Portability

#import is a GCC extension (or perhaps a preview of C++20).

There's no good reason not to simply #include <cmath> here.

Headers and namespaces

We don't use any string-stream, so #include <sstream> can be removed.

Bringing all names in from a namespace is problematic; namespace std particularly so. It can silently change the meaning of your program when you're not expecting it. Get used to using the namespace prefix (std is intentionally very short), or importing just the names you need into the smallest reasonable scope.

In this program, the only places the std:: prefix were missing were std::ifstream and std::sqrt, so this wasn't hard to fix.

readFile()

These lines can be simplified:

std::ifstream in_file;
in_file.open(filePath);

We can ask the constructor to open the file for us:

std::ifstream in_file(filePath);

This loop has some error checking, but it's not complete:

while(std::getline(in_file,line,'\r')){
    numbers.push_back(std::stoi(line));
}

Firstly, we don't expect any carriage-return in the input file (we opened it in text mode, so on systems that use CR as line delimiter, they will be converted to \n). Secondly, std::stoi throws exceptions when the string cannot be converted, but we probably also want to check whether there are leftover, unconverted characters after our integer (e.g. if someone thought they could supply decimal values).

computeMean()

Why return a float rather than double? Single-precision floats are normally used only where the storage size is an important consideration, which is not the case here. (Note that on many platforms, double is the natural (and fastest) size of floating-point.)

We should pass the vector by reference to a const object, as we don't want to make a copy or to modify the value.

Instead of returning zero when there are no members, perhaps we should return a NaN value (which is more consistent with arithmetic 0.0 / 0):

if (numbers.empty())
    return std::numeric_limits<double>::quiet_NaN();

This loop:

double total = 0;
for (int number : numbers) {
    total += number;
}

can be written (with #include <numeric>) as

double total = std::accumulate(numbers.begin(), numbers.end(), 0.0);

computeVariance()

We need to be clear which variance (sample or population) we're returning. We're also missing a size check similar to that for the mean.

Apart from that, the comments above for computeMean() are relevant:

double computeSampleVariance(const double mean, const std::vector<int>& numbers)
{
    if (numbers.size() <= 1u)
        return std::numeric_limits<double>::quiet_NaN();

    auto add_square = [mean](double sum, int i)
        {
            auto d = i - mean;
            return sum + d*d;
        };
    double total = std::accumulate(numbers.begin(), numbers.end(), 0.0, add_square);
    return total / (numbers.size() - 1);
}

Single-pass algorithm

It is possible to compute the mean and variances in a single pass - but not using the (exact-arithmetic) methods you likely learnt in high school, which suffer from lack of precision with the inexact floating-point types we can use. The topic is too deep for this review, but if you research Welford's Algorithm, you will find reference implementations to guide you.

That said, for your purposes, the straightforward two-pass algorithm is probably appropriate, and it's easy to read and understand, so I wouldn't recommend changing it unless you reach a point where your input set becomes too large to hold in a vector (and even then, only if you can't read the file multiple times).

---

My version

#include <algorithm>
#include <cmath>
#include <fstream>
#include <iostream>
#include <iterator>
#include <limits>
#include <numeric>
#include <vector>

std::vector<int> readFile(const std::string& filePath)
{
    std::ifstream in_file(filePath);
    std::istream_iterator<int> start{in_file}, end;
    std::vector<int> numbers;
    std::copy(start, end, std::back_inserter(numbers));
    return numbers;
}

double computeMean(const std::vector<int>& numbers)
{
    if (numbers.empty())
        return std::numeric_limits<double>::quiet_NaN();

    return std::accumulate(numbers.begin(), numbers.end(), 0.0) / numbers.size();
}

double computeSampleVariance(const double mean, const std::vector<int>& numbers)
{
    if (numbers.size() <= 1u)
        return std::numeric_limits<double>::quiet_NaN();

    auto const add_square = [mean](double sum, int i) {
        auto d = i - mean;
        return sum + d*d;
    };
    double total = std::accumulate(numbers.begin(), numbers.end(), 0.0, add_square);
    return total / (numbers.size() - 1);
}

int main()
{
#ifdef TEST
    const std::vector<int> numbers = { -2, -1, 1, 2, 100000-2, 100000-1, 100000+1, 100000+2};
#else
    std::cout << "Please enter the file path :" << std::endl;
    std::string filePath;
    std::cin >> filePath;
    const std::vector<int> numbers = readFile(filePath);
#endif

    double mean = computeMean(numbers);
    double variance = computeSampleVariance(mean, numbers);
    double standardDeviation = std::sqrt(variance);

    std::cout << numbers.size() << " numbers : ";
    auto separator = "";
    for (int number: numbers) {
        std::cout << separator << number;
        separator = ", ";
    }
    std::cout << std::endl;

    std::cout << "Mean : " << std::to_string(mean)
              << "Variance : " << std::to_string(variance)
              << "Standard Deviation : " << std::to_string(standardDeviation) << std::endl;
    return 0;
}

Answer 2

You could compute both the mean and variance on a single pass which removes the need for storing the numbers.

\$\begin{align} \sigma(x) \times (n-1) &= \sum( (\overline x-x_i)^2 ) \\ &= \sum( \overline x^2 &-&2 \overline x \times x_i &+ x_i^2 ) \\ &= n\times\overline x^2 &-& 2 \overline x\times\sum x_i &+ \sum x_i^2 \\ &= n\times(\sum x_i/n)^2&-& 2 \sum(x_i)/n\times\sum x_i &+ \sum x_i^2\\ &= 1/n(\sum x_i)^2 &-& 2/n (\sum(x_i))^2 &+ \sum x_i^2\\ &= -1/n (\sum x_i)^2 &&& +\sum x_i^2 \end{align}\$

So all you need is is to get the sum of the values and the sum of the squares of the values.

However if the values are large and the variance is small then you can run into stability issues. So instead you can subtract a constant (pick the first value) from each value to compute the variance.

std::pair<float, float> computeVarianceAndMean(std::vector<int> const& numbers)
{
    float sum = 0;
    float sumAdjusted= 0;
    float sumSquares = 0;
    int constant = numbers.front();
    for(int number : numbers)
    {
        sum += number;
        sumAdjusted += number-constant;
        sumSquares += (number-constant)*(number-constant)
    }
    float average = sum / (numbers.size());
    float variance = (-sumAdjusted/numbers.size() + sumSquares)/(numbers.size()-1);
    return std::make_pair(average , variance);
}

Answer 3

In computeMean and computeVariance, you can pass the numbers vector in by const &. This avoids making a copy of the vector.

If you only have one number in your file, computeVariance will return a NaN, because it'll divide 0 by 0. There are two different ways to calculate variance. Are you using the correct one? This will have an effect on the computed standard deviation.

When you output all the numbers, don't build a string. Just output the numbers. The comma can be handled by outputting it before the number, if the number isn't the first one.

bool first = true;
for (auto number: numbers) {
    if (!first) std::cout << ", ";
    first = false;
    std::cout << number;
}
std::cout << std::endl;

endl will flush the output stream, and since you're doing more output right away you could use '\n' instead.

When outputting the results, just output the number; don't convert it to a string first.