Still something wrong with factor saving/loading

[arr28]

base.connect4simultaneous works fine when starting from scratch, but then fails on the next game. Must be something wrong with saving/loading.

19:35:38.094 ! Exception in player during meta-gaming
java.lang.NullPointerException
    at org.ggp.base.util.statemachine.implementation.propnet.forwardDeadReckon.ForwardDeadReckonPropnetStateMachine.getNextState(ForwardDeadReckonPropnetStateMachine.java:2940) ~[bin/:?]
    at org.ggp.base.player.gamer.statemachine.sancho.TreeNode.expandInternal(TreeNode.java:3330) ~[bin/:?]
    at org.ggp.base.player.gamer.statemachine.sancho.TreeNode.expand(TreeNode.java:2590) ~[bin/:?]
    at org.ggp.base.player.gamer.statemachine.sancho.MCTSTree.setRootState(MCTSTree.java:796) ~[bin/:?]
    at org.ggp.base.player.gamer.statemachine.sancho.GameSearcher.startSearch(GameSearcher.java:951) ~[bin/:?]
    at org.ggp.base.player.gamer.statemachine.sancho.Sancho.stateMachineMetaGame(Sancho.java:1133) ~[bin/:?]
    at org.ggp.base.player.gamer.statemachine.StateMachineGamer.metaGame(StateMachineGamer.java:208) [bin/:?]
    at org.ggp.base.player.request.grammar.StartRequest.process(StartRequest.java:70) [bin/:?]
    at org.ggp.base.player.GamePlayer.run(GamePlayer.java:111) [bin/:?]

[arr28]

The control set really does seem to save and reload correctly. Factor doesn't override toString (yet).

Ah - but is has a dump() method which write debug trace.

[arr28]

Okay - so this is wacky in a way that I don't have time to understand tonight.

When the Factors are first created (by full analysis), each factor gets 2 copies of all the moves in that factor. Factor.dump() prints out the following (manually sorted).

DEBUG Factor                         Factor moves:
DEBUG Factor                           ( drop 1 1 )
DEBUG Factor                           ( drop 1 1 )
DEBUG Factor                           ( drop 2 1 )
DEBUG Factor                           ( drop 2 1 )
DEBUG Factor                           ( drop 3 1 )
DEBUG Factor                           ( drop 3 1 )
DEBUG Factor                           ( drop 4 1 )
DEBUG Factor                           ( drop 4 1 )
DEBUG Factor                           ( drop 5 1 )
DEBUG Factor                           ( drop 5 1 )
DEBUG Factor                           ( drop 6 1 )
DEBUG Factor                           ( drop 6 1 )
DEBUG Factor                           ( drop 7 1 )
DEBUG Factor                           ( drop 7 1 )
DEBUG Factor                           ( drop 8 1 )
DEBUG Factor                           ( drop 8 1 )

And when serialized to file, there are two copies of each move in the serialized representation. See the end of the following...

INFO  RuntimeGameCharacteristics     Factor: v1~( true ( cell 3 2 1 red ) ),( true ( cell 7 1 1 black ) ),( true ( cell 4 1 1 black ) ),( true ( cell 1 6 1 red ) ),( true ( cell 2 6 1 red ) ),( true ( cell 8 3 1 black ) ),( true ( cell 2 1 1 black ) ),( true ( cell 7 3 1 red ) ),( true ( cell 8 4 1 red ) ),( true ( cell 8 3 1 red ) ),( true ( cell 4 5 1 red ) ),( true ( cell 4 6 1 black ) ),( true ( cell 7 6 1 red ) ),( true ( cell 1 3 1 red ) ),( true ( cell 1 1 1 black ) ),( true ( cell 8 1 1 red ) ),( true ( cell 5 2 1 red ) ),( true ( cell 2 2 1 red ) ),( true ( cell 5 4 1 black ) ),( true ( cell 2 4 1 red ) ),( true ( cell 1 5 1 black ) ),( true ( cell 1 4 1 black ) ),( true ( cell 5 4 1 red ) ),( true ( cell 3 6 1 black ) ),( true ( cell 1 3 1 black ) ),( true ( cell 2 5 1 red ) ),( true ( cell 3 5 1 red ) ),( true ( cell 6 1 1 black ) ),( true ( cell 5 5 1 red ) ),( true ( cell 6 5 1 black ) ),( true ( cell 2 2 1 black ) ),( true ( cell 5 6 1 black ) ),( true ( cell 4 4 1 black ) ),( true ( cell 4 4 1 red ) ),( true ( cell 3 6 1 red ) ),( true ( cell 4 2 1 black ) ),( true ( cell 1 2 1 red ) ),( true ( cell 6 1 1 red ) ),( true ( cell 3 4 1 red ) ),( true ( cell 1 6 1 black ) ),( true ( cell 2 6 1 black ) ),( true ( cell 4 3 1 red ) ),( true ( cell 4 3 1 black ) ),( true ( cell 2 3 1 black ) ),( true ( cell 1 5 1 red ) ),( true ( cell 8 1 1 black ) ),( true ( cell 7 5 1 red ) ),( true ( cell 1 2 1 black ) ),( true ( cell 7 4 1 black ) ),( true ( cell 5 3 1 red ) ),( true ( cell 5 6 1 red ) ),( true ( cell 7 2 1 black ) ),( true ( cell 8 2 1 red ) ),( true ( cell 8 2 1 black ) ),( true ( cell 6 3 1 red ) ),( true ( cell 2 1 1 red ) ),( true ( cell 4 1 1 red ) ),( true ( cell 5 1 1 black ) ),( true ( cell 2 3 1 red ) ),( true ( cell 6 3 1 black ) ),( true ( cell 8 6 1 red ) ),( true ( cell 6 6 1 black ) ),( true ( cell 3 5 1 black ) ),( true ( cell 8 5 1 black ) ),( true ( cell 7 3 1 black ) ),( true ( cell 3 3 1 red ) ),( true ( cell 3 1 1 black ) ),( true ( cell 4 2 1 red ) ),( true ( cell 1 1 1 red ) ),( true ( cell 5 5 1 black ) ),( true ( cell 6 4 1 red ) ),( true ( cell 7 6 1 black ) ),( true ( cell 7 2 1 red ) ),( true ( cell 2 4 1 black ) ),( true ( cell 4 5 1 black ) ),( true ( cell 8 5 1 red ) ),( true ( cell 7 5 1 black ) ),( true ( cell 5 2 1 black ) ),( true ( cell 5 1 1 red ) ),( true ( cell 7 4 1 red ) ),( true ( cell 6 6 1 red ) ),( true ( cell 1 4 1 red ) ),( true ( cell 4 6 1 red ) ),( true ( cell 6 4 1 black ) ),( true ( cell 2 5 1 black ) ),( true ( cell 3 2 1 black ) ),( true ( cell 3 4 1 black ) ),( true ( cell 6 2 1 red ) ),( true ( cell 6 2 1 black ) ),( true ( cell 7 1 1 red ) ),( true ( cell 3 3 1 black ) ),( true ( cell 3 1 1 red ) ),( true ( cell 8 4 1 black ) ),( true ( cell 5 3 1 black ) ),( true ( cell 6 5 1 red ) ),( true ( cell 8 6 1 black ) )~( drop 2 1 ),( drop 6 1 ),( drop 5 1 ),( drop 4 1 ),( drop 3 1 ),( drop 7 1 ),( drop 2 1 ),( drop 6 1 ),( drop 4 1 ),( drop 5 1 ),( drop 1 1 ),( drop 1 1 ),( drop 3 1 ),( drop 8 1 ),( drop 8 1 ),( drop 7 1 )

After reloading, we only have 1 of each move in the Set<ForwardDeadReckonLegalMoveInfo> mMoveInfos. That's not surprising. If the String representation was the same (and it was) for two underlying ForwardDeadReckonLegalMoveInfos then the deserialization code will pick the same one twice. On insertion into the Set, the second insert attempt will be ignored.

So, the key problem is either that we have two (nearly) identical FDRLMIs or, more likely, the we can't differentiate them by their string representation.

I wonder what the difference is? Perhaps associated with the FDRLMI is not only the move itself but also which role the move belongs to?

Assuming it's something along these lines then the fix is to create a more verbose toString method for FDRLMI (either by actually updating toString or, if that will affect other parts of the code, by creating a parallel method for use in serialization / deserialization). And if the difference between the apparently identical moves is the role they're associated with then care will be required to ensure that the game still reloads correctly when we're playing the other role. (So using role names instead of the role index might be necessary. At the very least, we'd need to use the raw role index, not the mapped version that we use in most places.)

[SteveDraper]

I think I can explain this one fairly easily (based on the above, not on having tried to actually debug anything though, so take with a pinch of salt).

ForwardDeadReckonLegalMoveInfo encapsulates a choice-by-role (essentially an input proposition), not just a Move. However, its toString() override just outputs the Move element. Each factor will have drops in each column (on one board) for EACH role. Hence there will be a FDRLMI for ( drop 1 1 ) for red, and another for black, but both will output the same string.

Each factor therefore SHOULD have 2 'copies' (they are not actually identical), as should its deserialization.

[SteveDraper]

Confirmed that it crashes after trying tom play with reloaded factors, so it's almost certainly losing the moves for one role based on the above (I'm playing with learning off for now until this is resolved).

[arr28]

Yep - so bug was just as I'd hypothesised.

Now fixed. And I've deleted the saved factors from all games with factors. So safe to re-enable learning.

(Leaving open until I've regenerated and saved the factors for all the factored games.)

[arr28]

Oops - have been committing against #327 by mistake.

[arr28]

Factors for Stanford games regenerated and committed. snap-ci going again.

However - I've discovered a problem with the factoring of chinook. It causes us to select an illegal move every other turn.

08:54:44.250 ? Selected illegal move!!

This doesn't appear to be related to factor saving/reloading because it happens even when there were no saved factors to start with. And it happens with learning disabled. I'll raise it as a separate issue.

[arr28]

All factors regenerated in the new style. Saving & reloading working just fine now.