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

Better interoperability with deep learning frameworks #1970

Open
noskill opened this issue Dec 25, 2018 · 67 comments
Open

Better interoperability with deep learning frameworks #1970

noskill opened this issue Dec 25, 2018 · 67 comments

Comments

@noskill
Copy link
Contributor

noskill commented Dec 25, 2018

This issue is to document and discuss changes necessary to use deep learning frameworks with opencog.

Our usecases:

  1. We would like to be able to pass between ExecutionOutputLinks python objects, particularly pytorch tensors. Pytorch saves in it's tensor instances necessary information for performing backward propagation.

  2. More convenient api than GroundedSchemaNode for calling objects methods.

Example:

Our motivating example is implementing transparent by design networks(https://arxiv.org/pdf/1803.05268)
with opencog.
The idea of this example is answering some question about a picture by
applying series of filters, implemented as pytorch neural networks.
Each nn accepts original picture + mask from previous filter(initially mask is all zeroes) generating new mask.

First i describe current implementation:
ExecutionOutputLinks for the question "What is the large purple object is made of?":

https://github.com/singnet/semantic-vision/blob/9ca40eedd78eb6aec7af469defd436eace2c4be5/experiments/opencog/pattern_matcher_vqa/tbd_cog/tbd_helpers.py#L140-L166

    (ExecutionOutputLink
      (GroundedSchemaNode "py:filter")
      (ListLink
        (ConceptNode "material")
        (VariableNode "$X")
        (ExecutionOutputLink
          (GroundedSchemaNode "py:filter")
          (ListLink
            (ConceptNode "color")
            (ConceptNode "purple")
            (ExecutionOutputLink
              (GroundedSchemaNode "py:filter")
              (ListLink
                (ConceptNode "size")
                (ConceptNode "large")
                (ExecutionOutputLink
                  (GroundedSchemaNode "py:init_scene")
                  (ListLink
                    (VariableNode "$Scene")
                  )
                )
              )
            )
          )
        )
      )
    )

Here pattern matcher grounds VariableNode "$X" to different ConceptNodes representing materials since the constraint:

(InheritanceLink
      (VariableNode "$X")
      (ConceptNode "material")
)

where filter is wrapper to call some pytorch module object:
https://github.com/singnet/semantic-vision/blob/9ca40eedd78eb6aec7af469defd436eace2c4be5/experiments/opencog/pattern_matcher_vqa/tbd_cog/tbd_helpers.py#L389-L404

def filter(filter_type, filter_type_instance, data_atom):
       module_type = 'filter_' + filter_type.name + '[' + filter_type_instance.name + ']'
       module = tbd.function_modules[module_type]
       atomspace = data_atom.atomspace
       key_attention, key_scene, key_shape_attention, key_shape_scene = generate_keys(atomspace)
       feat_input = extract_tensor(data_atom, key_scene, key_shape_scene)
       feat_attention = extract_tensor(data_atom, key_attention, key_shape_attention)
       out = module(feat_input.float(), feat_attention.float())
       set_attention_map(data_atom, key_attention, key_shape_attention, out)
       return data_atom

and init_scene accept scene atom and generate new atom which holds dummy attention map and features from scene. This atom is then reused to pass values between filters.

There are issues with current implementation
a. It requires to convert back and forth between pytorch tensor objects and FloatValue for each ExecutionOutputLink application.
See https://github.com/singnet/semantic-vision/blob/9ca40eedd78eb6aec7af469defd436eace2c4be5/experiments/opencog/pattern_matcher_vqa/tbd_cog/tbd_helpers.py#L281
b. This implementation doesn't allow to backpropagate error to neural network weights since information is lost due to conversion. Pytorch Tensor object keeps both: current numeric values and link to computation graph which allows to backpropagate error automatically.

PtrValue
Both issues may be addressed with introduction of new value type: PtrValue. Then to implement storing of values for different binding language one would inherit from base PtrValue type. For python there will be c++ class PythonValue(PtrValue) and for haskell HaskelValue(PtrValue) etc..

Then extracting tensor object in "py:filter" will look like:

atom.get_value(PredicateNode("PythonValue"))

and returning value will be done by creating a new atom to hold value:

layer_result = ConceptNode(gen_random_uuid())
layer_result.set_value(PredicateNode("PythonValue"), PythonValue(tensor_mask))

ExecutionValueLink
In addition to PtrValue we may introduce new link type: ExecutionValueLink which would return a PtrValue. This would allow to return from "py:filter" PythonValue(tensor_mask).

That's for addressing of usecase 1.

To address usecase 2 - more convenient api than GroundedSchemaNode for calling objects methods:

One way is to use proposed PtrValue alongside with wrapper function.

def callPythonMethod(atom_obj, atom_method_name, *args):
    obj = atom_obj.get_value(PredicateNode("py:callPythonMethod"))
    return getattr(obj, atom_method_name.name)(*args)

Then calling method will be a bit verbose but quite straightforward:

ExecutionOutputLink(
        GroundedSchemaNode("py:callPythonMethod"),
        ListLink(ConceptNode("FilterRed"),
                 ConceptNode("forward"),
                 ExecutionOutputLink...
                )

GroundedObjectNode

Another way to address the same issue is to use LibraryManager to store python objects.
GroundedObjectNode atom type would register python object in LibraryManager.
Like:

import torch
GroundedObjectNode("dot", torch.dot)

then calling with ExecutionOutputLink or any other executable link:

ExecutionOutputLink
   GroundedSchemaNode("obj: dot.__call__")
   ListLink
      VariableNode("$OtherWeights")
      VariableNode("$SomeWeights")
@linas
Copy link
Member

linas commented Dec 29, 2018

There was a detailed discussion on how to integrate deep learning with the atomspace that happened in the spring, with @Necr0x0Der and @misgeatgit is implementing the prototype for this, with the space-time server. I gave a sketch of this in pull req #1971 -- I'll respond to the above shortly, when I get a chance to read it.

@vsbogd
Copy link
Contributor

vsbogd commented Dec 29, 2018

After experimenting with first prototype we have found that representing NN features as FloatValues is not enough. This improvement is suggested by @Necr0x0Der next step for integration NN and OpenCog.

@linas
Copy link
Member

linas commented Dec 29, 2018

Is this code already written, or is this a design proposal? I don't think you should do with nested execution output links -- that's NOT going to work well at all ... for many different reasons

@linas
Copy link
Member

linas commented Dec 29, 2018

OK, first -- some background lectures: the atomspace is meant to be a declarative data store, and not a procedural programming language. Your example is treating it as a procedural programming language, and that will not work well; it will have terrible performance, it cannot be used with the pattern matcher, the pattern miner, the rule engine, PLN, etc.

An example of a declarative way of declaring what you wrote would be this:

(SequentialAndLink
    (PredicateNode "init scene")
    (EquivalentLink
          (PredicateNode "size")
          (PredicateNode "large"))
    (EquivalentLink
         (PredicateNode "color")
         (PredicateNode "purple"))
   (EquivalentLink
         (PredicateNode "material")
         (VariableNode "$result")))

This is one way to declare it. It's not a good way (there are better ways), but it is a way, and since its simple, let me use it as an example for the next 24 or 48 hours.

Anyway, the above is a declaration. It doesn't actually do anything. To make it "actually do something", you could (for example) write this:

(ExecutionOutputLink
    (GroundedSchemaNode "py:process_scene")
    (SequentialAndLink .... (the stuff above))

The py:process_scene code would compile the contents of the sequential-and, to create an executable network of filters. Then, every time the ExecuteOutputLink is called, you would execute that network once.

There are several problems with my proposal, above. Next post will talk about how to solve them. Issues include:

  1. There is nowhere for the compiled code to be cached. (you want to compile it only once, and cache the results; you want to execute it many times

  2. It probably does not make sense to return the result as an Atom (it should be a value)

  3. the SequentialAndLink format is ugly-ish.

@linas
Copy link
Member

linas commented Dec 29, 2018

Let's solve issues 1) and 2) part of 3) The solution that seems natural, at the moment, is to invent a new link type, call it VisualLink. In the atomspace, it would be used like so (figure three from the Arxiv article)

(VisualLink
    (QueryNode "color")
    (SequentialAnd
         (AttendNode "large")
         (IntersectLink
               (SequentialAnd
                      (RelateNode "left")
                      (AttendNode "sphere")
                      (AttendNode "metal")
                      (AttendNode "large"))
              (SequentialAnd
                     (RelateNode "right")
                     (AttendNode "metal")
                     (AttendNode "green")))))

The above is just the digram from figure 3, written upside-down. (because Atomese is upside-down ...)

The VisualLink is a brand new C++ class. It might look something like this pseudocode: (next post)

@linas
Copy link
Member

linas commented Dec 29, 2018

The VisualLink is a brand new C++ class. Actually, you need TWO C++ classes. The other one is VisualValue. But first, VisualLink might look something like this pseudocode:

class VisualLink: public Link 
{
    private:
         PyExec * compiled_query;
    public:
        VisualLink() { // ctor
               // Convert the outgoing set of AttendNodes and RelateNodes to
               // a PyTorch program, and save that program.
               compiled_query = PyMangle("blargle", getOutgoingSet());
               ValuePtr torchy = createVisualValue(compiled_query);
                this->set_value(PredicateNode("visual answer"), torchy);
        }
}

That's it. Not much there, except a big blob of code to convert the list of AttendNodes and RelateNodes into a pytorch program. Next, we have this:

class VisualValue : public FloatValue
   private:
        PyExec * compiled_query;
        virtual void update() const {    // overload update() from FloatValue
               // Execute the previously generated PyTorch program
               std::vector<double> result = PyExecute(compiled_query);
               this->_value = result // store the result right here.
        }
};

That's it. Nothing more to do here. Its almost trivial. How does the user use this? Like so:

(define myprocessor (VisualLink ...etc)) ; stuff from above

;; Now get the value; the Predicate must be the same as in the C++ code!
(define myview (cog-get-value myprocessor (PredicateNode "visual answer"))) 

;; Now run the processing pipeline exactly once
(display "hello world\n")
(display (cog-value->list  myview))
(newline)

; Now run it again...
(display (cog-value->list  myview))

Every time you call the scheme function cog-value->list it will call the C++ FloatValue::value() method which calls the C++ VisualValue::update() method (because update is virtual) and VisualValue::update() calls the previously compiled pytorch code to get an answer.

That's it. Although this is pseudocode, there is not a lot here. VisualValue is really pretty simple, almsot exactly like the pseudocode; the hard parts are the compiler in VisualLink

@linas
Copy link
Member

linas commented Dec 29, 2018

Why implement it like this?

A) When late-night talk-show host asks Sophia, "hey is there a large green metal sphere on your left?", the ghost subsystem can convert this English-language sentence into the VisualLink given above, then run it, then get the answer.

B) Some suitable form of PLN can make inferences about the contents of the VisualLink, viz, "if round things are balls and shiny things are metal, therefore (VisualLink round metal)"

C) The pattern miner can look for frequent patterns "there were 23 VisualLinks for round things and 48 Visual Links for shiny things and 5555 Visual links for small shiny colored squares" -> Sophia tells the talk show host "Gee, I think I really like glitter!"

