Work in progress (was : [gnugo-devel] a patch from a newbie :))

[Portela Fernand]

Hi all,

Ok, here's what I did so far :

- added 2 new move reasons
    OWL_ATTACK_MOVE_GAIN
    OWL_DEFENSE_MOVE_LOSS
  The associated .what field references the biggest affected _worm_
- added 2 new fields in the dragon_data structure :
    int owl_attack_kworm;
    int owl_defense_kworm;
  which account for the (k)illed worm in case of GAIN/LOSS
  attack/defense codes
- added supporting code in :
    dragon.c :
      make_dragons()
    move_reasons.c
      get_pos()
      owl_attack_move_reason_known()
      owl_defense_move_reason_known()
      get_biggest_owl_target()
      add_owl_attack_move()
      add_owl_defense_move()
      get_saved_worms()
      mark_changed_dragon()
      list_move_reasons()
    owl.c
      owl_attack()           needs a supplementary parameter
      do_owl_attack()
      owl_defend()           needs a supplementary parameter
      owl_reasons()
      owl_confirm_safety()
      catalog_goal()         has been resurrected
    value_moves.c
      examine_move_safety()
      estimate_territorial_value()

   [note: changes due to the change of prototype of owl_attack() and 
          owl_defend() are not listed here]

Observations:

* It should always be possible to verify that the added code doesn't
  break the previous comportement of the engine, simply by deactivating
  the small portion of code in do_owl_attack(). I have run the
  regression tests like this right now and I haven't observed any
  differences with the breakage of the current CVS.
* Only owl_attack() and owl_defend() have been modified. It must be
  extended to other functions like owl_does_attack()
* I'm not too sure if I correctly coded the moves valuations, but it
  doesn't look too bad at first (see below)
* There are plenty small FIXMEs left behind, specially in play_gtp.c and
  sgfdecide.c. I guess the tracing macros also would possibly need an
  update...
  [Of course, I'm willing to do the dirty job, but I'd prefer first to
  know if there's any chance for this patch to get in the CVS]
* Almost forgot another TODO: I haven't looked at the move reasons
  discarding mechanism.
* I haven't noticed any change in the performance. I looked at the timings
  of some tests, patch active and inactive (but code present in the binary),
  the differences were random (plus and minus) and a couple 1/100 secs only.

Current regression breakage with patch applied:

---- File [blunder.tst]
12 PASSED
20 PASSED
---- File [lazarus.tst]
7 FAILED: Correct 'T5', got 'H3'
---- File [nngs.tst]
2040 PASSED
---- File [strategy4.tst]
219 PASSED
---- File [nngs2.tst]
40 PASSED

- D4 in blunder:12 is slightly undervalued this time (only 11 points)
- I'm having a look at lazarus:7, it looks "fishy". H3 gets clearly over-
  valued. Haven't a clue why. It might have something to do with the
  problem described below.


Question: does the above description look good enough or is there already
something wrong and I should stop/revise that work ?

Now, I come to my main problem :

At first, I didn't want to mess with the caching. So I implemented the
worm tracking mechanism with some new global variables in owl.c. Of course, 
this isn't satisfactory, since it is possible that a GAIN/LOSS code could
be returned with the wrong worm information.

After some thought, I think we don't have a choice : the information
needs to be somewhere in the cache. Without it, the patch is inefficient
and probably buggy. So :

- since it's unused for the OWL_ATTACK and OWL_DEFEND routines, would it
  be possible and/or wise to (ab)use the move2 field ? 

I've rapidly taken a look at the macros, it seems feasible.


/nando

PS: damn, that's a lot of work, for deceptively few PASSes... oh well, I'm
    having fun here, so... :)