Is my code suffering from an arithmetic error?

Click For Summary
The discussion centers on a method for approximating the value of pi using fractions, specifically by finding integers n and d that minimize the difference between pi and the fraction n/d. The implementation utilizes the decimal data type for higher precision compared to double, despite having a smaller range. The method iterates through possible denominators within a specified range and calculates corresponding numerators, tracking the closest fraction to pi. A notable point of critique is the potential for an error in the return value, as the method performs integer division for n and d, which could lead to incorrect results when returning a double. This highlights the importance of ensuring the correct data type is used in calculations to avoid inaccuracies.
SlurrerOfSpeech
Messages
141
Reaction score
11
What it does is described well in my comments. I'm using decimal, which is more precise than double but has a smaller range of values. https://msdn.microsoft.com/en-us/library/ms228360(v=vs.90).aspx

C:
        const decimal pi = 3.141592653589793238462643383279502884197169399375105820974944592307816406286208998628034m;

        /// <summary>
        /// Finds n,d that minimize |pi - n/d|, where n is in the range 1, 2, ...
        /// and d is in the range [min, max]
        /// </summary>
        /// <remarks>
        /// Can assume min > 0 and min <= max
        /// </remarks>
        static double FindPiFraction(long min, long max, out long n, out long d)
        {
            // Smallest distance between a fraction and pi. Can initialize to Decimal.MaxValue for now.
            decimal smallest = decimal.MaxValue;

            // Placeholders for n, d to satisfy compiler. They are guaranteed to get set to other values below.
            n = -1;
            d = -1;

            // for each possible denominator
            for (long i = min; i <= max; ++i)
            {
                // Begin iterating through numberators. Begin at i * pi rounded down, e.g. if i = 1000
                // then we want to start at 3140. Iterate until our fraction is greater than pi. Keep track
                // of minimum distance between a fraction and pi, and keep track of the numerator and
                // denominator of that fraction.
                for(long j = (long)Math.Floor(Math.PI * (double)i); ; ++j)
                {

                    decimal fraction = (decimal)j / (decimal)i;
                    decimal diff = fraction - pi;
                    decimal distance = diff < 0 ? diff * -1 : diff;
                    if(distance < smallest)
                    {
                        smallest = distance;
                        n = j;
                        d = i;
                    }
                    if(fraction > pi)
                    {
                        break;
                    }
                }
            }
            ReduceFraction(ref n, ref d);
            return n / d;
        }
 
Last edited by a moderator:
Technology news on Phys.org
SlurrerOfSpeech said:
What it does is described well in my comments. I'm using decimal, which is more precise than double but has a smaller range of values. https://msdn.microsoft.com/en-us/library/ms228360(v=vs.90).aspx

C:
        const decimal pi = 3.141592653589793238462643383279502884197169399375105820974944592307816406286208998628034m;

        /// <summary>
        /// Finds n,d that minimize |pi - n/d|, where n is in the range 1, 2, ...
        /// and d is in the range [min, max]
        /// </summary>
        /// <remarks>
        /// Can assume min > 0 and min <= max
        /// </remarks>
        static double FindPiFraction(long min, long max, out long n, out long d)
        {
            // Smallest distance between a fraction and pi. Can initialize to Decimal.MaxValue for now.
            decimal smallest = decimal.MaxValue;

            // Placeholders for n, d to satisfy compiler. They are guaranteed to get set to other values below.
            n = -1;
            d = -1;

            // for each possible denominator
            for (long i = min; i <= max; ++i)
            {
                // Begin iterating through numberators. Begin at i * pi rounded down, e.g. if i = 1000
                // then we want to start at 3140. Iterate until our fraction is greater than pi. Keep track
                // of minimum distance between a fraction and pi, and keep track of the numerator and
                // denominator of that fraction.
                for(long j = (long)Math.Floor(Math.PI * (double)i); ; ++j)
                {

                    decimal fraction = (decimal)j / (decimal)i;
                    decimal diff = fraction - pi;
                    decimal distance = diff < 0 ? diff * -1 : diff;
                    if(distance < smallest)
                    {
                        smallest = distance;
                        n = j;
                        d = i;
                    }
                    if(fraction > pi)
                    {
                        break;
                    }
                }
            }
            ReduceFraction(ref n, ref d);
            return n / d;
        }
Both n and d are declared as long, so n/d is computed using integer division, but you're returning a value of type double.

For example, if n is 19 and d is 10, the value returned will be the double value 1.0. This is very much a rookie error.
 
Last edited:
  • Like
Likes BvU
Learn If you want to write code for Python Machine learning, AI Statistics/data analysis Scientific research Web application servers Some microcontrollers JavaScript/Node JS/TypeScript Web sites Web application servers C# Games (Unity) Consumer applications (Windows) Business applications C++ Games (Unreal Engine) Operating systems, device drivers Microcontrollers/embedded systems Consumer applications (Linux) Some more tips: Do not learn C++ (or any other dialect of C) as a...

Similar threads

  • · Replies 2 ·
Replies
2
Views
2K
  • · Replies 54 ·
2
Replies
54
Views
5K
  • · Replies 2 ·
Replies
2
Views
2K
  • · Replies 49 ·
2
Replies
49
Views
11K
  • · Replies 125 ·
5
Replies
125
Views
20K
  • · Replies 3 ·
Replies
3
Views
7K
  • · Replies 52 ·
2
Replies
52
Views
13K
  • · Replies 48 ·
2
Replies
48
Views
12K
  • Poll Poll
  • · Replies 2 ·
Replies
2
Views
7K
  • · Replies 6 ·
Replies
6
Views
3K