[Freeciv-Dev] Re: (PR#12834) amortize()

["Jason Short" <jdorje@xxxxxxxxxxxxxxxxxxxxx>]

<URL: http://bugs.freeciv.org/Ticket/Display.html?id=12834 >

Benoit Hudson wrote:
> <URL: http://bugs.freeciv.org/Ticket/Display.html?id=12834 >
> 
> On Mon, Apr 18, 2005 at 02:47:03PM -0700, Brian Dunstan wrote:
> 
>><URL: http://bugs.freeciv.org/Ticket/Display.html?id=12834 >
>>
>>I noticed that the amortize() function uses a very
>>clever, but complicated, algorithm.
> 
> 
> This code is astounding.  Why don't we do this calculation in
> floating-point?  It would be (with casts to doubles made beforehand):
>         double discount = 1.0 - 1.0 / ((double)MORT);
>         return benefit * pow(discount, delay);
> or
>         return (int) (benefit * pow(discount,delay) + 0.5);
> if you want to round.

We probably do want to round but the value could be negative.  I thought
there was a round() function but my gcc claims otherwise.

Anyway, here's a patch.

-jason

Index: server/settlers.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/server/settlers.c,v
retrieving revision 1.225
diff -u -r1.225 settlers.c
--- server/settlers.c   13 Apr 2005 18:19:13 -0000      1.225
+++ server/settlers.c   19 Apr 2005 03:20:44 -0000
@@ -16,6 +16,7 @@
 #endif
 
 #include <assert.h>
+#include <math.h>
 #include <stdio.h>
 #include <string.h>
 
@@ -94,38 +95,15 @@
   terms this means we calculate how much less worth something is to us
   depending on how long it will take to complete.
 
-  amortize(benefit, delay) returns benefit * ((MORT - 1)/MORT)^delay
-  (^ = to the power of)
-
-  Plus, it has tests to prevent the numbers getting too big.  It takes
-  advantage of the fact that (23/24)^12 approximately = 3/5 to chug 
-  through delay in chunks of 12, and then does the remaining 
-  multiplications of (23/24).
+  This is based on a global interest rate as defined by the MORT value.
 **************************************************************************/
 int amortize(int benefit, int delay)
 {
-  int num = MORT - 1;
-  int denom;
-  int s = 1;
-  assert(delay >= 0);
-  if (benefit < 0) { s = -1; benefit *= s; }
-  while (delay > 0 && benefit != 0) {
-    denom = 1;
-    while (delay >= 12 && (benefit >> 28) == 0 && (denom >> 27) == 0) {
-      benefit *= 3;          /* this is a kluge but it is 99.9% accurate and 
saves time */
-      denom *= 5;      /* as long as MORT remains 24! -- Syela */
-      delay -= 12;
-    }
-    while ((benefit >> 25) == 0 && delay > 0 && (denom >> 25) == 0) {
-      benefit *= num;
-      denom *= MORT;
-      delay--;
-    }
-    if (denom > 1) { /* The "+ (denom/2)" makes the rounding correct */
-      benefit = (benefit + (denom/2)) / denom;
-    }
-  }
-  return(benefit * s);
+  double discount = 1.0 - 1.0 / ((double)MORT);
+
+  /* Note there's no rounding here.  We could round but it would probably
+   * be better just to return (and take) a double for the benefit. */
+  return benefit * pow(discount, delay);
 }
 
 /**************************************************************************