Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

StateMachineTest 5B fails for FDRPSM #196

Closed
arr28 opened this issue Feb 24, 2015 · 7 comments
Closed

StateMachineTest 5B fails for FDRPSM #196

arr28 opened this issue Feb 24, 2015 · 7 comments

Comments

@arr28
Copy link

arr28 commented Feb 24, 2015

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
Copy link
Author

arr28 commented Apr 13, 2015

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 added a commit that referenced this issue Apr 13, 2015
...instead of their old location in c:\temp.
@arr28
Copy link
Author

arr28 commented Apr 13, 2015

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
Copy link
Author

arr28 commented Apr 13, 2015

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
Copy link
Author

arr28 commented Apr 13, 2015

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
Copy link
Author

arr28 commented Apr 13, 2015

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
Copy link

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

@arr28
Copy link
Author

arr28 commented Apr 13, 2015

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
#196 (comment).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants