[Freeciv-Dev] Re: [PATCH] [1.2] move some common AI equations to kill_desire() (PR#1279)

[Gregory Berkolaiko <gberkolaiko@xxxxxxxxxxx>]

Ok, you are in for some trouble, boy.

First of all, I don't think the desire equation is fully correct.

To be mathematically sound it has to be of the following form

Desire = Probability_To_Win * Amount_Won - Probability_To_Lose * Amount_Lost

This would estimate your average win from the action (if you perform this
action N times, N is big, you'd be N*Desire better off).

The probabilities are estimated in this way:
Probability_To_Win = Attack_Rating / (Attack_Rating + Defence_Rating)
Probability_To_Lose = Defence_Rating / (Attack_Rating + Defence_Rating)
and it's alright (although win_chance would be the exact value).

What troubles me the most is the use of victim_count and of ai.f (it's added
to Amount_Lost).
NB: the next line, which you didn't bother to convert to use kill_desire and
you should, basically says "if there are enough units on the job, we don't
need this one".  Add a comment please.

The problem with ai.f is that bigger it is, less is our desire to go help
other units.  Now suppose we need five cavalries to take over a city, we have
four (which is not enough), then we will be severely discouraged to build the
fifth one.  Where is logic in this??!?!

The problem with victim_count is that if you do simple mathematical
operations, you can reduce the equation to the form:
Desire = Attack_Rating * Amount_Won - Attack_Rating * Amount_Lost 
         / (Attack_Rating / Victim_Count + Defence_Rating) 

That is, with all other quantities constant and Victim_Count big our desire to
go and bash this city (possibly alone) would go up.  But they are NOT
helppless Victims, it's a city, not a stack!!!

I would do it completely differently, but it's a separate story.
So the bottom line is:
1. Victim_Count is used in a wrong way.  It can be argued that you patch
doesn't aim to fix this, just to make it more clear.
2. Your patch doesn't include all occurences of the kill_desire equation.  Fix
it!!! (see below)
3. This is a complicated issue.  I wish you could edit readme.AI to include
the basic explanation for the equation there.  You can cut-n-paste my email,
write your own, whatever.

> > Index: advdomestic.c
> ===================================================================
> RCS file: /home/cvs/aiciv/freeciv-a2/ai/advdomestic.c,v
> retrieving revision 1.1.1.5
> retrieving revision 1.20
> diff -u -r1.1.1.5 -r1.20
> Index: advmilitary.c
> ===================================================================
> RCS file: /home/cvs/aiciv/freeciv-a2/ai/advmilitary.c,v
> retrieving revision 1.1.1.9
> retrieving revision 1.2
> diff -u -r1.1.1.9 -r1.2
> --- advmilitary.c     22 Feb 2002 19:32:40 -0000      1.1.1.9
> +++ advmilitary.c     24 Feb 2002 21:36:20 -0000      1.2
> @@ -620,7 +620,7 @@
>                unit_types[i].move_type == HELI_MOVING) && acity &&
>                acity->ai.invasion == 2) b0 = f * SHIELD_WEIGHTING;
>        else {
> -        b0 = (b * a - (f + (acity ? acity->ai.f : 0)) * d) * g *
> SHIELD_WEIGHTING / (a + g * d);
> +        b0 = kill_desire(b, a, (f + (acity ? acity->ai.f : 0)), d, g);
>          if (acity && b * acity->ai.a * acity->ai.a > acity->ai.f * d)
>            b0 -= (b * acity->ai.a * acity->ai.a - acity->ai.f * d) *
>                             g * SHIELD_WEIGHTING / (acity->ai.a *
                    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
                      this is the same equation

> acity->ai.a + g * d);
> @@ -826,7 +826,7 @@
>                unit_types[v].move_type == HELI_MOVING) && acity &&
>                acity->ai.invasion == 2) b0 = f * SHIELD_WEIGHTING;
>      else {
> -      b0 = (b * a - (f + (acity ? acity->ai.f : 0)) * d) * g *
> SHIELD_WEIGHTING / (a + g * d);
> +      b0 = kill_desire(b, a, (f + (acity ? acity->ai.f : 0)), d, g);
>        if (acity && b * acity->ai.a * acity->ai.a > acity->ai.f * d)
>          b0 -= (b * acity->ai.a * acity->ai.a - acity->ai.f * d) *
>                 g * SHIELD_WEIGHTING / (acity->ai.a * acity->ai.a + g * d);
                  ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
                             ditto


> Index: aiunit.c
> ===================================================================
> RCS file: /home/cvs/aiciv/freeciv-a2/ai/aiunit.c,v
> retrieving revision 1.1.1.10
> retrieving revision 1.46
> diff -u -r1.1.1.10 -r1.46
> --- aiunit.c  23 Feb 2002 21:03:31 -0000      1.1.1.10
> +++ aiunit.c  25 Feb 2002 13:46:30 -0000      1.46
> @@ -616,6 +616,35 @@
>  }
>  
>  /**************************************************************************
> +Compute how much we want to kill certain victim we've chosen.
> +
> +Benefit is something like 'attractiveness' of the victim, how nice it would
> be
> +to destroy it. Larger value, worse loss for enemy.

Add that it's usually counted in SHIELDS

> +
> +Attack is the total possible attack power we can throw on the victim. Note
> that
> +we will even square this.

The second part is heritage from findvictim.  Change to "usually comes
squared".

> +
> +Loss is the possible loss when we would lose the unit we want to attack.

ditto

> +
> +Vuln is vulnerability of our unit when attacking the enemy.

it's actually defence power of the defender, but it's sort of the same thing.
Also squared!!

> +
> +Victim count is number of victims stacked in the target tile. FIXME?: The
> +equation is not accurate as the other values can vary for other units on
> the
> +target tile (we take values from best defender), however I believe it's
> +accurate just enough now and lost speed isn't worth that.

If you want, can add a comment from me: "Extremely dodgy usage of victim
count"

> +**************************************************************************/
> +int kill_desire(int benefit, int attack, int loss, int vuln, int
> victim_count)
> +{
> +  int desire;
> +
> +  /*         attractiveness     danger */ 
> +  desire = ((benefit * attack - loss * vuln) * victim_count *
> SHIELD_WEIGHTING
> +            / (attack + vuln * victim_count));
> +
> +  return desire;
> +}
> +
> +/**************************************************************************
>  Military "want" estimates are amortized in this complicated way.
>  COMMENTME: Why not use simple amortize? -- GB
>  **************************************************************************/
> @@ -805,20 +834,10 @@
>                  unit_owner(pdef)->name, unit_type(pdef)->name);
>          
>        } else {
> +        /* See description of kill_desire() about this variables. */
>          int vuln = unit_vulnerability(punit, pdef);
> -        
> -        /* The total possible attack power we can throw on the victim. Note
> -         * that we will even square this. */
>          int attack = reinforcements_value(punit, pdef->x, pdef->y) +
> bellig;
> -        
> -        /* Something like 'attractiveness' of the victim, how nice it would
> be
> -         * to destroy it. Larger value, worse loss for enemy. */
>          int benefit = unit_type(pdef)->build_cost;
> -        
> -        /* We're only dealing with adjacent victims here. */
> -        int move_cost = 0;
> -        
> -        /* The possible loss when we would lose the unit we want to attack.
> */
>          int loss = unit_type(punit)->build_cost;
>          
>          attack *= attack;
> @@ -836,15 +855,10 @@
>          /* If we have non-zero belligerence... */
>          if (attack > 0 && is_my_turn(punit, pdef)) {
>            int desire;
> -          
> -          /* TODO: This equation is simplified version of much worse ones
> in
> -           * that long fat routines, but it's still a common pattern, so we
> -           * will can this equation to one separate readable function.
> We'll
> -           * also be able to remove move_cost and loss variables. */
> -         
> -          /*         attractiveness     danger */ 
> -          desire = ((benefit * attack - loss * vuln) * SHIELD_WEIGHTING
> -                    / (attack + vuln) - move_cost * SHIELD_WEIGHTING);
> +
> +          /* FIXME? Why we don't use stack_size as victim_count? --pasky */
> +
> +          desire = kill_desire(benefit, attack, loss, vuln, 1);
>            
>            /* No need to amortize! We're doing it in one-turn horizon (?).

Don't think yourself stupid, remove this question mark, you are absolutely
right.

> */
>            
> @@ -1520,7 +1534,7 @@
>            else if ((is_ground_unit(punit) || is_heli_unit(punit)) &&
>                     acity->ai.invasion == 2) b0 = f * SHIELD_WEIGHTING;
>            else {
> -            b0 = (b * a - (f + acity->ai.f) * d) * g * SHIELD_WEIGHTING /
> (a + g * d);
> +            b0 = kill_desire(b, a, (f + acity->ai.f), d, g);
>              if (b * acity->ai.a * acity->ai.a > acity->ai.f * d)
>                 b0 -= (b * acity->ai.a * acity->ai.a - acity->ai.f * d) *
>                        g * SHIELD_WEIGHTING / (acity->ai.a * acity->ai.a + g
                          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^


> * d);
> Index: aiunit.h
> ===================================================================
> RCS file: /home/cvs/aiciv/freeciv-a2/ai/aiunit.h,v
> retrieving revision 1.1.1.4
> retrieving revision 1.7
> diff -u -r1.1.1.4 -r1.7
> --- aiunit.h  19 Feb 2002 19:33:54 -0000      1.1.1.4
> +++ aiunit.h  24 Feb 2002 21:36:21 -0000      1.7
> @@ -37,6 +37,7 @@
>  int unit_vulnerability_basic(struct unit *punit, struct unit *pdef);
>  int unit_vulnerability_virtual(struct unit *punit);
>  int unit_vulnerability(struct unit *punit, struct unit *pdef);
> +int kill_desire(int benefit, int attack, int loss, int vuln, int
> attack_count);

good boy, now I can easily fix autoattack if I ever receive the long-promised
savegame from Raahul


G.

__________________________________________________
Do You Yahoo!?
Everything you'll ever need on one web page
from News and Sport to Email and Music Charts
http://uk.my.yahoo.com