opencog / atomspace

The OpenCog (hyper-)graph database and graph rewriting system
https://wiki.opencog.org/w/AtomSpace
Other
822 stars 234 forks source link

Can't tell the PM to set a connector as not evaluatable #490

Closed ngeiswei closed 8 years ago

ngeiswei commented 8 years ago

Let's add the following

scheme@(guile-user)> (And (Concept "A") (Concept "B")) 
scheme@(guile-user)> (define x (Variable "$X"))
scheme@(guile-user)> (define y (Variable "$Y"))
scheme@(guile-user)> (define concept-type (Type "ConceptNode"))
scheme@(guile-user)> (define var-decl (VariableList (TypedVariable x concept-type) (TypedVariable y concept-type)))
scheme@(guile-user)> (define body (AndLink x y))
scheme@(guile-user)> (define pat (GetLink var-decl (AndLink body)))

Now let's try to match it

scheme@(guile-user)> (cog-satisfying-set pat)
$5 = (SetLink
)

It should match A and B as groundings for x and y, not the empty set.

I found that the problem comes from PatternLink::trace_connectives that wrongly infers that (AndLink x y) is evaluatable. This is of course wrong given the type of the variables x and y which are ConceptNode. If I disable the trace_connectives call it works as expected.

I thought using QuoteLink and UnquoteLink might work but apparently not, as you may see:

scheme@(guile-user)> (define q-body (QuoteLink (AndLink (UnquoteLink x) (UnquoteLink y))))
scheme@(guile-user)> (define q-pat (GetLink var-decl (AndLink q-body)))
ERROR: In procedure cog-new-link:
ERROR: Throw to key `C++-EXCEPTION' with args `("cog-new-link" "The variable (VariableNode \"$X\") ; [5][1]\n does not appear (unquoted) in any clause! (/home/nilg/OpenCog/atomspace/opencog/atoms/pattern/PatternLink.cc:474)")'.

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

