Look for symmetrical position in cache.

[TFiFiE]

As suggested in #1084 (comment).

I took the liberty of changing the benchmark position to one in which this doesn't cause a speedup, because it's not typical for the game as a whole.

[gcp]

How does the performance look?

Edit: To explain, I'm very skeptical about code that can only be useful for at most a few moves per game. But if this is basically free, it's worth considering.

[TFiFiE]

This gives a speed boost for the first three moves or so and any performance hit afterwards is too insignificant for me to detect.

[gcp]

The other PR broke this, but I'll need a bit to review this anyway, wrt calc_hash changes, performance impact and the need for the rotation table (which relates to performance impact again).

[gcp]

@TFiFiE can you do a me a favor and rebase this?

[Hersmunch]

How much of a pain would it be to make it so there is only one of these tables? Or is there a reason why it makes sense to have one for each object?

[TFiFiE]

It's tied to the board size, which is variable for this class.

[Hersmunch]

Of course. Can totally see it now, thanks.
I had it stuck in my head as being stored in an X by Y array which would be the same for all sizes but no, it's a vertex array.

[gcp]

If we can live with an extra indirection you can make it a static class owned vector (indexed by board size) and just check if that is initialized the moment you initialize FastBoard.

Alternatively, do the calculation on the fly...if necessary limit the symmetry check to the first 10-20 moves or so?

That's some of the things I want to look at.

[Hersmunch]

Name clash now that rotation parameter has been renamed. Sorry about that..

[AncalagonX]

I have been running this in my custom LZ build (essentially LZ_next + roy7's LCB_max_root) for a week now with Lizzie, testing it over the course of several dozen real-life 9dan vs. LZ games.

I have noticed no downsides or performance decreases with this PR. For the first few moves when symmetrical positions are extremely likely, it indeed massively boosts reported visit counts for valid symmetrical board root moves as expected.

Although I cannot personally confirm one way or the other whether this PR's implementation has or does not have any bugs in aggregating symmetrical move scores or whether it truly results in better move selection, I can say half empirically and half anecdotally that: 1) I have noticed no errors, misplays, or bizarrities from enabling this PR (which presumably is in effect for the entire game?). 2) I have noticed no decrease in n/sec overall, although on this point I have only conducted the bare minimum of performance testing and comparison, and didn't keep records of my limited testing results.

If there are no identifiable problems in how this is implemented, this PR could be enabled soon for selfplay and provide a mild boost to early-game search strength.

Though do note that -v 3200 might interfere with the greatly boosted visit counts from this PR—and so perhaps a special allowance of extra visits for early game symmetrical positions should be given, or else a "true visits" value should be tallied separately from the boosted "true + symmetrically_identical_position_visits" search results for the purposes of correctly enforcing a full search with 3200 real visits. Otherwise, using the visit counts directly from this PR might result in early-game searches erroneously ending search calculations hundreds or thousands of visits earlier than the intended 3200v mark.

This is a simple issue to address in any case, but I cannot fully interpret the code above on my own so it's even possible this effect has already been accounted for, in which case disregard.

[gcp]

This doesn't interfere with visit counting AFAIK, just makes it faster.

I basically only want to see if we can get the extra array out of FastBoard, and if getting rid of it would ( to keep performance parity) require limiting this to the first moves of the game.

[TFiFiE]

Why would you want such a change if only one FastBoard object is typically constructed and FastBoard::reset_board isn't called all the time?

[AncalagonX]

I've only conducted rather shallow testing regarding performance, just enough to satisfy me to continue running it. I recall noticing zero performance degradation from my one or two haphazard tests from a week or so ago. I can't quantify or shed light on performance impact (or lack there of) any further than that.

That said, it's pretty clear the benefit of collating symmetrical moves is nearly guaranteed to be negligible beyond the first handful number of moves. I'd be happy with a max of 10 moves, just judging by gut feelings. It probably goes without saying, but if we end up limiting this, then further considerations might need to be made for symmetrical play involving pre-placed handicap stones depending on how LZ's internal move count interacts with the set move limit.

[gcp]

if only one FastBoard object is typically constructed

Because it might be quite useful to construct multiple, for example if asked to list dead stones.

[gcp]

I guess this should be disabled during training? Else we can't take advantage of rotational asymmetry in the opening.

[gcp]

I reworked this a bit in #1421.

[gcp]

I can't measure a performance difference, and the other pull addresses the self-play games, so going to close this.

[TFiFiE]

@gcp: Can you reopen this?