Crash Pattern Matcher on EvaluationLink with GroundedSchema

[ngeiswei]

The following

(define gl-2 
  (Get
    (Variable "$Y")
    (Evaluation
      (GroundedPredicate "scm: dummy")
      (Variable "$Y"))))

with

(define (dummy A) (stv 1 1))

Crashes opencog/guile.

[ngeiswei]

I suppose the pattern matcher should be interpreting that as

(define gl-3
  (Get
    (Variable "$Y")
    (And
      (Variable "$Y")
      (Evaluation
        (GroundedPredicate "scm: dummy")
        (Variable "$Y")))))

The following returns

Backtrace:
In ice-9/top-repl.scm:
  33: 19 [#<procedure 28f6200 at ice-9/top-repl.scm:31:6 (thunk)> #<procedure 28f6100 at ice-9/top-repl.scm:66:5 ()>]
  76: 18 [#<procedure 28f6100 at ice-9/top-repl.scm:66:5 ()>]
In system/repl/repl.scm:
 142: 17 [start-repl* scheme #f #<procedure prompting-meta-read (repl)>]
 188: 16 [run-repl* # #<procedure prompting-meta-read (repl)>]
In ice-9/boot-9.scm:
 157: 15 [catch #t ...]
In system/vm/trap-state.scm:
 172: 14 [with-default-trap-handler #f ...]
In ice-9/boot-9.scm:
1724: 13 [%start-stack #t #<procedure 2ffc150 at system/repl/repl.scm:189:14 ()>]
1729: 12 [#<procedure 2ffc060 ()>]
 157: 11 [catch quit #<procedure 2ffc030 at system/repl/repl.scm:191:18 ()> ...]
In system/repl/repl.scm:
 201: 10 [#<procedure 2ffc030 at system/repl/repl.scm:191:18 ()>]
In ice-9/boot-9.scm:
 157: 9 [catch #t ...]
In system/vm/trap-state.scm:
 172: 8 [with-default-trap-handler #<procedure debug-trap-handler (frame trap-idx trap-name)> ...]
In ice-9/boot-9.scm:
1724: 7 [%start-stack #t #<procedure 3010840 at system/repl/repl.scm:202:32 ()>]
1729: 6 [#<procedure 3010750 ()>]
In system/repl/repl.scm:
 158: 5 [with-stack-and-prompt #<procedure 30073c0>]
In ice-9/boot-9.scm:
1724: 4 [%start-stack #t #<procedure 30070e0 at system/repl/repl.scm:159:33 ()>]
1729: 3 [#<procedure 3010720 ()>]
In unknown file:
   ?: 2 [opencog-extension cog-satisfying-set-first-n (# -1)]
In ice-9/boot-9.scm:
 157: 1 [catch #t #<catch-closure 30a19a0> ...]
In unknown file:
   ?: 0 [apply-smob/1 #<catch-closure 30a19a0>]

ERROR: In procedure apply-smob/1:
ERROR: Stack overflow
Backtrace:
In ice-9/top-repl.scm:
  33: 19 [#<procedure 28f6200 at ice-9/top-repl.scm:31:6 (thunk)> #<procedure 28f6100 at ice-9/top-repl.scm:66:5 ()>]
  76: 18 [#<procedure 28f6100 at ice-9/top-repl.scm:66:5 ()>]
In system/repl/repl.scm:
 142: 17 [start-repl* scheme #f #<procedure prompting-meta-read (repl)>]
 188: 16 [run-repl* # #<procedure prompting-meta-read (repl)>]
In ice-9/boot-9.scm:
 157: 15 [catch #t ...]
In system/vm/trap-state.scm:
 172: 14 [with-default-trap-handler #f ...]
In ice-9/boot-9.scm:
1724: 13 [%start-stack #t #<procedure 2ffc150 at system/repl/repl.scm:189:14 ()>]
1729: 12 [#<procedure 2ffc060 ()>]
 157: 11 [catch quit #<procedure 2ffc030 at system/repl/repl.scm:191:18 ()> ...]
In system/repl/repl.scm:
 201: 10 [#<procedure 2ffc030 at system/repl/repl.scm:191:18 ()>]
In ice-9/boot-9.scm:
 157: 9 [catch #t ...]
In system/vm/trap-state.scm:
 172: 8 [with-default-trap-handler #<procedure debug-trap-handler (frame trap-idx trap-name)> ...]
In ice-9/boot-9.scm:
1724: 7 [%start-stack #t #<procedure 3010840 at system/repl/repl.scm:202:32 ()>]
1729: 6 [#<procedure 3010750 ()>]
In system/repl/repl.scm:
 158: 5 [with-stack-and-prompt #<procedure 30073c0>]
In ice-9/boot-9.scm:
1724: 4 [%start-stack #t #<procedure 30070e0 at system/repl/repl.scm:159:33 ()>]
1729: 3 [#<procedure 3010720 ()>]
In unknown file:
   ?: 2 [opencog-extension cog-satisfying-set-first-n (# -1)]
In ice-9/boot-9.scm:
 157: 1 [catch #t #<catch-closure 30d02a0> ...]
In unknown file:
   ?: 0 [apply-smob/1 #<catch-closure 30d02a0>]

ERROR: In procedure apply-smob/1:
ERROR: Stack overflow
ERROR: In procedure opencog-extension:
ERROR: Throw to key `C++-EXCEPTION' with args `("cog-satisfying-set-first-n" "Expecting a TruthValue for an evaluatable link: (EvaluationLink\n  (GroundedPredicateNode \"scm: dummy\")\n  (AndLink\n    (VariableNode \"$Y\")\n    (EvaluationLink\n      (GroundedPredicateNode \"scm: dummy\")\n      (VariableNode \"$Y\")\n    )\n  )\n)\n\n (/home/nilg/OpenCog/atomspace/opencog/query/DefaultPatternMatchCB.cc:651)\nFunction args:\n((GetLink\n   (VariableNode \"$Y\")\n   (AndLink\n      (VariableNode \"$Y\")\n      (EvaluationLink\n         (GroundedPredicateNode \"scm: dummy\")\n         (VariableNode \"$Y\")\n      )\n   )\n)\n -1)")'.

Entering a new prompt.  Type `,bt' for a backtrace or `,q' to continue.

[ngeiswei]

Obviously this points to an infinite loop...

[ngeiswei]

The following also crashes

(define gl
  (Get
    (Variable "$X")
     (Inheritance
      (Evaluation
        (GroundedPredicate "scm: dummy")
        (Variable "$X"))
      (Concept "A"))))

while the grounded predicate should obviously not be a candidate for evaluation by the pattern matcher.

The only way I get it to work is to wrap the EvaluationLink in a QuoteLink (and it's argument in an UnquoteLink). Maybe I'll go with that, LocalQuoteLink crashes as well.

[ngeiswei]

Yeah, I can see that this line https://github.com/opencog/atomspace/blob/master/opencog/atoms/pattern/PatternLink.cc#L604 only cares about finding a GroundedPredicate, doesn't care whether it is or not part of a Pattern Matcher propositional query, however it does care whether it quoted or not (as FindAtoms does take into account quotation). So that explains it all.

At this point I think either I fix that, e.i. I make sure an Evaluation can only be an evaluatable if it is in well formed propositional pattern matcher clause (formed by And, Or or Not), or shortcut that problem by wrapping my EvaluationLink with quote and unquote.

[linas]

I get this:

scheme@(guile-user)> (cog-execute! gl-2)
ERROR: In procedure opencog-extension:
ERROR: Throw to key `C++-EXCEPTION' with args `("cog-execute!" "map::at\nFunction args:\n((GetLink\n   (VariableNode \"$Y\")\n   (EvaluationLink\n      (GroundedPredicateNode \"scm: dummy\")\n      (VariableNode \"$Y\")\n   )\n)\n)")'.

[linas]

For me, gl-3 gives an infinite loop.

[linas]

and gl gives:

scheme@(guile-user)> (cog-execute! gl)
ERROR: In procedure opencog-extension:
ERROR: Throw to key `C++-EXCEPTION' with args `("cog-execute!" "map::at\nFunction args:\n((GetLink\n   (VariableNode \"$X\")\n   (InheritanceLink\n      (EvaluationLink\n         (GroundedPredicateNode \"scm: dummy\")\n         (VariableNode \"$X\")\n      )\n      (ConceptNode \"A\")\n   )\n)\n)")'.

Entering a new prompt.  Type `,bt' for a backtrace or `,q' to continue.

[linas]

OK, so I am looking at this, and its somewhat poorly formed. When you wrote gl-2, I think you meant that you wanted gl-3, and that when you wrote gl-3, you really wanted

(define gl-4
  (Get
    (Variable "$Y")
    (And
      (Present (Variable "$Y"))
      (Evaluation
        (GroundedPredicate "scm: dummy")
        (Variable "$Y")))))

A different interpretation for gl-2 is that it should be looking for all EvaluatinLinks of the given form, and never actually evaluating them. Or at least, that is the old backwards-compat version of it.

[linas]

OK, the infinite loop detector is at line 776 of InitiateSearchCB.cc and its supposed to halt at 1000, but the stack overflow happens when it counts to 442. That is, gl-3 and gl-4 have an infinite loop in them.

gl and gl-2 fail differently, because they don't try to ground the variable. I'm trying to fix that, in which case, they will also become infinite loops...

[linas]

There are three alternatives: 1) "don't do that" 2) implement lazy evaluation for GroundedPredicate viz, don't evaluate Variable $Y before passing it in to dummy, which is the root cause of the infinite loop. However, I suspect this will break many usages. It also makes it harder for users of GroundedPredicate because it is not up to them to be non-lazy in the evaluation, and its up to them to avoid any infinite loops that might result. 3) Implement LazyGroundedPredicate for those who really want it.

Meanwhile, I will lower the inf-loop counter from 1000 to 300.

[linas]

Ohh. This is interesting:

(define gl-5
  (Get
    (VariableList (Variable "$Y") (Variable "$Z"))
    (Evaluation
        (GroundedPredicate "scm: dum2")
        (List (Variable "$Y") (Variable "$Z")))))

does not even allow the thing to be created:

ERROR: In procedure cog-new-link:
ERROR: Throw to key `C++-EXCEPTION' with args `("cog-new-link" "Variable not groundable: (VariableNode \"$Y\") ; [5319076102514835978][1]\n\n (/home/linas/src/novamente/src/atomspace-git/opencog/atoms/pattern/PatternLink.cc:853)\nFunction args:\n((VariableList\n   (VariableNode \"$Y\")\n   (VariableNode \"$Z\")\n)\n (EvaluationLink\n   (GroundedPredicateNode \"scm: dum2\")\n   (ListLink\n      (VariableNode \"$Y\")\n      (VariableNode \"$Z\")\n   )\n)\n)")'.

[linas]

And this variant:

(define gl-6
  (Get
    (VariableList (Variable "$Y") (Variable "$Z"))
    (And
        (Present (Variable "$Y"))
        (Present (Variable "$Z"))
        (Evaluation
            (GroundedPredicate "scm: dum2")
            (List (Variable "$Y") (Variable "$Z"))))))

(cog-execute! gl-6)

crashes because the infinite-loop detector is too stupid to discover it. Do recall that infinite-loop detection is turing-undecidable.

[linas]

OK, so when I fix the code to disallow the borken Get/Bind links, I see unit test failures in

AlphaConvertUTest
GlobUTest
BITUTest

I am going through these now. ... never mind. my bad.

[linas]

I tried to write some code that would reject gl-2 as a valid link, but it became too ugly and difficult, so I scrapped that. It seems to be a corner case that seems unlikely to occur in "real code".

[linas]

OK pull #1452 might change some of this behavior, slightly.

I conclude that most of what's reported here is "user error" and don't see what else to do.

[linas]

Nil, I somehow missed your last comment entirely:

Yeah, I can see that this line https://github.com/opencog/atomspace/blob/master/opencog/atoms/pattern/PatternLink.cc#L604 only cares about finding a GroundedPredicate, doesn't care whether it is or not part of a Pattern Matcher propositional query, however it does care whether it quoted or not (as FindAtoms does take into account quotation). So that explains it all.

At this point I think either I fix that, e.i. I make sure an Evaluation can only be an evaluatable if it is in well formed propositional pattern matcher clause (formed by And, Or or Not), or shortcut that problem by wrapping my EvaluationLink with quote and unquote.

and it seems like a good comment, so let me reply to that. The "best" long term solution, it seems to me, would be to create C++ classes for EvaluationLink, And, Or, Not which contain a method is_black_box() which is true whenever these wrap something opaque, such as a GPN.

If you don't do this, then you have to write more complicated code to replace PatternLink.cc#L604 and then you have to cut-n-paste that code into e.g. the unifier. For the moment, it seems that the idea that "atom types are objects" seems to be working, so we may as well stick to that.

[ngeiswei]

For now I have decided to merely wrap any EvaluationLink my pattern miner encouters into Quote/Unquote links and it works so I will not attempt to fix it. Thanks for looking into it and improving the user error handling though.

[linas]

As of right now, I get this:

(use-modules (opencog) (opencog exec))
(define (dummy A) (stv 1 1))
(define gl-2 
  (Get
    (Variable "$Y")
    (Evaluation
      (GroundedPredicate "scm: dummy")
      (Variable "$Y"))))
(cog-execute! gl-2 )
ERROR: In procedure opencog-extension:
Throw to key `C++-EXCEPTION' with args `("cog-execute!" "Backtrace:\n           5 (opencog-extension cog-execute! (#))\n           4 (apply-smob/1 #<catch-closure 56529cb330e0>)\n           3 (apply-smob/1 #<catch-closure 56529cb33060>)\nIn current input:\n    205:0  2 (dummy _)\nIn ice-9/boot-9.scm:\n   748:25  1 (dispatch-exception 0 wrong-number-of-args (#f \"Wrong number…\" …))\nIn unknown file:\n           0 (apply-smob/1 #<catch-closure 56529cb33020> wrong-number-of-args …)\n\nERROR: In procedure apply-smob/1:\nWrong number of arguments to #<procedure dummy (A)>\nABORT: wrong-number-of-args\n (/home/linas/src/novamente/src/atomspace-git/opencog/guile/SchemeEval.cc:1066)\nFunction args:\n((GetLink\n   (VariableNode \"$Y\")\n   (EvaluationLink\n      (GroundedPredicateNode \"scm: dummy\")\n      (VariableNode \"$Y\")\n   )\n)\n)")'

which is ... a "feature" should be a bug: the GroundedPredicate handle expects a ListLink to wrap the args, and it is not getting that.

[linas]

Closing. There is nothing to do here.