Calculating the value of pi-what is wrong with my code

Question

I'm doing another C++ exercise. I have to calculate the value of pi from the infinite series:

pi=4 - 4/3 + 4/5 – 4/7 + 4/9 -4/11+ . . .

The program has to print the approximate value of pi after each of the first 1,000 terms of this series. Here is my code:

#include <iostream>
using namespace std;

int main()
{
    double pi=0.0;
    int counter=1;

    for (int i=1;;i+=2)//infinite loop, should "break" when pi=3.14159
    {
        double a=4.0;
        double b=0.0;

        b=a/static_cast<double>(i);

        if(counter%2==0)
            pi-=b;
        else
            pi+=b;

        if(i%1000==0)//should print pi value after 1000 terms,but it doesn't  
            cout<<pi<<endl;

        if(pi==3.14159)//this if statement doesn't work as well
            break;

        counter++;
    }

    return 0;
}

It compiles without errors and warnings, but only the empty console window appears after execution. If I remove line” if(i%1000==0)” , I can see it does run and print every pi value, but it doesn’t stop, which means the second if statement doesn’t work either. I’m not sure what else to do. I’m assuming it is probably a simple logical error.

Answer 1

Well, i % 1000 will never = 0, as your counter runs from i = 1, then in increments of 2. Hence, i is always odd, and will never be a multiple of 1000.

The reason it never terminates is that the algorithm doesn't converge to exactly 3.14157 - it'll be a higher precision either under or over approximation. You want to say "When within a given delta of 3.14157", so write

if (fabs(pi - 3.14157) < 0.001)
  break

or something similar, for however "close" you want to get before you stop.

Answer 2

Since you start i at 1 and increment by 2, i is always an odd number, so i % 1000 will never be 0.

Answer 3

you have more than one problem:

A. i%1000==0 will never be true because you're iterating only odd numbers.

B. pi==3.14159 : you cannot compare double values just like that because the way floating point numbers are represented (you can read about it here in another question). in order for it to work you should compare the values in another way - one way is to subtract them from each other and check that the absolute result is lower than 0.0000001.

Answer 4

1. You have floating point precision issues. Try if(abs(pi - 3.14159) < 0.000005).
2. i%1000 will never be 0 because i is always odd.

Answer 5

Shouldn't it be:

if (counter%1000==0)

Answer 6

1. i starts at 1 and then increments by 2. Therefore i is always odd and will never be a multiple of 1000, which is why if (i % 1000 == 0) never passes.

2. Directly comparing floats doesn't work, due to floating precision issues. You will need to compare that the difference between the values is close enough.

Answer 7

pi=4 - 4/3 + 4/5 – 4/7 + 4/9 -4/11 + ...

Generalising

pi = Σ_i=0^∞ (-1)ⁱ 4 / (2i+1)

Which gives us a cleaner approach to each term; the i'th term is given by:

double term = pow(-1,i%2) * 4 / (2*i+1);

where i=0,1,2,...,N

So, our loop can be fairly simple, given some number of iterations N

int N=2000;
double pi=0;
for(int i=0; i<N; i++)
{
    double term = pow(-1,i%2) * 4 / (2*(double)i+1);
    pi += term;
    cout << i << "\t" << pi <<endl;
}

Your original question stated "The program has to print the approximate value of pi after each of the first 1,000 terms of this series". This does not imply any need to check whether 3.14159 has been reached, so I have not included this here. The pow(-1,i%2) call is just to avoid if statements (which are slow) and prevent any complications with large i.

Be aware that after a number of iterations, the difference between the magnitude of pi and the magnitude of the correcting term (say -4/25) will be so small that it will go beyond the precision of a double, so you would need higher precision types to deal with it.

Answer 8

By default abs uses the abs macro which is for int. For doubles, use the cmath library.

#include <iostream>
#include <cmath>

int main()
{
    double pi=0.0;

    double a=4.0;
    int i = 1; 

    for (i=1;;i+=2)
    {

        pi += (1 - 2 * ((i/2)%2)) * a/static_cast<double>(i);          

        if( std::abs(pi - 3.14159) < 0.000001 )
              break;

        if (i > 2000) //1k iterations
              break;
    }

    std::cout<<pi<<std::endl;

    return 0;
}

Answer 9

Here is the corrected code. I thought it may be helpful in the future if somebody has similar problem.

#include <iostream>
#include <cmath>

using namespace std;

int main()
{
double pi=0.0;
int counter=1;

for (int i=1;;i+=2)
{
 double a=4.0;
 double b=0.0;

 b=a/static_cast<double>(i);

 if(counter%2==0)
  pi-=b;
 else
  pi+=b;

 if(counter%1000==0) 
  cout<<pi<<" "<<counter<<endl;


 if (fabs(pi - 3.14159) < 0.000001) 
  break;

 counter++;
}
cout<<pi;

 return 0;
}

Answer 10

Here is a better one:

class pi_1000
{
public:
    double doLeibniz( int i ) // Leibniz famous formula for pi, source: Calculus II :)
    {
        return ( ( pow( -1, i ) ) * 4 ) / ( ( 2 * i ) + 1 );
    }

 void piCalc()
{
    double pi = 4;
    int i;

    cout << "\npi calculated each iteration from 1 to 1000\n"; //wording was a bit confusing.
                                                    //I wasn't sure which one is the right one: 0-1000 or each 1000th.
    for( i = 1; i < 1000; i++ )
    {
        pi = pi + doLeibniz( i );
        cout << fixed << setprecision( 5 ) << pi << "\t" << i + 1 << "\n";
    }

    pi = 4;
    cout << "\npi calculated each 1000th iteration from 1 to 20000\n";
    for( i = 1; i < 21000; i++ )
    {
        pi = pi + doLeibniz( i );
        if( ( ( i - 1 ) % 1000 )  == 0 )
            cout << fixed << setprecision( 5 ) << pi << "\t" << i - 1 << "\n";
    }

}