Is my code suffering from an arithmetic error?

Join the discussion
Ask a follow-up here, or get your own question answered by working scientists, mathematicians and engineers — people, not an autocomplete.
Real named experts · corrections over time · the nuance an AI answer skips
1 reply · 1K views
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:
Physics 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   Reactions: BvU