Equality at pattern compile time (static-analysis unification)

[linas]

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]

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]

Another example, from @ngeiswei in https://github.com/opencog/atomspace/issues/2650#issuecomment-637982797

(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]

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]

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]

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]

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]

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]

@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 https://github.com/opencog/atomspace/issues/1970. So far it's the only difference, but i expect divergence between branches will grow.

[linas]

@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]

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]

@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]

@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]

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