SanchoGGP / ggp-base

The General Game Playing Base Package
8 stars 4 forks source link

StateMachineTest 5B fails for FDRPSM #196

Closed arr28 closed 9 years ago

arr28 commented 9 years ago

ProverStateMachineTests contained a test (5B) with the following GDL.

;; This tests an edge case when handling GDL.
;; This test case ensures that a player (or state machine)
;; can handle certain patterns of doubled-up variables.
;; (This is a regression test for the ProverStateMachine.)

(role you)
(goal you 100)
(next done)
(<= terminal
    (true done))
(legal you (draw 1 1 1 2))

(<= (next (line ?x1 ?y1 ?x2 ?y2))
    (does ?player (draw ?x1 ?y1 ?x2 ?y2)))

(<= (h_drawn ?x1 ?y ?x2 ?y)
    (does ?player (draw ?x1 ?y ?x2 ?y)))
(<= (next (box ?x1 ?y1))
    (h_drawn ?x1 ?y1 ?x2 ?y1))

The ProverStateMachine can be initialized, produce an initial state, simulate the only legal move and end up in a terminal state.

When FDRPSM attempts to initialize, it hangs indefinitely attempting to do a depth charge (to determine the X-sentence).

arr28 commented 9 years ago

The propnet generated for this GDL is wrong from the start.

      fullPropNet = (ForwardDeadReckonPropNet)OptimizingPolymorphicPropNetFactory.create(
                                                                               description,
                                                                               new ForwardDeadReckonComponentFactory());
      fullPropNet.renderToFile("propnet_001.dot");

Even before any optimizations, this is the generated network.

propnet_001 dot

Note the lack of a done proposition and the way that terminal is fed directly from FALSE.

arr28 commented 9 years ago

Here are the trace-level logs.

22:44:34.357 . GDL
( role you )
( goal you 100 )
( next done )
( <= terminal ( true done ) )
( legal you ( draw 1 1 1 2 ) )
( <= ( next ( line ?x1 ?y1 ?x2 ?y2 ) ) ( does ?player ( draw ?x1 ?y1 ?x2 ?y2 ) ) )
( <= ( h_drawn ?x1 ?y ?x2 ?y ) ( does ?player ( draw ?x1 ?y ?x2 ?y ) ) )
( <= ( next ( box ?x1 ?y1 ) ) ( h_drawn ?x1 ?y1 ?x2 ?y1 ) )

22:44:34.363 . Building propnet
22:44:34.521 . GdlRelation: ( role you )
22:44:34.521 . GdlRelation: ( goal you 100 )
22:44:34.521 . GdlRelation: ( next done )
22:44:34.521 . GdlRelation: ( legal you ( draw 1 1 1 2 ) )
22:44:34.521 . GdlRule: ( <= terminal ( true done ) )
22:44:34.521 . GdlRule: ( <= ( next ( line ?x1 ?y1 ?x2 ?y2 ) ) ( does ?player ( draw ?x1 ?y1 ?x2 ?y2 ) ) )
22:44:34.521 . GdlRule: ( <= ( h_drawn ?x1 ?y ?x2 ?y ) ( does ?player ( draw ?x1 ?y ?x2 ?y ) ) )
22:44:34.521 . GdlRule: ( <= ( next ( box ?x1 ?y1 ) ) ( h_drawn ?x1 ?y1 ?x2 ?y1 ) )
22:44:34.545 . Setting constants...
22:44:34.546 . Done setting constants
22:44:34.546 . Computing topological ordering... 
22:44:34.546 . Done computing topological ordering
22:44:34.548 . Adding sentence form: ( role _ )
22:44:34.548 . Adding sentence form: ( does _ ( draw _ _ _ _ ) )
22:44:34.549 . Adding sentence form: ( next _ )
22:44:34.549 . Adding sentence form: ( true ( line _ _ _ _ ) )
22:44:34.549 . Adding sentence form: ( true _ )
22:44:34.549 . Adding sentence form: ( legal _ ( draw _ _ _ _ ) )
22:44:34.549 . Adding sentence form: ( true ( box _ _ ) )
22:44:34.549 . Adding sentence form: ( next ( line _ _ _ _ ) )
22:44:34.553 . Adding sentence form: ( goal _ _ )
22:44:34.553 . Adding sentence form: ( h_drawn _ _ _ _ )
22:44:34.554 . Adding sentence form: terminal
22:44:34.554 . Adding sentence form: ( next ( box _ _ ) )
22:44:34.555 . Final function info
22:44:34.555 . FunctionInfoImpl [form=( role _ ), dependentSlots=[true], valueMaps=[{[]=you}]]
22:44:34.555 . FunctionInfoImpl [form=( next _ ), dependentSlots=[true], valueMaps=[{[]=done}]]
22:44:34.555 . FunctionInfoImpl [form=( legal _ ( draw _ _ _ _ ) ), dependentSlots=[true, true, true, true, true], valueMaps=[{[1, 1, 1, 2]=you}, {[you, 1, 1, 2]=1}, {[you, 1, 1, 2]=1}, {[you, 1, 1, 2]=1}, {[you, 1, 1, 1]=2}]]
22:44:34.555 . FunctionInfoImpl [form=( goal _ _ ), dependentSlots=[true, true], valueMaps=[{[100]=you}, {[you]=100}]]
22:44:34.555 . Adding transitions...
22:44:34.555 . Setting up 'init' proposition...
22:44:34.555 . Num components before useless removed: 7
22:44:34.555 . Num components after useless removed: 7
22:44:34.556 . Creating component set...
22:44:34.556 . Initializing propnet object...
22:44:34.556 . Num components at end of propnet construction (pre-optimization): 9
arr28 commented 9 years ago

This is the code that's broken (all from OPPNF#create).

    ConstantChecker constantChecker = ConstantCheckerFactory.createWithForwardChaining(model);

<snip>

    for (SentenceForm form : topologicalOrdering)
    {
      ConcurrencyUtils.checkForInterruption();

      LOGGER.trace("Adding sentence form: " + form);

      if (constantChecker.isConstantForm(form))
      {
        // We only add sentence in constant form if they are important (i.e. legal, goal or init).
        if (form.getName().equals(LEGAL) || form.getName().equals(GOAL) || form.getName().equals(INIT))
        {
          for (GdlSentence trueSentence : constantChecker.getTrueSentences(form))
          {
            // Create the proposition and wire it up to the 'true' constant.
            PolymorphicProposition trueProp = xiComponentFactory.createProposition(-1, trueSentence);
            trueProp.addInput(trueComponent);
            trueComponent.addOutput(trueProp);
            components.put(trueSentence, trueComponent);
          }
        }

        addConstantsToFunctionInfo(form, constantChecker, functionInfoMap);
        addFormToCompletedValues(form,
                                 completedSentenceFormValues,
                                 constantChecker);
      }

The constant checker determines that "next _" (i.e. the "form" of next done) is a constant form, which from my limited understanding of this code seems to be reasonable. However, as a result, we neither create a proposition nor wire it up to TRUE.

arr28 commented 9 years ago

Removing the "is this a sufficiently important constant form" check (if (form.getName().equals(LEGAL) || form.getName().equals(GOAL) || form.getName().equals(INIT))) makes the test case pass, but I don't know what other unfortunate side effects it's likely to have. I've asked Alex is he's able to comment.

arr28 commented 9 years ago

I note that it doesn't affect the final propnet size for any of the games that we measure.

    PropNetSize[] lTests = new PropNetSize[] {new PropNetSize("base", "ticTacToe",      244,   168,   168,    57),
                                              new PropNetSize("base", "connectFour",    765,   619,   619,   299),
                                              new PropNetSize("base", "breakthrough",  1844,  1203,  1203,   142),
                                              new PropNetSize("base", "sudokuGrade1",  5483,  4446,  4399,  1722),
                                              new PropNetSize("base", "reversi",      17795,  3997,  3997, 15195),
                                              new PropNetSize("base", "speedChess",   35416, 15710, 15710,  9512),
                                              new PropNetSize("base", "hex",          81507, 61164, 61164,  3250),
                                              new PropNetSize("base", "hexPie",       83023, 82342, 82335,  3300),
                                              };
AlexLandau commented 9 years ago

NEXT should be added to the set of important sentence names. (Arguably TERMINAL as well.)

arr28 commented 9 years ago

Super, thanks Alex. That does the trick (and is hopefully more limited in potentially unwanted side effects).

On 13 April 2015 at 23:15, AlexLandau notifications@github.com wrote:

NEXT should be added to the set of important sentence names. (Arguably TERMINAL as well.)

— Reply to this email directly or view it on GitHub https://github.com/SanchoGGP/ggp-base/issues/196#issuecomment-92515391.