D) Almost no compute cycles happen in the atomspace, or on the CPU. After compiling the visual network; it gets loaded onto the GPU's where it sits and does its thing in real time. The ONLY time you waste CPU cycles is when the user does the cog-value->list which causes pytorch to ask the GPU for an answer. Otherwise, the system is idle.

The declarative, stateless form of the VisualLink allows all kinds of subsystems to gain access to the scene description. If you hide the state inside of GroundedObjectNodes, it becomes impossible to find it, reason over it, edit it, construct new visual queries on the fly, etc.

@noskill
Copy link
Contributor Author

noskill commented Jan 5, 2019

Is this code already written, or is this a design proposal? I don't think you should do with nested execution output links -- that's NOT going to work well at all ... for many different reasons

Code with nested execution output links is already written, it is written that way mainly due to reason that we don't have URE rule for working with tensor(pixelwise) truth values. Once we have more seamless integration we will implement declarative solution.

@noskill
Copy link
Contributor Author

noskill commented Jan 5, 2019

My thought on VisualLink vs ValuePtr:
ValuePtr allows to write more declarative code since it allows us to interleave inference in URE with calls to neural networks, URE determines actual sequence of calls to NN. On the other hand VisualLink requires one to write some imperative code to execute it's content:

The py:process_scene code would compile the contents of the sequential-and, to create an executable network of filters.

