4

I have implemented this function:

double heron(double a)
{
    double x = (a + 1) / 2;
    while (x * x - a > 0.000001) {
        x = 0.5 * (x + a / x);
    }
    return x;
}

This function is working as intended, however I would wish to improve it. It's supposed to use and endless while loop to check if something similar to x * x is a. a is the number the user should input.

So far I have no working function using that method...This is my miserably failed attempt:

double heron(double a)
{
    double x = (a + 1) / 2;
    while (x * x != a) {
        x = 0.5 * (x + a / x);
    }
    return x;
}

This is my first post so if there is anything unclear or something I should add please let me know.

Failed attempt number 2:

double heron(double a)
{
    double x = (a + 1) / 2;
    while (1) {
        if (x * x == a){
            break;
        } else {
            x = 0.5 * (x + a / x);
        }
    }
    return x;
}

Heron's formula

tk421
  • 5,775
  • 6
  • 23
  • 34
Compil3
  • 137
  • 4
  • 13
  • 4
    You should never really test floating-point variables for equality, especially irrational values. Your second function will probably loop forever in most circumstances. Also, there are faster ways of calculating square roots, but you knew that already, right? – r3mainer Nov 16 '18 at 23:20
  • 4
    `while (x * x != a)` is quite likely always to be true, except for a few values with exact representation. There is a reason why the first example uses an epsilon tolerance. Please see [Is floating point math broken?](http://stackoverflow.com/questions/588004/is-floating-point-math-broken) and [Why Are Floating Point Numbers Inaccurate?](https://stackoverflow.com/questions/21895756/why-are-floating-point-numbers-inaccurate) – Weather Vane Nov 16 '18 at 23:20
  • 3
    This is a rare time that testing floating-point variables for equality is good. Equality indicates the loop is done. The proviso is that the test for equality is not the only test for completion. @WeatherVane The first code's `x * x - a > 0.000001` is a poor use of some epsilon of a FP function. It is meaningless for large values of `x` and too large for small values of `x`. A _relative_ use of some epsilon may make sense here, but not an absolute one as coded. – chux - Reinstate Monica Nov 17 '18 at 02:41

1 Answers1

5

It's supposed to use and endless while loop to check if something similar to x * x is a

Problems:

Slow convergence

When the initial x is quite wrong, the improved |x - sqrt(a)| error may still be only half as big. Given the wide range of double, the may take hundreds of iterations to get close.

Ref: Heron's formula.

For a novel 1st estimation method: Fast inverse square root.

Overflow

x * x in x * x != a is prone to overflow. x != a/x affords a like test without that range problem. Should overflow occur, x may get "infected" with "infinity" or "not-a-number" and fail to achieve convergence.

Oscillations

Once x is "close" to sqrt(a) (within a factor of 2) , the error convergence is quadratic - the number of bits "right" doubles each iteration. This continues until x == a/x or, due to peculiarities of double math, x will endlessly oscillate between two values as will the quotient.

Getting in this oscillation causes OP's loop to not terminate


Putting this together, with a test harness, demonstrates adequate convergence.

#include <assert.h>
#include <math.h>
#include <stdlib.h>
#include <stdio.h>

double rand_finite_double(void) {
  union {
    double d;
    unsigned char uc[sizeof(double)];
  } u;
  do {
    for (unsigned i = 0; i < sizeof u.uc; i++) {
      u.uc[i] = (unsigned char) rand();
    }
  } while (!isfinite(u.d));
  return u.d;
}

double sqrt_heron(double a) {
  double x = (a + 1) / 2;
  double x_previous = -1.0;
  for (int i = 0; i < 1000; i++) {
    double quotient = a / x;
    if (x == quotient || x == x_previous) {
      if (x == quotient) {
        return x;
      }
      return ((x + x_previous) / 2);
    }
    x_previous = x;
    x = 0.5 * (x + quotient);
  }
  // As this code is (should) never be reached, the `for(i)`
  // loop "safety" net code is not needed.
  assert(0);
}

double test_heron(double xx) {
  double x0 = sqrt(xx);
  double x1 = sqrt_heron(xx);
  if (x0 != x1) {
    double delta = fabs(x1 - x0);
    double err = delta / x0;
    static double emax = 0.0;
    if (err > emax) {
      emax = err;
      printf("    %-24.17e %-24.17e %-24.17e %-24.17e\n", xx, x0, x1, err);
      fflush(stdout);
    }
  }
  return 0;
}

int main(void) {
  for (int i = 0; i < 100000000; i++) {
    test_heron(fabs(rand_finite_double()));
  }
  return 0;
}

Improvements

  • sqrt_heron(0.0) works.

  • Change code for a better initial guess.


double sqrt_heron(double a) {
  if (a > 0.0 && a <= DBL_MAX) {
    // Better initial guess - halve the exponent of `a`
    // Could possible use bit inspection if `double` format known.  
    int expo;
    double significand = frexp(a, &expo);
    double x = ldexp(significand, expo / 2);

    double x_previous = -1.0;
    for (int i = 0; i < 8; i++) {  // Notice limit moved from 1000 down to < 10
      double quotient = a / x;
      if (x == quotient) {
        return x;
      }
      if (x == x_previous) {
        return (0.5 * (x + x_previous));
      }
      x_previous = x;
      x = 0.5 * (x + quotient);
    }
    assert(0);
  }
  if (a >= 0.0) return a;
  assert(0);  // invalid argument.
}
chux - Reinstate Monica
  • 143,097
  • 13
  • 135
  • 256
  • Suggest increasing `1000` loop limit before `assert(0);` (maybe `2000`?) With `1000`: `"sqrt_herons_formula: sqrt_herons_formula.c:38: sqrt_heron: Assertion '0' failed."` (I'm not sure how you would back-out an optimal limit, with `2000` code completes in ~3m37sec) – David C. Rankin Nov 17 '18 at 21:29
  • Improved `sqrt_heron` cuts that down to about 14 sec. Impressive. – David C. Rankin Nov 17 '18 at 21:37
  • @david The 1000 comes from a factor of the max power-of-2 for the `double`. (e.g. 10e308). Hmmm, perhaps something else failed? On my machine the max iteration was about 520. How about you? – chux - Reinstate Monica Nov 17 '18 at 21:42
  • @DavidC.Rankin `sqrt` routines are like trig functions. The algorithm to use, once we are in the "primary" range, is well taught, researched, and coded. It is the wrangling the original `x` over the entire FP range to the "primary" range is often the harder problem to do right. In the case of `my_sqrt()`, some non-portable cheat to halved the exponent quickly is needed vs. the portable, clunky, `frexp(), ldexp()` or OP's weak `(a + 1) / 2`. – chux - Reinstate Monica Nov 17 '18 at 21:47
  • Running the computation now. It will be fun to see what the `max` ends up being on my box compared to yours. Only thing I can think of is some strange difference in how the floating point units handle the math. (still waiting on the 3min37s to pass. tic tic tic tic) Max is `1022` on my box. – David C. Rankin Nov 17 '18 at 21:51
  • Also interesting on AMD (FX-8350 Eight-Core), final `assert(0); // invalid argument.` is triggered reporting `a` is `nan`, while adding a check at the end of the `rand_finite_double` `do ... while (!isfinite(u.d) || -u.d > DBL_MAX);` ensures `u.d` should meet the `if (a > 0.0 && a <= DBL_MAX)` condition at the beginning of the improved heron's formula. Must be catching a corner case on that processor. Can you think of another check to include? – David C. Rankin Nov 17 '18 at 22:55
  • @DavidC.Rankin When `!isfinite(u.d)` is not sufficient, yet `(!isfinite(u.d) || -u.d > DBL_MAX)` is, can you post the value that squeaked through? Suggest `"%a"` or `"%.20e"`. – chux - Reinstate Monica Nov 17 '18 at 23:01
  • I guess that check `-u.d > DBL_MAX` is bad anyway as it would be UB to begin with. I tried printing the value with `%f` and get `nan` from `printf`. Can you think of another way to output it? – David C. Rankin Nov 17 '18 at 23:02
  • Let us [continue this discussion in chat](https://chat.stackoverflow.com/rooms/183830/discussion-between-chux-and-david-c-rankin). – chux - Reinstate Monica Nov 17 '18 at 23:02