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

Implement LocalQuoteLink #980

Closed
ngeiswei opened this issue Nov 7, 2016 · 17 comments
Closed

Implement LocalQuoteLink #980

ngeiswei opened this issue Nov 7, 2016 · 17 comments

Comments

@ngeiswei
Copy link
Member

ngeiswei commented Nov 7, 2016

The idea is to have a quote link that only applies to the direct child but not its descendants, as initially suggested in #132. This has the potential to simplify many PLN rules. For instance the following

 (QuoteLink
   (ImplicationScopeLink
     (UnquoteLink (VariableNode "$TyVs"))
     (UnquoteLink (VariableNode "$P"))
     (UnquoteLink (VariableNode "$Q"))))

would be replaced by

 (LocalQuoteLink
   (ImplicationScopeLink
     (VariableNode "$TyVs")
     (VariableNode "$P")
     (VariableNode "$Q")))

It's not very urgent but worth introducing I believe.

@linas
Copy link
Member

linas commented Nov 7, 2016

Yes, its a good idea. From what I can tell, almost all uses of quote/unquote are actually localquote.

Side comment: handling full quote/unquote semantics in the pattern matcher turned out to be very painful and difficult and confusing, I spent a difficult week untangling it.

@ngeiswei
Copy link
Member Author

ngeiswei commented Nov 8, 2016

I decided to work on that next as it's gonna simplify my subsequent work.

@linas
Copy link
Member

linas commented Nov 8, 2016

OK. Well, please don't invent a brand-new framework to handle it. Just alter PatternTerm as needed. In fact, we should probably use pattern term in more places, not just in the pattern matcher, because the kind of stuff that you are doing is very top-down hierarchical and asymmetric, and for that, PatternTerm seems to be the correct concept.

@linas
Copy link
Member

linas commented Nov 8, 2016

Um, I hope this will not collide with your work, but I want to move PatternTerm.h PatternTerm.cc and Pattern.h from the opencog/query directory to the opencog/atoms/pattern directory.

@ngeiswei
Copy link
Member Author

ngeiswei commented Nov 8, 2016

Oh, OK, let me push what I've done so far then.

@ngeiswei
Copy link
Member Author

ngeiswei commented Nov 8, 2016

I've merged my work so far, if you can be done with you changes by the end today that would be ideal, that way I can resume tomorrow, otherwise I'll switch to another task till you're done. Basically if your plan is to use PatternTerm more through out the pattern matcher code that is super welcome, it would definitely make completing LocalQuote implementation much easier.

@linas
Copy link
Member

linas commented Nov 8, 2016

See pull req #988 for movement of header files.

Here's the general idea: PatternTerm should be renamed: maybe to AtomHolder or AtomParent or something like that. Its kind-of-like a special-case of incoming set, or an inverted incoming set: given an atom, you can see who is holding that atom... and, most importantly, if the holder has quoted the atom. Thus, for any given atom, it can be used to tell you about the "context" of that atom.

Right now, only the pattern matcher needs to know about this kind of context; however, given how popular quoting has become, its probably the case that other subsystems might need it, also. Maybe. So I'm looking at it as a kind of generic concept for the atomspace, and it would be best if it got re-used instead of re-invented.

@ngeiswei
Copy link
Member Author

ngeiswei commented Nov 9, 2016

That sounds like an excellent idea. QUOTE_LINK has already found it's way to the AtomTable and ScopeLink classes. So I'm thinking, if you intend to do that change soon I'll pause the implementation of LocalScopeLink and resume after you're done. If you don't intend to do it soon, I'll complete the LocalScopeLink first.

@linas
Copy link
Member

linas commented Nov 9, 2016

I have no plans to do any such work. I'm merely point it out as something to keep in mind.

@ngeiswei
Copy link
Member Author

Hmm, there's http://wiki.opencog.org/w/DontExecLink, I think LocalQuoteLink encompasses it.

@linas
Copy link
Member

linas commented Nov 10, 2016

yes. without thinking very hard about it, I opened #992

@ngeiswei
Copy link
Member Author

ngeiswei commented Nov 15, 2016

@linas For now a hack is used by ignoring variables when seeing an UNQUOTE_LINK, as in https://github.com/opencog/atomspace/blob/master/opencog/atoms/core/ScopeLink.cc#L119, but maybe the ScopeLink::factory shouldn't have been called over a quoted ScopeLink in the first place, what do you think?

@ngeiswei
Copy link
Member Author

Basically, in order to have LocalQuoteLink work with ScopeLink, I'm thinking, either

  1. Change the code so that ScopeLink::factory is never called over quoted scope link
  2. Change the ScopeLink constructors to have a quoted flag and ignore the variables when that flag is true.

@linas
Copy link
Member

linas commented Nov 15, 2016

Heh. But its impossible to know when a scope link might be quoted, or not! That's the whole point of PatternTerm. Consider this:

(define sco (ScopeLink ...))
(define ba (BindLink (Variable $x) (AndLink sco foo)))
(define bb (BindLink (Variable $x) (AndLink (QuoteLink sco) bar)))

These are added in sequential order. When sco is added, its .. fresh, not quoted or unquoted, its top level, the constructor must run. When ba is added, sco is unquoted, and it must resolve and be the same atom that sco is. Constructor does not run, because sco is already in the atomspace. When bb is added, the scope is quoted -- but again, the constructor does not run, because sco already is in the atomspace. So, for both ba and bb, the ctor never ran. Worse: one is and the other is not quoted. (or local-quoted).

The above problem illustrates why I said, earlier, that I am really starting to hate quotation. Quotation breaks the idea that an atom is the same, always: because a quoted atom is not the same as an unquoted one, but it has to be the same to make the atomspace work. So .. its the same but not the same .. the context-dependence is ugly.

I guess I should get used to it -- the ContextLink .. actually, any link at all, presents similar issues. Its just that the issues are the most obvious for quote/localquote.

@ngeiswei
Copy link
Member Author

You're absolutely right. What I take from that is handling quotation should be the job of the algorithm, pattern matcher, etc, using the quoted atoms.

@ngeiswei
Copy link
Member Author

OK, I'm beginning to see the light! Basically the problem is that a ScopeLink used as pattern may not match a ScopeLink used as grounding, even though they may the same UUID as they are used in different contexts! Now I understand why DefaultPatternMatchCB::is_self_ground is so complicated. I don't think supporting LocalQuote should be so hard though, hopefully.

@ngeiswei
Copy link
Member Author

There might be some cases left but the bulk of the work is done so I'm closing.

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