@linas
Copy link
Member

linas commented Jan 6, 2019

Code with nested execution output links is already written,

What's the github repo?

VisualLink requires one to write some imperative code to execute it's content

Yes, absolutely. That's the whole point! That's the intent! I feel that maybe there is still some misunderstanding. Code written in python, C++, etc. is necessarily imperative, because that is what these languages are. Code written "inside of atoms", implementing them, is necessarily imperative: it "does something".

URE

Nothing above makes use of the URE in any way. You haven't yet written a single rule, much less something that would require a URE.

@ngeiswei
Copy link
Member

ngeiswei commented Jan 7, 2019

Personally, I don't see anything wrong with nesting ExecutionLinks or any kind of links. A sequence is underneath a nesting of links too (see https://wiki.opencog.org/w/ConsLink).

@linas
Copy link
Member

linas commented Jan 7, 2019

Argh @ngeiswei this hurts. First, ConsLink is a terrible idea, for the same reason that SetLink is a terrible idea. Now that we know, from five-plus years of hard-core here's-dirt-in-your-face experience about the evilness of SetLink, and the evilness of nesting in general ... this is not some rugby game where you stand up and say "please smash my face into the dirt again, I like how that feels". Nesting doesn't work because it's not editable, its not pattern matchable, its not rewritable. Cough cough -- "quote link" -- "Doctor doctor, it hurts when I do this" -- "well, don't do that!"

ExecutionOutputLinks are just ugly hacks meant to provide emergency escape routes for incompletely-designed systems. I don't want to encourage bad design when there is a simple, easy, obvious alternative. We need to take the accumulated experience, and move forward.

@ngeiswei
Copy link
Member

ngeiswei commented Jan 8, 2019

OK, but sometimes you want things to be immutable.

The difference between

Member
  E1
  A
...
Member
  En
  A

Inheritance
  A
  B

and

Inheritance
  Set
    E1 ... En
  B

is that the latter is set in stone, the former isn't.

Which one is better depend on the usage, sure for iteratively returning pattern matcher results a MemberLink is better than a SetLink, in other situations, not.

@noskill
Copy link
Contributor Author

noskill commented Jan 10, 2019

@linas Rules will be something like that(for conjunction over tensors):

variables = VariableNode("X1"), VariableNode("X2")

pattern = AndLink(variables)
rewrite = ExecutionOutputLink(GroundedSchemaNode("py: fuzzy_tensor_conjunction_introduction_formula"),
                              ListLink(AndLink(variables), SetLink(variables)))

BindLink(pattern, rewrite)

Then it will be possible to rewrite nested execution output links to more declarative form:

     (AndLink
        (ExecutionOutputLink
          (GroundedSchemaNode "py:filter")
          (ListLink
            (ConceptNode "color")
            (ConceptNode "purple")
            (VariableNode "$Scene")))
         (ExecutionOutputLink
              (GroundedSchemaNode "py:filter")
              (ListLink
                (ConceptNode "size")
                (ConceptNode "large")
                (VariableNode "$Scene")))
          )

@noskill
Copy link
Contributor Author

noskill commented Jan 10, 2019

By the way how introduction of VisualValue is better than PtrValue? It has pointer to python object just like PtrValue..

@linas
Copy link
Member

linas commented Jan 10, 2019

@ngeiswil yes, SetLink does have some valid use cases. It got over-used in the current API, and that is my fault.

@linas
Copy link
Member

linas commented Jan 10, 2019

@noskill, as stated elsewhere: GroundedSchema and ExecutionOutput are ugly hacks meant to solve simple problems, and you are pushing them far beyond what they can carry,

ExecutionOutput/GroundedScheme are like Atomese "GOTO" statements: they have valid uses, they solve certain simple problems, one should not write complicated code with them. The PtrValue is like a void * pointer: it points at anything. You keep trying to tell me that you want to write complex programs using GOTO's and void* pointers, and I keep saying "don't do that". You keep saying that you can make it look structured, or declarative, or maybe even object-oriented, if you really try hard, and yes, I'm sure that's true.

