Reputation: 497

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

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.

Upvotes: 2

Answers (10)

Mr. Vee

Reputation: 1

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

}

Upvotes: 0

Phil H

Reputation: 20151

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.

Upvotes: 2

Mike55

Reputation: 497

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

Upvotes: 0

Todd

Reputation: 2401

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

Upvotes: 0

Jason Berkan

Reputation: 8914

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.
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.

Upvotes: 2

Adam Wright

Reputation: 49396

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.

Upvotes: 20

Moshe Levi

Reputation: 3493

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.

Upvotes: 4