I see 2 ways to fix this bug:

  1. Fix QuoteLink/UnquoteLink (I think I recall that UnquoteLink isn't yet supported). Alternatively one might implement LocalQuoteLink.
  2. Make evaluatable on the connectors more correct.

I'm not sure yet what would be the right way to go, I guess both fixes are useful in the long-run. I'll probably pick up the one which I find easier.

ngeiswei commented 8 years ago

I'm probably try to implement limited support for LocalQuoteLink by merely upgrading PatternLink::trace_connectives to escape connectors wrapped with a LocalQuoteLink.

linas commented 8 years ago

OK, so LocalQuote is identical to quote, we already had and resolved this discussion by email, so please don;t invent a new way. Just finish the partially implemented Unquote so that it does the right thing.

Re the semantics of the AndLink and the semantics of ConceptNode, and the interplay of TV's and connectives: This is exactly what I was talking about (and partially coded up) with the connectives. Instead of over-loading AndLink to have multiple meanings, it might be better to have a CrispAndLink which work with crisp true-false truth values, and another AndLink called SetIntersectionLink that implements Bayesian probability as an axiomatic set intersection, (and SetUnion instead of OrLink, SetSubgraction for conditional probabilities, etc.)

That way, the things that pattern matcher treats as logical connectives are well defined, and does not interfere with the search for other link types which should not be interpreted.

To restate: the problem you are having with searching for AndLink is that the PM is also trying to interpret the AndLink. One proposal is to use QuoteLink/UnquoteLink to mean "interpret/don't interpret". But It might be easier to just invent new links that are iinterpreted, and revert AndLink, etc. to not be interpreted.

This is what I mean: the semantics of the PM and the semantics of PLN are different, and here we see where they collide.

ngeiswei commented 8 years ago

I agree about creating a CrispAndLink, etc for the pattern matcher. Regarding LocalQuote, yes one can use QuoteLink and UnquoteLink (if properly implemented), however LocalQuote has the advantage of being terser in that situation, and I suspect will often be the case, which is why I still think it is worth adding.

Anyway, I'll see what is easier to code for now.

linas commented 8 years ago

I don't like LocalQuote because it introduces new weird semantics (that frankly, I never understood) -- whereas most or all functional programming languages already have quotes that are well-defined, that people are already familiar with. So I was suggesting just sticking with the well-known variant.

ngeiswei commented 8 years ago

It looks like UnquoteLink is at least already partially supported, hopefully the only place I need to add its support is in PatternLink::trace_connectives. If that's the case I'm fine to just complete UnquoteLink support.

ngeiswei commented 8 years ago

Looks like adding support in PatternLink::trace_connectives isn't enough. Anyway, we do need full support for UnquoteLink, so let's do it...

ngeiswei commented 8 years ago

The way Unquote is supposed to behave with multiple QuoteLinks is not entirely clear to me though.

If I have

   QuoteLink
      AndLink
         UnquoteLink
            VariableNode "$X"
         VariableNode "$Y"

Then pretty clearly VariableNode "$X" is unquoted. But if I have

QuoteLink
   QuoteLink
      AndLink
         UnquoteLink
            VariableNode "$X"
         VariableNode "$Y"

Should X be unquoted or quoted?

The existing code seems a bit contradicting. In particular PatternTerm::remQuote decreases PatternTerm::_quote_depth, and this combined with isQuoted() that considers the term is quoted if _quote_depth of the parent is greater than 0 means that to unquote a term one needs multiple unquotes.

There is other code in PatternMatchEngine::tree_compare, says

    if (ptm->isQuoted() and UNQUOTE_LINK == tp)
        return quote_compare(ptm, hg);

That seems to contradict that (or rather makes utterly no sense).

Anyway, regardless of whether the current code makes sense, we need to agree on the precise behavior of Quote and Unquote. There is no wiki page for UnquoteLink.

ngeiswei commented 8 years ago

They do not say here https://www.gnu.org/software/guile/manual/html_node/Expression-Syntax.html#index-quasiquote if quasiquote should increment the level of quasiquotation, but here http://docs.racket-lang.org/reference/quasiquote.html they say so.

So I guess we could follow the Racket convention, although I don't know how useful it is in our case. But let's do and try... Of course any opinion on that matter is welcome.

ngeiswei commented 8 years ago

I've added an embryo of wiki page for UnquoteLink http://wiki.opencog.org/wikihome/index.php/UnquoteLink and I'll proceed with the suggested implementation, unless I realize it doesn't make sense later on.

linas commented 8 years ago

yes, our quotelink works like a scheme quasiquote. Yes, to implement unquote correctly, it seems like quote-counting would be required. Yes, part of implementing unquote would be to add the needed count increment when a quote is seen.

linas commented 8 years ago

Closing because it looks like this is fixed:

(cog-satisfying-set q-pat)

$5 = (SetLink
   (ListLink
      (ConceptNode "A")
      (ConceptNode "B")
   )
   (ListLink
      (ConceptNode "B")
      (ConceptNode "A")
   )
)

My guess is that it was probably pull req #688 and that this bug might be a duplicate of bug #646

If this is still an issue, please open a new bug report

linas commented 8 years ago

Huh. There is a different bug: this doesn't work as I might have guessed:

(define n-body (DontExec (And x y)))
(define n-pat (GetLink var-decl (AndLink n-body)))
(cog-satisfying-set n-pat)
linas commented 8 years ago

I opened #735 to track above ... it seems like the DontExecLink is kind of like the LocalQuote proposal, which I did not understand when it was first proposed. Hey Nil, when I reject or stall or debate your ideas, make sure I understand them as you intended; reading above, it seems like I did not. (that is, if the idea was that LocalQuote should work exactly like DontExec)

ngeiswei commented 8 years ago

If DontExecLink applies only on the direct child, but not the subtree, then yes it is equivalent to LocalQuote.

linas commented 8 years ago

Applies to whole tree.