I'm saying that structured programming, object-oriented programming was invented for a reason. There's a history for why people do it that way. This history is older than me, and I'm fine with it. It doesn't need to be rediscovered/reinvented.

Create an AttendNode C++ object. Using ordinary OO programing style. If it needs to hold state, put that state into the C++ object. Just make sure that it's reconstructable state: nothing that would ever need to be transmitted on the network, nothing that would ever need to be saved to disk. Create 23 different OO method calls on class AttentionNode. Many of these method calls might call py:filter, under the covers where they user cannot see the call to python. If you need to pass extra hidden values, put them into the class AttentionNode. If you really really need to, use void* pointers in the C++ code, but I'm pretty sure that PyObject* would be better.

Create a RelateNode. If the RelateNode needs to call methods on the class AttendNode to get secret parameters and hidden values, .. well, then, do that. If VisualLink needs to talk to both RelateNode and AttendNode so that it can cache partial results that py:filter returned, -- great! Do that! Whatever it is that py:filter returns, whatever additional stuff that it is that it needs as input, cache it inside of VisualLnk and AttendNode and RelateNode as C/C++ pointers, maybe PyObject* pointers, whatever, I don't care. Just use good, clean, normal OO programming style. Don't get hacky.

@linas
Copy link
Member

linas commented Jan 10, 2019

I am afraid that saying more will confuse the issue, but I can't stop. I think that comment #1970 (comment) and #1970 (comment) really really describe the correct implementation for this.

Above, I said "object oriented" several times. I mean it, and that is very important. I want good, clean, college-textbook-standard OO style. The thing I did not say was "structured query" and "relational data", which is also very very important. I want you to follow the rules of good clean text-book-style relational design. Atomese should look exactly like what they teach in school as good relational style.

Here is a very simple nice example: There are five short tutorials here: https://grakn.ai/grakn-core Please please please read them and study them.

  • Panel 1 is labeled ER "Entity-Relationship" and these are like our EvaluationLink/PredicateNode idea.

  • Panel 2 is "Types", which is like our type system (the "deep types" part)

  • Panel 3 is called "Rules" and its almost identical to our BindLink

  • Panel 4 is called "Inference" and it is almost identical to running our GetLink

  • Panel 5 is called "Analytics" and we don't do that.

The grakn.ai example is a very nice example of good, clean, solid well-designed relational programming style. Its just like SQL, but 50 years newer and better. That is the same kind of style that we want to have for Atomese.

Notice that grakn.ai does not have GroundedSchema and ExecutionOutputLinks. That is not an accident, a short-coming, or a missing feature. That is because they have a good, clean high-quality design that eschews nasty ugly callback hacks. (Notice that SQL doesn't have these, either. Notice that datalog doesn't have these, either.)

Atomese should look like, be written like, have the same general style as grakn.ai, or SQL, or datalog. It should follow all the rules in a typical textbook on relational data, including concepts like "normalization".

Atomese should NOT look like lambda calculus or CamL or Haskel or Lisp or scheme.

Atomese should NOT look like a random mashup of GOTO's and void* pointers.

@ngeiswei
Copy link
Member

Panel 4 is called "Inference" and it is almost identical to running our GetLink

I think panel 4 demonstrates true inference, thus would be closer to what the rule-engine does.

Atomese should look like, be written like, have the same general style as grakn.ai, or SQL, or datalog.

Maybe, or maybe a layer on top of Atomese could look like grakn.ai and such.

It's kinda hard for me to have a feel about all this because I lack the experience of building large and complex knowledge bases with Atomese, only worked on toy problems so far.

@noskill
Copy link
Contributor Author

noskill commented Jan 11, 2019

@linas Ok, i kind of understand your position. Our goals require more functionality than provided by grakn.ai or datalog. I hope it's possible to make atomspace extendable enough so that we can implement PtrValue and GroundedObjectNode in separate repository.

@ngeiswei
Copy link
Member

ngeiswei commented Jan 11, 2019

@noskill, intead of

def callPythonMethod(atom_obj, atom_method_name, *args):
    obj = atom_obj.get_value(PredicateNode("py:callPythonMethod"))
    return getattr(obj, atom_method_name.name)(*args)

I think you mean

def callPythonMethod(atom_obj, atom_method_name, *args):
    obj = atom_obj.get_value(PredicateNode("PythonValue"))
    return getattr(obj, atom_method_name.name)(*args)

that is "py:callPythonMethod" has been replaced by "PythonValue", right?

@bgoertzel
Copy link
Contributor

bgoertzel commented Jan 11, 2019 via email

@noskill
Copy link
Contributor Author

noskill commented Jan 11, 2019

@ngeiswei It should be some meaningful string, maybe "PythonValue"

@linas
Copy link
Member

linas commented Jan 11, 2019

@bgoertzel I wrote the desired psuedocode in these two comments: #1970 (comment) and #1970 (comment) -- also please note that the pseudocode in those two comments is almost a cut-n-paste of the octomap API -- both of which are the formal result from the long email chain with Alexey Potapov from last spring, where these details got worked out.

@linas
Copy link
Member

linas commented Jan 11, 2019

@noskill You wrote: "Our goals require more functionality than provided by grakn.ai or datalog."

I am trying to use these two as examples of high-quality relational design. The atomspace is very nearly a textbook-standard relational database. If you don't understand what relations are, or how to write them, you will have a very difficult time abstracting your problem and arriving at a good solution.

