finding pi - using Machin's formula. Different iterations are giving same result

Question

I've written a few programs to find pi, this one being the most advanced. I used Machin's formula, pi/4 = 4(arc-tan(1/5)) - (arc-tan(1/239)).

The problem is that however many iterations I do, I get the same result, and I can't seem to understand why.

#include "stdafx.h"
#include <iostream>
#include <iomanip>
#include <math.h>
using namespace std;

double arctan_series(int x, double y) // x is the # of iterations while y     is the number
{
    double pi = y;
    double temp_Pi;
    for (int i = 1, j = 3; i < x; i++, j += 2) 
    {
        temp_Pi = pow(y, j) / j; //the actual value of the iteration
        if (i % 2 != 0)     // for every odd iteration that subtracts
        {
            pi -= temp_Pi;
        }
        else    // for every even iteration that adds
        {
            pi += temp_Pi;
        }
    }
    pi = pi * 4;
    return pi;
}

double calculations(int x) // x is the # of iterations
{
    double value_1, value_2, answer;
    value_1 = arctan_series(x, 0.2);
    value_2 = arctan_series(x, 1.0 / 239.0);
    answer = (4 * value_1) - (value_2);
    return answer;
}

int main() 
{
    double pi;
    int iteration_num;
    cout << "Enter the number of iterations: ";
    cin >> iteration_num;
    pi = calculations(iteration_num);
    cout << "Pi has the value of: " << setprecision(100) << fixed << pi     << endl;
    return 0;

}

Answer 1

I have not been able to reproduce your issue, but here is a bit cleaned up code with a few C++11 idioms and better variable names.

#include <iostream>
#include <iomanip>
#include <math.h>

using namespace std;

// double arctan_series(int x, double y) // x is the # of iterations while y     is the number
// then why not name the parameters accoringly? In math we usually use x for the parameter.
// prefer C++11 and the auto notation wherever possible
auto arctan_series(int iterations, double x) -> double
{
    // note, that we don't need any temporaries here.

    // note, that this loop will never run, when iterations = 1
    // is that really what was intended?
    for (int i = 1, j = 3; i < iterations; i++, j += 2) 
    {
        // declare variables as late as possible and always initialize them
        auto t = pow(x, j) / j;
        // in such simple cases I prefer ?: over if-else. Your milage may vary
        x += (i % 2 != 0) ? -t : t;
    }
    return x * 4;
}

// double calculations(int x) // x is the # of iterations
// then why not name the parameter accordingly
// BTW rename the function to what it is supposed to do
auto approximate_pi(int iterations) -> double
{
    // we don't need all of these temporaries. Just write one expression.
    return 4 * arctan_series(iterations, 0.2) - arctan_series(iterations, 1.0 / 239.0);
}

auto main(int, char**) -> int
{
    cout << "Enter the number of iterations: ";
    // in C++ you should declare variables as late as possible
    // and always initialize them.
    int iteration_num = 0;
    cin >> iteration_num;

    cout << "Pi has the value of: "
         << setprecision(100) << fixed
         << approximate_pi(iteration_num) << endl;
    return 0;
}

When you remove my explanatory comments, you'll see, that the resulting code is a lot more concise, easier to read, and therefore easier to maintain.

I tried a bit:

Enter the number of iterations: 3
Pi has the value of: 3.1416210293250346197169164952356368303298950195312500000000000000000000000000000000000000000000000000

Enter the number of iterations: 2
Pi has the value of: 3.1405970293260603298790556436870247125625610351562500000000000000000000000000000000000000000000000000

Enter the number of iterations: 7
Pi has the value of: 3.1415926536235549981768144789384678006172180175781250000000000000000000000000000000000000000000000000

Enter the number of iterations: 42
Pi has the value of: 3.1415926535897940041763831686694175004959106445312500000000000000000000000000000000000000000000000000

As you see, I obviously get different results for different numbers of iterations.