Influence code cleanup

[arend]

The influence code is currently a bit of a mess, and needs a major cleanup. In no specific order:

• The interface needs review: Do we really need so many ways to export influence data?
• Technically, all interface prototypes should be in influence.h, and influence.c should only include that and board.h, but not liberty.h.
• Tests like q == escape_influence are forbidden. In fact, it would be cleaner to have the struct influence_data variables allocated by the users, instead of the global variables move_influence etc. (I.e. making influence.c pretty much completely OO.)
• Influence pattern matching is a mess. Clean it up!
• No dependence on global variables, please! (doing_scoring, use_optimistic_territory)
• color parameter to escape_influence seems superfluous (except for cashing).
• Enhancement patterns may be superfluous, only used in escape influence right now.
• Unfortunately, the code in value_territory() is quite complex. Not clear anything can be done about it.
• Do we still need EXPLICIT_LOOP_UNROLLING with modern compilers?

[gunnar]

• The interface needs review: Do we really need so many ways to export influence data?
  • Probably not, but I can't say I'm up to speed on the current set of options. For debugging the gtp functions are mostly sufficient in conjunction with view.pike, except when one wants to find what patterns matched in the influence computation for a specific move.
• Technically, all interface prototypes should be in influence.h, and influence.c should only include that and board.h, but not liberty.h.
  • That was not how it once was designed. The purpose of influence.h was to provide data structures and constants for the internal use of influence.c. I don't mind changing it but then most of the current content should be moved into influence.c.
• Tests like q == escape_influence are forbidden. In fact, it would be cleaner to have the struct influence_data variables allocated by the users, instead of the global variables move_influence etc. (I.e. making influence.c pretty much completely OO.)
  • Forbidden in what sense? It's not so simple to let the users allocate the influence_data without exposing them to the full details of the struct. It can be done with a create function inside influence.c but then we're forced to use dynamic allocation, which I'm not quite sure we want.
• Influence pattern matching is a mess. Clean it up!
  • Yes, please do.
• No dependence on global variables, please! (doing_scoring, use_optimistic_territory)
  • Passing around doing_scoring in any other way is going to hurt quite a bit. use_optimistic_territory is maybe easier to get rid of.
• color parameter to escape_influence seems superfluous (except for cashing).
  • Historical reasons I would assume.
• Enhancement patterns may be superfluous, only used in escape influence right now.
  • They were used more in the past, weren't they? If they aren't needed I don't mind if they go away.
• Unfortunately, the code in value_territory() is quite complex. Not clear anything can be done about it.
  • Doesn't look like that, without drastically changing algorithm.
• Do we still need EXPLICIT_LOOP_UNROLLING with modern compilers?
  • Probably. Already when it was introduced the modern compilers of that time were expected not to need it, but it turned out to have a substantial impact. Testing is the only way to know of course.

[arend]

Don't pass color on from compute_escape_influence(); check for q->is_territorial_influence instead q != &escape_influence

[arend]

Remove influence_get_moyo_segmentation(), it was unused.

[arend]

Mechanical code cleanup and removal of obviously unused fragments

[arend]

Remove color parameter from many functions, it is redundant with q->color_to_move.

[arend]

Remove checks for attack_codes

[arend]

Remove influence_get_moyo_data(); its only caller compute_surrounding_moyo_sizes() can use whose_moyo_restricted() directly instead.

[arend]

Don't compute area/moyo/territory segmentation, nobody was using it.

[arend]

Remove use_optimistic_territory and whose_loose_territory(), both unused

[arend]

Simplify influence_callback(), no logic changed.

[arend]

Indentation and similar trivia.

[arend]

The patches arend_7_9.1a-j capture the available low-hanging fruit. They remove quite a bit that was just unused. There is one functional change, in that references to cut_possible(), attack() and worm[].attack_codes were removed. The breakage of this is rather random but positive:

nicklas2:2102   PASS PASS [PASS]
nicklas2:2103   PASS PASS [PASS]
trevor:412      FAIL B3 [C4]
ninestones:370  FAIL O4 [R5]
olympiad2004:3  PASS P5 [!Q10]
Total nodes: 1656178030 3278751 12457489
(-1.2% reading nodes, rest almost unchanged)

[arend]

I will close the ticket for now, since I am not planning to do more about this in the short term.

For the record, I think these are the things still worth doing IMO:

• Use influence.h as the interface, and don't include liberty.h from influence.c. This would enforce at compile-time that the influence code is a separate module, and doesn't do callbacks to the engine, or interpret worm/dragon status etc.
• It might be worth putting all data of which we need a copy for black and white (q->black_permeability etc.) into an array of size 2. Then we could replace the frequent idiom

  if (color == BLACK)
    q->black_permeability...
  else
    q->white_permeability...
  

by

q->c[color - 1].peremability...

• Test whether EXPLICIT_LOOP_UNROLLING is still necessary (after porting some obvious optimizations over from the explicitly unrolled case into the loop).