@noskill, you wrote "I hope it's possible to make atomspace extendable enough so that we can implement PtrValue and GroundedObjectNode in separate repository."

I gave some very explicit and precise pseudocode that explains exactly how to write a good implementation that follows textbook relational-database style, and also textbook object-oriented style programming, in comments #1970 (comment) and #1970 (comment)

I think that pseudocode does exactly everything that you need to do, and provides for all features and functions for encapsulating your project. Its an API that has already been proven to work; its used both in the octomap implementation, and in example programs that were explicitly created for your neural-net project.

I don't understand why there is a lot of resistance and friction here. I'm pretty sure that you were told from the very beginning of the project, by many people, that you should NOT use GroundedSchemaNodes and GroundedPredicateNodes -- I don't understand why you decided to go this way, anyway, despite being told not to ... and why you are rejecting what seems to be a very simple, clear and obvious design.

@bgoertzel
Copy link
Contributor

Linas, I don't understand your metaphor

"GroundedSchema and ExecutionOutput are ugly hacks meant to solve simple problems, and you are pushing them far beyond what they can carry,

ExecutionOutput/GroundedScheme are like Atomese "GOTO" statements: "

There may be ugly hacks in the current implementation of these Atoms (I haven't looked lately) but as the person who first introduced these concepts into Atomese, I can tell you they were not intended to be anything like Atomese GOTO statements. They were really intended to be more functional-programming-ish than GOTO-ish. ExOutLink lets a GroundedSchemaNode applied to some argument produce some entity which then gets passed along to a different GroundedSchemaNode. This is not like a GOTO statement that passes a pointer to a certain position, it's just passing the output of one function as input to another function.... I don't consider this an ugly hack but rather a generally useful functionality...

Regarding "The atomspace is very nearly a textbook-standard relational database" ==> honestly I think this statement is only true at a high level of abstraction that is not really useful for this discussion...

Regarding "Atomese should look like, be written like, have the same general style as grakn.ai, or SQL, or datalog. It should follow all the rules in a typical textbook on relational data, including concepts like "normalization".

Atomese should NOT look like lambda calculus or CamL or Haskel or Lisp or scheme." ==> I understand what you're saying here but I just don't agree with you...

What you describe is NOT the vision that I had for the Atomspace when I came up with the idea, and not the vision that I had when thinking about how different cognitive algorithms would interact with Atomspace, etc. It is also not how I have been thinking about Atomspace when discussing it with Nil, Alexey and others...

This view of Atomspace doesn't accommodate for how PLN currently works in the Atomspace, with explicit quantifiers and quantifier bindings, etc. And it also doesn't accommodate for how I think makes most sense to integrate deep NNs with Atomspace (which does involve a more functional style, including ExOutLinks passing their outputs to other GSNs...)

So as far as I can understand, where we are now is: You (Linas) have one view of how Atomspace/Atomese should be used, both in general and in this particular case; and we (Alexey and his team and myself) have a different view. It's obvious you think your approach is much better, but I prefer to just look at it as two different approaches.

The question then is how do we manage this on a codebase level so that everything is maximally copacetic. While I am immensely thankful to and respectful of you for writing and designing the bulk of the current code in the Atomspace repo, nevertheless I don't agree with your current perspective about how this deep-NN/OpenCog related code should be implemented (nor with your general current philosophy about eschewing lambda-calculus-ish and functional-programming-ish idioms in Atomese in favor of SQL-ish/datalog-ish idioms...).... I don't want to screw with your development work or style, but I also don't want to have to accommodate everyone's work to your particular taste... (which I note changes over time like all of our tastes do...)

It would really be better NOT to have to fork Atomspace into Linas-space versus Singularity-space ...

Clearly MOST of the stuff that Vitaly/Anatoly/Alexey want to do here can be modularized from the core Atomspace code so you don't have to deal with in your own work if you don't want to. However it seems there are a few things that they need to do that would need to go into the core Atomspace code. My own view would be: These are not huge code changes, and they are features that you don't need to use in your own work if you don't want to. So I don't see why they should be blocked.

I do understand your VisualLink suggestion. I just think that design is worse, it's awkward by comparison and will lead to a big proliferation of link types instead of a simpler abstract mechanism. I understand your taste is different than that of myself/Anatoly/Vitaly/Alexey in this matter but these are highly competent AI developers and theorists and should not have to do every little thing according to your particular taste IMO...

Apologies if the above remarks are a bit rambling and not 100% coherent, I don't have a lot of time for this but have read the various comments on this issue moderately carefully and am very eager to see this move forward. There is so much cool stuff happening in the "deep NN for vision and language world" these days and so much value to be created by interfacing this stuff appropriately with OpenCog, it's a shame to be held back from exploiting this potential via codebase-management issues. I understand these are not trivial issues, they are about how cognitive architecture intersects w/ programming paradigm etc. etc., but nevertheless given the task we have collectively taken on we gotta be able to sort through this stuff more efficiently than is currently occurring...

@linas
Copy link
Member

linas commented Jan 12, 2019 via email

@vsbogd
Copy link
Contributor

vsbogd commented Jan 17, 2019

@linas, do you mean SequentialLink can be used to implement evaluation loop? Is there something similar for executable links?

@noskill
Copy link
Contributor Author

noskill commented Jan 17, 2019

So SequentialLink is the conditional operator in atomese.. I believe it would be more convenient to have more usual conditional operator, like cond in scheme.

@ngeiswei
Copy link
Member

ngeiswei commented Jan 17, 2019

Actually, SequentualLink isn't supposed to be conditional, however SeqentialAndLink or SequentialOrLink are. They would also supposedly have a temporal semantics and allow PLN to reason about temporal events, but that isn't implemented.

CondLink or IfThenElseLink would be definitely welcome IMO.

@bgoertzel
Copy link
Contributor

bgoertzel commented Jan 17, 2019 via email

@linas
Copy link
Member

linas commented Jan 18, 2019

I want to very strongly resist IfThenElseLink and I want to very strongly resist ForLoopLink. There are multiple reasons for this.

First, we already have IfThenElseLink, it just has a different name: it is called SequentialOrLink. The SequentialOrLink evaluates its first atom, and if it returns true, then done. Else it evaluates the second atom. So we already have that - we've had it for 3-4 years now, we don't need to reinvent it.

@linas
Copy link
Member

linas commented Jan 18, 2019

We also do not need a ForLoopLink, because we already have one of those, too, just under a different name. The example examples/atomspace/recursive-loop.scm explains how to use it.

The example examples/atomspace/random-choice.scm demonstrates how to run a loop exactly 1000 times. It computes the average of 1000 random numbers.

Both of these examples also exist as unit tests; as long as the unit tests pass, then the examples will work.

Here:
https://github.com/opencog/atomspace/blob/master/examples/atomspace/recursive-loop.scm
https://github.com/opencog/atomspace/blob/master/examples/atomspace/random-choice.scm

@noskill
Copy link
Contributor Author

noskill commented Jan 18, 2019

@linas i believe this is not correct translation of my code:

GroundedSchemaNode "py: some_func"
    # ... calls python code 
    def some_func():
         a = 42
         print ("Its" + str(a) + "\n")   # print a
    # return from python; variable a goes out of block scope. 
GroundedSchemaNode "py: other_func"
    # ... calls python code 
    def other_func():
         print ("Now Its" + str(a) + "\n")   # invalid! there is no "a" in this scope!

It is more like:

GroundedSchemaNode "py: some_func"
    # ... calls python code 
    def some_func():
         a = 42
         print ("Its" + str(a) + "\n")   # print a
         return  a
    # return from python; variable a stored somewhere 
GroundedSchemaNode "py: other_func"
    # ... calls python code 
    def other_func(a):
         print ("Now Its" + str(a) + "\n")   # valid! there is "a" in this scope!

By the way is there a Link that is intended to return some Value(for example PtrValue), not just truth value?

@linas
Copy link
Member

linas commented Jan 18, 2019

Loops and tail recursion were added to the atomspace in the fall of 2015. They were added to make it easier to control the Sophia robot. They are used in the old behavior-tree code, and in ghost. They are typically used with threads.

The recursive loop example was explicitly added for Dr. Alexey Potapov, to make it clear how to write a loop with neural nets. Unlike the old behavior-tree/ghost loops, that particular example uses values for the loop exit criteria.

The suggestion was that the the RandomStream in that example was going to be replaced by some neural net output. We did not yet know what that neural net would look like, so RandomStream was used for the example. Way back then (May 2018) it was imagined that there might be some TensorFlowStream or something like that; it would generate a stream of values, and those values could be monitored and used to control flow inside of the atomspace.

I mentioned threads. We have two different links that create threads and join threads, they are called ParallelLink and JoinLink https://wiki.opencog.org/w/JoinLink -- they can be used with loops, and it should all work fine. It seems that we do not have any examples using these two. I'll try to create some now.

During the May 2018 discussions, it was assumed that the neural net code would be using the threading links, so that the main atomspace would not be blocked while TensorFlowStream accessed the GPU's to obtain values.

@linas
Copy link
Member

linas commented Jan 18, 2019

@noskill -- again -- the code in this example: https://github.com/opencog/atomspace/blob/master/examples/atomspace/recursive-loop.scm is a copy of an email from the mailing list with Alexey, where a prototype code for the neural net code was created. In that prototype, and in that example, it was assumed that Streams would be used to wrap the neural nets.

There was no GroundedSchemaNode in the original prototype. A value, inheriting from StreamValue would store everything that the neural net needed. The ValueOfLink was the mechanism to return arbitrary values. Since the PlusLink assumes numbers, that demo works only if the returned values are floats. However, other kinds of values can be returned, and because they are values, you can store them anywhere at all.

@noskill
Copy link
Contributor Author

noskill commented Jan 18, 2019

Recursion is common for all functional languages, it is something expected to be in Atomese instead of for loop. But IfThenElseLink would be nice to have:
Examples look somewhat awkward since all the functions used for side effects or some computation are forced to be predicates:

print function returns truth value:
(define (print-stuff) (display "hi there!\n") (stv 1 1))

If True then assign variable:

(True (Put
             (State (Anchor "sum-A") (Variable "$x"))

@linas
Copy link
Member

linas commented Jan 18, 2019

The stream.scm example might be a simpler place to start. It started life as a prototype neural-net example, showing how to move neural-net outputs into the atomspace. It used RandomStream as an example of a "typical" neural net. Again, this is just a cut-n-paste from the mailing list, Alexey should have walked you through these examples.

It is simpler to understand than the looping examples, and might be a better place to start.

@linas
Copy link
Member

linas commented Jan 18, 2019

We already have an IfThenElseLink; it is called SequentialOrLink See above.

@linas
Copy link
Member

linas commented Jan 18, 2019

Re: IfThenElseLink, and "examples look somewhat awkward". Keep in mind:

  • Yes, the examples will feel like reading and writing assembly code; it will seem awkward and verbose to humans. There is a reason for that.
  • The reason for the awkwardness is to make it as easy as possible to perform algorithmic reasoning on Atomese. The goal is to make it easy for PLN, for the pattern miner, for the pattern matcher, for openpsi to take a SequentialOrLink as input and perform manipulations on it -- taking it apart, reassembling it in different ways.
  • Having minimalist, highly uniform atoms makes this easier., for the algorithms. Kind of like RISC vs. CISC -- RISC is easier for the machine, for the algo; CISC is easier for the human. Humans should not be writing assembly code. Humans should not be writing Atomese.
  • Or rather, write just enough Atomese by hand, to see how it is might work, and then give the whole thing over PLN+pattern-miner+openpsi+moses, and let these high-level systems create the Atomese for you. You don't program in assembly; you should not be programming in Atomese, either.

To recap: Atomese is not meant for humans. It's not a functional programming language. It is a knowledge representation language.

However, it is also very clear that we need to somehow make it easier for humans. We never explained values very well, it seems that they are very hard to understand. Looking at the https://grakn.ai examples also makes it clear that we have ignored the ease-of-use of EvaluationLink/PredicateNode for far too long. Humans are not using the EvaluationLink/PredicateNode combination because it is just too verbose, takes up too much space, requires too much typing.

Just right now, it feels like it would be a good idea if we had some higher-level system that was as easy to use as grakn.ai, and hid the messy EvaluationLink/PredicateNode combo from "ordinary users". it would also be a place to add if-then constructs, c++/python-style for-loops, and stuff that procedural programmers are used to. Somehow hide values there too, so they don't trip you up as much. Right now, I cannot think of any easy solution for this higher-level, easier-for-humans layer. It's gonna take a lot of work.

@ngeiswei
Copy link
Member

The problem with SequentialOrLink is that it returns a TV while IfThenElseLink would return any atom type executed in its branches. That could be worked around I suppose, but the most elegant solution I think is to introduce CondLink or IfThenElseLink.

@linas
Copy link
Member

linas commented Jan 21, 2019

SequentialOrLink is that it returns a TV while IfThenElseLink would return any atom

Ah, OK, yes. Down this path is the tangle that pull req #1996 just scratches the surface of: when are we working with "just TV's", when do we work with "just Atoms", and when with "general Values" (keeping in mind that Atoms are a special case of values).

This helps make it increasingly clear that, for performance reasons (not just usability) we need to have a crisp boolean yes/no inside of the pattern matcher and other places, instead of using TV's for this. That is, IfThenElse is very clearly a crisp-truth-value thing. Its not a fuzzy-logic thing, or a probabilistic-logic thing, or a probabilistic-programming random sampler.

Should we give up on the dream of having AndLink, OrLink, etc. ever being anything other than crisp-truth ? Yes, AndLink is kind-of-like "set intersection", OrLink is like "set union", but none of our code anywhere ever does set intersection to compute the truth value of AndLink.

There's a link, called ValueOfLink, that was added last summer, for the deep-learning API. See
https://github.com/opencog/atomspace/blob/master/examples/atomspace/stream.scm

@linas
Copy link
Member

linas commented Jan 21, 2019

SequentialOrLink is that it returns a TV while IfThenElseLink would return any atom

This also exposes the unfulfilled dream of Atomese. The goal of Atomese is NOT to just invent another programming language, badly. The goal is to have something that a reasoning engine can examine, and reason with.

Unless PLN has an axiom that DEFINES what IfThenElse "actually means", unless PLN has a rule that takes IfThenElse as input, and pits out something else as output, then it's kind-of pointless to have it. Instead of programming in Atomese, write your programs in C++ or python or whatever.

For PLN, you can substitute MOSES and Reduct. One of the Reduct rules should take IfThenElseLink as input, and do something with it.

For PLN, you can substitute the PatternMiner: If mining reveals that IfThenElse has a large "surprisingness", then do .. something with it.

IfThenElse is a core component of state machines and behavior trees. Yet, oddly enough, none of the four state-machine demos need it:
https://github.com/opencog/atomspace/blob/master/examples/pattern-matcher/fsm-basic.scm
https://github.com/opencog/atomspace/blob/master/examples/pattern-matcher/fsm-full.scm
https://github.com/opencog/atomspace/blob/master/examples/pattern-matcher/fsm-mealy.scm
https://github.com/opencog/atomspace/blob/master/examples/pattern-matcher/markov-chain.scm

So, do we really need IfThenElse?

For PLN, substitute the natural-language comprehension and the natural-language generation subsystems. Look at the Sophia-robot late-nite TV show host interview problem again: the TV show host says something to Sophia, and she converts that to Atomese. What computational process allows her to generate a reasonable reply? How does that process need IfThenElse link?

@linas
Copy link
Member

linas commented Jan 22, 2019

Meanwhile: this comment: #2004 (comment) explains a way of doing what you wanted to do with ValuePtr in a way that would work well with the existing system.

@ngeiswei
Copy link
Member

Unless PLN has an axiom that DEFINES what IfThenElse "actually means", unless PLN has a rule that takes IfThenElse as input, and pits out something else as output, then it's kind-of pointless to have it.

Absolutely. My plan is to ultimately add tons of axioms about math, and atomese in particular in

https://github.com/opencog/opencog/tree/master/opencog/pln/facts

so that PLN can reason about Atomese programs, in a much more open-ended way than MOSES or the pattern miner.

@noskill
Copy link
Contributor Author

noskill commented Jan 25, 2019

@linas You just renamed GroundedObjectNode to PythonEvaluationLink. Introduction of GroundedObjectNode doesn't solve the problem of passing of arbitrary python objects between ExecutionOutputLinks. I mean i could wrap every returned object in new PythonEvaluationLink, but what's the point? We already have PtrValue for that.

@noskill
Copy link
Contributor Author

noskill commented Mar 6, 2019

Example of why ValuePtr is usefull
This is query to rule engine with rewritten fuzzy conjunction formula:

(AndLink
  (EvaluationLink
    (GroundedPredicateNode "py:CogModule.callMethod")
    (ListLink
      (ConceptNode "green") 
      (ConceptNode "call_forward_tv") 
      (ListLink
        (ConceptNode "apple") 
      ) 
    ) 
  ) 
  (InheritanceLink (stv 0.800000 0.990000)
    (ConceptNode "green") 
    (ConceptNode "color") s
  ) 
)

Here (ConceptNode "apple") has some data(pytorch tensor) attached using ValuePtr.
(ConceptNode "green") has some pytorch model attached, which is run on data from (ConceptNode "apple") . The InheritanceLink also has ValuePtr mirroring (stv 0.800000 0.990000).
Function "call_forward_tv" runs the model and attaches pytorch array to the EvaluationLink it called from.
Then fuzzy conjunction rule takes these two arrays - one from InheritanceLink and one from EvaluationLink and and computes new tensor value which is attached to the AndLink.

Now if we have training set with correct answer we can use backprogation to update both: weights of the model which classifies green objects and strength of the InheritanceLink that green is a color. After weights update is done it is possible to update simple truth value of InheritanceLink to mirror the changes. Thus we can learn truth values of InheritanceLinks using backpropagation.

Examples are here https://github.com/singnet/semantic-vision/tree/master/experiments/opencog/cog_module, they require singnet atomspace and pytorch to run. They are all runnable, but still work in progress.

@linas
Copy link
Member

linas commented Mar 6, 2019

Can you open a new issue that describes what you are actually proposing, and how it works?

The discussion above has gotten very long, and it's impossible to tell which ideas were implemented, which ideas were abandoned, which ideas were rejected.

I'm not sure, but I think that you are proposing that GroundedPredicateNode should support the format:

GroundedPredicateNode "py: instance_of_some_python_class.method_on_that_class"

or maybe

GroundedPredicateNode "pyobj: instance_of_some_python_class % method_on_that_class"

or something like that. That seems reasonable to me. I think that this can be accomplished without having to store any actual C++ pointers inside of GroundedPredicateNode

This is already "trivially" possible in scheme. because scheme OO-programming objects are just closures. (i.e. closures are more-or-less the same thing as objects. They resemble javascript objects a lot more than they resemble python objects) In scheme, the following should work:

ExecutionOutput
     GroundedPredicate "scm:name-of-closure" 
     ListLink
            ConceptNode "name-of-method"
            ConceptNode "... the other args to the method ..."

It should work. If someone wanted to get fancy, the could implement

GroundedPredicate "scm-obj: name-of-closure 'name-of-method" 

@linas
Copy link
Member

linas commented Mar 6, 2019

Regarding values, what was done for OctoMap was to create a value, called OctoMapValue that knows which instance of the OctoMap is being queried. In all other respects, it behaves as a FloatValue i.e. returns x,y,z when queried (which are the x,y,z coordinates of the object in the octomap)

@noskill
Copy link
Contributor Author

noskill commented Apr 19, 2019

@linas I made TensorTruthValue which wraps python object in truth value, so it retains torch computation graph and behaves like truth value from c++ side - singnet#92.

The same can easily be done for FloatValue<->torch.Tensor. Do you think this design is good enough?

@linas
Copy link
Member

linas commented Apr 22, 2019

It seems reasonable. I suggest the following minor changes:

  • Call it PythonTruthValue or maybe DynamicTruthValue since it has nothing at all to do with tensors, and just seems to be a generic way of getting a value from some generic python code.

  • The TruthValue::clone() is probably not needed. (?) The clone() methods are left-over cruft from an earlier era, and I don't think they are used anywhere (except in some unit tests)

  • It is probably more efficient if you alter it to get both confidence and mean in one go. Must users of truth values request both in rapid succession.

  • Its probably better if you change this to derive from FloatValue instead of TruthValue -- that way, someone else could track 6 or 12 different numbers if they wished, instead of being limited to only only 2 or 3. (So, for example, in my work, I usually track: the count n, the normalized frequency n/N between 0.0 and 1.0 and also log-of-frequency, and sometimes log-of-count. Why so many? Its handy, convenient. None of them are truly "mean" or "confidence", which are something else again...)

Other than that, I think I like it.

I might spend the rest of the afternoon surgically removing clone() from truth values -- I really think its just obsolete/unused/un-needed. I need to look more carefully...

@linas
Copy link
Member

linas commented Apr 22, 2019

@noskill I just removed the clone() method here: #2156 If you make the suggested changes above, I'd be happy to merge this into mainline. It might be better if the code lived in the opencog/cython directory, instead of the opencog/atoms/truthvalue directory, but I'm not sure it matters all that much; I guess either is probably OK in either directory.

@noskill
Copy link
Contributor Author

noskill commented Nov 25, 2019

We published paper describing this proposal http://agi-conf.org/2019/wp-content/uploads/2019/07/paper_19.pdf
It was implemented in singet repository.

It can be reimplemented with this DynamicTruthValue, since it is still pointer to arbitrary pyobject..

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants