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

[Petr Baudis <pasky@xxxxxxxxxxx>]

Dear diary, on Mon, Feb 25, 2002 at 05:58:07PM CET, I got a letter,
where Gregory Berkolaiko <gberkolaiko@xxxxxxxxxxx> told me, that...
> Ok, you are in for some trouble, boy.

I hope it's nothing too serious, mum.

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

Impossible! ;)))

> 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).

And we don't..?

> 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).

Yup.

> 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.

Ok.

> 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??!?!

Nowhere. Added a FIXME ;). (fixing this is definitively out of scope of this
patch and I really have no time and mainly energy to think about it beyond
understanding your point).

> 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) 

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!!!

That's true. As city is completely different case than a stack (it's many
independent units on one tile, NOT acting as a stack at all), I wouldn't treat
it as a stack at all.

Added a comment about this to kill_desire()'s description.

> I would do it completely differently, but it's a separate story.

Tell us, when bored ;).

> 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.

I've a FIXME for that.

> 2. Your patch doesn't include all occurences of the kill_desire equation.  Fix
> it!!! (see below)

Oh please, don't cry at me.. I wondered why there's so little number of
occurences of this than I expected ;). Done.

> 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.

Can you please write something more organized? I don't really feel like that
now (very tired), and this mail seems somewhat chaotic to me :).

> > 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

Done.

> > +
> > +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".

Oh yes, done.

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

Counted in SHIELDs? (done)

> > +
> > +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!!

Oh wait a little, please - this is IMHO not so simple - first, you get this out
of unit_vulnerability() already squared, so it can be a bit confusing -
however, mainly you can still modify the value after squaring. But ok, it's
probably better to mention that so it's clear that both attack and vuln are
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"

Ok ;). (BTW, Raimar: I don't think dodgy is so unknown; I hear it quite
frequently)

> > @@ -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.

Ok :).

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

;)).

Improved patch attached. I also moved the FIXME, as Raimar requested. I hope I
didn't forget anything you suggested (I've found your review very insightful
and useful, BTW).

-- 

                                Petr "Pasky" Baudis

* elinks maintainer                * IPv6 guy (XS26 co-coordinator)
* IRCnet operator                  * FreeCiv AI hacker
.
No one can feel as helpless as the owner of a sick goldfish.
.
Public PGP key && geekcode && homepage: http://pasky.ji.cz/~pasky/

killdesire.patch
Description: Text document