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

Equality at pattern compile time (static-analysis unification) #554

Open
linas opened this issue Dec 27, 2015 · 13 comments
Open

Equality at pattern compile time (static-analysis unification) #554

linas opened this issue Dec 27, 2015 · 13 comments

Comments

@linas
Copy link
Member

linas commented Dec 27, 2015

Consider the following:

(use-modules (opencog) (opencog query) (opencog exec))
(Inheritance (Concept "foo") (Concept "baz"))
(Inheritance (Concept "bar") (Concept "baz"))

(define b
   (BindLink
      (And
         (Inheritance (Concept "foo") (Variable "$x"))
         (Equal (Variable "$x") (Variable "$y"))
      )
      (Variable "$y")))

ERROR: Throw to key `C++-EXCEPTION' with args `("cog-new-link" "Variable
not groundable: (VariableNode \"$y\") ; [782][1]\n\n
(/src/atomspace-git/opencog/atoms/pattern/PatternLink.cc:776)")'.

Obviously, we should be able to deduce, whatever x is, that is what y should be, so the above should execute fine, and do what we think it should, instead of throwing.

@linas
Copy link
Member Author

linas commented May 30, 2020

Another example:

(Get
   (And
      (Equal (Variable "?AGENT") (Concept "Andrew"))
      (Evaluation
         (Predicate "leaving")
         (List (Variable "?AGENT") (Variable "?LOCATION")))
     (Inheritance
         (Variable "?LOCATION") (Concept "movie-theatre"))))

The above can be reduced at pattern-compile-time, by plugging in (Concept "Andrew") for (Variable "?AGENT") throughout the entire pattern. Thus, the complexity of the search can be reduced.

This can be generally performed whenever the EqualLink has a "straight-forward" structure, e.g. with one side being a single variable.

More generally, if the EqualLink has a more complex structure, then "unification" can be done, at the time that the pattern is compiled, instead of at the time that the pattern is run.

@linas linas changed the title Implement transitive equality in pattern matcher. (Theory of) equality at pattern compile time. May 30, 2020
@linas linas changed the title (Theory of) equality at pattern compile time. Equality at pattern compile time (compile-time unification) May 30, 2020
@linas
Copy link
Member Author

linas commented Jun 4, 2020

Another example, from @ngeiswei in #2650 (comment)

(EqualLink
  (Inheritance (Concept "A") (Variable "$Y"))
  (Inheritance (Variable "$X") (Concept "B"))

can be statically analyzed before the search is even started. The URE has a unifier that does this.

@ngeiswei
Copy link
Member

ngeiswei commented Jun 4, 2020

I suppose the unifier could be moved to its own repository, if it is to be used more broadly than by just the URE.

@linas linas changed the title Equality at pattern compile time (compile-time unification) Equality at pattern compile time (static-analysis unification) Jun 5, 2020
@linas
Copy link
Member Author

linas commented Jun 5, 2020

So far, the atomspace has no dependencies other than cogutils.

Although it could be a stand-alone repo, it would need work to be usable for this issue. For example, Unify.h has BindLinktr which makes no sense in the current context. There are substitute methods in there ... which must be only the 4th or 5th or 6th sixth substitute-stuff code that we have laying around. Sadly, the kind of substitution that would be needed for this issue is likely to be different, again, from all the others....

Right now, its just two files, about 2KLocs of code .. do you expect that it might grow?

@ngeiswei
Copy link
Member

ngeiswei commented Jun 5, 2020

Right now, its just two files, about 2KLocs of code .. do you expect that it might grow?

Not much, it's pretty mature at that point. There are limitations with with types (for instance deep types are not supported), but this can or should be done outside of the unifier.

However, I'm frankly uncomfortable having that code go to the atomspace repo, due to the opencog/atomspace vs singnet/atomspace divergence. Keeping my main work away from the atomspace repo allows me to minimize the pain resulting from the opencog vs singnet situation. Ideally I would only develop on one side but for reasons outside of my control that's not possible.

@ngeiswei
Copy link
Member

ngeiswei commented Jun 5, 2020

I do agree however that from a design standpoint, having the unifer inside the atomspace repo is sensible, and it should be examined, independently of my personal comfort.

@linas
Copy link
Member Author

linas commented Jun 6, 2020

the opencog/atomspace vs singnet/atomspace divergence

I very strongly and very sharply encourage this divergence to be fixed. If you have any pull or sway with Ben or the other developers, please, please, please encourage them to set things right again! I've tried to convince @vsbogd to do this, but he is unwilling, because it's a fair amount of work. Someone of authority would need to apply pressure to make this happen. If you can bend the ear of those people .. that would be great.

@noskill
Copy link
Contributor

noskill commented Jun 7, 2020

@linas have you changed your mind on merging ptrvalue and grounded object node from singnet? We use them for integration with pytorch as it was proposed here #1970. So far it's the only difference, but i expect divergence between branches will grow.

@linas
Copy link
Member Author

linas commented Jun 7, 2020

@noskill I have not changed my mind. Please note that agi-bio defines several different atom types. It is NOT a fork of the atomspace; instead, it is layered on top, as a module. Please note that spacetime defines maybe 6 or ten new atom types. They are very similar to GroundedObjectNodes, in that they have an octree inside of them (You could call them "GroundedOctreeNodes", although they have a different name) They allow you to obtain the 3D space and time positions of things. It is a module that is layered on top of the atomspace. The attention-bank git repo contains a third atomspace module, defining two new Values and 6 or 8 new Atom types. The nlp module defines several dozen atom types. Two of them could be called GroundedLinkGrammarNode and GroundedDictionaryNode (although they have different names). For example (EvaluationLink (GroundedDicationaryNode "ru") (WordNode "давно")) and it will return the link-grammar Russian dictionary entry for "давно" -- a long time ago. Maybe five years ago. There are another 4 or 6 or 8 atomspace modules defined in other places.

I see two paths forward: merge the singnet version into the atomspace, and then immediately rip out the grounded object node and place it into it's own repository, so that it would be a module just like agi-bio and all the others. Alternately, you could split out grounded object into it's own module, first, and then we can merge what is left over.

@noskill, you could have created a module from the very beginning, and none of this would have happened. But this is not your fault -- you were told to intentionally fork the atomspace, to create a competing project, for political reasons, not for technical reasons. You were told to NOT create a module, even though technically, it would have been the right thing to do. You were told to make the atomspace "better", because, politically, it needed to "look better". Otherwise, there would have been no reason for the fork. The fork must be "better".

I think the political pressures have subsided. I think it is now safe to split grounded object node into it's own module. I think you need to just do this. I'm worried that if you ask, even politely, they might still say "no" and to "not waste your time on it", and that "it is a low priority". Technically it is the right thing to do. Please do it.

Tag @vsbogd as this is part of your stuff.

@linas
Copy link
Member Author

linas commented Jun 7, 2020

Wow... it suddenly occurs to me that if we ripped out GroudnedPredicateNode and GroundedSchemaNode, and placed them into their own module, all the stupid circular-dependency issues would go away! Hmmm...

@noskill
Copy link
Contributor

noskill commented Jun 8, 2020

@linas I agree. PtrValue was at first living in it's own repo, i guess we can move it again somewhere, maybe to opencog/opencog

@linas
Copy link
Member Author

linas commented Jun 8, 2020

@noskill well, one of the goals is to split up opencog/opencog into pieces -- an nlp piece, a ghost piece, a few other things, so that it ends up empty or mostly empty. (See opencog/opencog#3391)

You should create a brand new repo (or I can create it and give you admin authority) to hold all of the needed parts. Pick a name -- vqa? tensorflow? something else? (a long name - visual-question-answering? something funny - vizquest?)

@linas linas pinned this issue Jun 12, 2020
@linas
Copy link
Member Author

linas commented Jun 12, 2020

URE generates PatternLinks with this content:

(Not (Identical (Concept "criminal") (Concept "criminal")))

which evaluates to always-false, and thus can be deduced to always fail, at static-analysis time. See opencog/ure#92

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

3 participants