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

Question

Hi,

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

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

Answer 2

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 3

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 4

Shouldn't it be:

if (counter%1000==0)

Answer 5

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 6

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 7

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 8

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;
}