GroundedObjectNode is desirable

[Necr0x0Der]

This issue is closely related to #42 , but is a separate and more controversial one. The idea is to implement a support for GroundedObjectNodes in addition to GroundedPredicate/Schema nodes. The idea comes from the necessity to access (e.g. python) objects from schema nodes grounded in methods of other objects, and these methods can accepts these objects as arguments, so names of references to these objects can change, so referring to objects by their (python) names is much worse than referring to functions by their names. Thus, grounded objects should be accessed by their Atomese names or as Atoms. For example, C++ API for GroundedSchemas uses LibraryManager with setLocalFunc, which associates the given name with the pointer to the function, so it can be accessed from anywhere by this name (while PythonEval retrieves PyObject* pointer to the function each time by its Python name).
Implementing GroundedObjectNodes, which either store pointers to the grounding objects inside them or in a sort of global LibraryManager, seems to be a convenient (although not absolutely necessary) way to handle certain use cases like Neural Module Networks implemented as a set of (possibly dynamically created) objects, and can have other use cases.
The first questions is, are there objections against implementing GroundedObjectNode (like "this functionality should be implemented in the target language using existing API" or whatever)?

[ngeiswei]

It makes sense, I have no objection. @Necr0x0Der I think you should duplicate that issue in opencog/atomspace as it is a rather core one.

[Necr0x0Der]

Thanks, @ngeiswei . I'll do this slightly later with more details (and after dealing with #42 ).

[Necr0x0Der]

@ngeiswei , I'm starting to think that it would be easier right now to avoid introducing new GroundedObjectNode type. What we mostly need now is to run object methods from URE/BC as formulas for rules. If we have something like

(ExecutionOutputLink
   (GroundedObjectNode "DNN_module")
   (ConceptNode "forward") ; method to be called
   (ListLink ... arguments

then, we will need to extend URE to deal with formulas implemented in GroundedObjectNodes, because it extracts premises from formula arguments, etc. It's not a big deal, but still requires some changes.
For now, it would be ok to have something like (GroundedSchemaNode "obj: DNN_module.forward) (where DNN_module will be not a python name of the object, but a registered Atomese name like in the case of setLocalFunction), so it will be completely consistent with the existing functionality of Pattern Matcher and URE.
Maybe, there are other use cases, in which GroundedObjectNode would be useful (beyond the access to its methods), but I don't have them in mind.
What do you think?

[ngeiswei]

Oh, I'm confused, I think I might have misunderstood the purpose of GroundedObjectNode, I thought it was merely equivalent to a nullary grounded schemata, but you seem to suggest it is something else.

So you do mean that DNN_module would be a class object (previously registered via something equivalent to setLocalFunction for class objects), and forward would be a method of it?

[Necr0x0Der]

yes

[vsbogd]

BTW, we could have some GroundedSchemaNode contructors or factory methods to produce GSN with specific object and method (or function) reference. For instance in Python API we could have:

gsnA = GroundedSchemaNode("gsnA", lambda x : mm.nn.submodule.forward(x)) # call forward on mm.nn.submodule
eolA = ExecutionOutputLink(gsnA, ListLink(ConceptNode("aa"), ConceptNode("bb")))
execute_atom(a, eolA) == ConceptNode("bb")

gsnB = GroundedSchemaNode("gsnA", lambda x : nn.forward(x)) # call forward on nn
eolB = ExecutionOutputLink(gsnB, ListLink(ConceptNode("aa"), ConceptNode("bb")))
execute_atom(a, eolB) == ConceptNode("aa"),

In example above the code which will be executed is not encoded in GSN name but injected using language specific API constructor. And GSN name is just an id to find corresponding node in the atomspace.

[Necr0x0Der]

@vsbogd , how do you suppose GroundedSchemaNode will accept a python lambda function as input? It doesn't do this. It accesses functions by names. Again, we can do something similar to setLocalFuncion for python functions, so it will assign an Atomese name for a function, which will be stored in LibraryManager as PyObject*. But this would be the same as proposed for "obj:", but without access to all class methods...

[ngeiswei]

@vsbogd, you mean "gsnB" in the second GroundedSchemaNode definition, right?

Also I don't understand gsnA and gsnB seems to be unary functions by yet are called with 2 values aa and bb (and the outputs are one of these, it's confusing).

[ngeiswei]

@vsbogd in case you meant to represent a record of mapping from inputs to outputs, don't forget that ExecutionLink would be used instead (although it is not currently processed by the atomese interpreter/pattern matcher).

[vsbogd]

Actually I found that my idea is very similar to manual functions registration which were implemented by Alexey some time ago. And it obviously doesn't work in case of atom serialization/deserialization. So it is not very useful.

[vsbogd]

I didn't keep in mind that keeping function names as strings in GSN allows resolving them at the moment of execution even after atom deserialization.

[ngeiswei]

The other option would be to have the constructor turn the python code into actual atomese code (using LambdaLink, etc), but that's probably overkill for now. ;-)

[Necr0x0Der]

I'm afraid Atomese is far from being expressive enough for this (not to mention efficiency issues and capabilities to process values and data structure).

[Necr0x0Der]

Well, it seems we do have a use case for GroundedObjectNode besides accessing its methods via ExecutionOutputLink or EvaluationLink. Namely, it would be convenient for us to pass GroundedObjects (e.g. tensors) to GroundedSchemas. Since we need to avoid converting pytorch tensors into FloatValues to maintain a differentiable computational graph, we either need a PointerValue to pass between grounded schemas, or to pass GroundedObjects which are connected to PyObject*... So, we are thinking again about implementing GroundedObjectNodes...

[noskill]

Added new issue in opencog's repo opencog#1970 describing our usecases

[Necr0x0Der]

We would like to continue the discussion on GroundedObjectNode (GON) possible implementations.
We saw different ways (or a number of separate or interrelated choices to be made) with different pros and cons (technical, organizational, subjective, etc).
First of all, we want GroundedObjectNode for two reasons: to pass foreign objects to grounded functions and to execute methods of foreign objects as grounded functions. It is discussable if foreign objects should be represented by Atoms in Atomspace instead of Values. But it would be quite convenient in many use cases: objects can live for a long time, while values of their fields can be much more dynamically mutable. So, it is a convenient mechanism to access values stored in any structured way in foreign objects (while complex data structures are not well supported in Atomese).
So, we have the following design choices:

1. Pointers to actual foreign/non-Atomese objects can be stored in the field of GroundedObjectNode C++ object itself or in a LibraryManager (as it is done for the "lib" case of GroundedSchemaNode and GroundedPredicateNode). If we use LibraryManager, we can access object methods even without GroundedObjectNode type and using existing ExecutionOutput and GroundedSchema links. In this case, do_execute should simply be extended to deal with something like (GroundedSchemaNode "obj: DNN_module.forward), where DNN_module is a name explicitly associated with some object (like setLocalSchema is used to associate C++ functions with some names to access them from GroundedSchemaNode). So, if we want to avoid introducing GON, we can use this mechanism to access object methods (but not to passing them as arguments). Passing them as arguments might be done with the use of PtrValues though. But if we want to introduce GON, then there is no benefit in storing pointers to objects in GON. Storing these pointers in GON objects would be more modular and extendable I guess. For example, we can introduce GroundedPythonObject inherited from GON (although personally I would not like to have too many Atom types; what seems reasonable from traditional software engineering practice, might not be too good for AGI-ish systems; although we can have GroundedPythonObject class which is represented by the same GON Atom in Atomspace). BTW, this causes thoughts regarding removing LibraryManager at all and storing pointers to grounded library functions in GroundedSchemaNodes themselves, but this is a minor comment...
   So, is it better to have something like

class GroundedObjectNode: public Node
{
    private:
         void * foreign_object;
    ...

2a) Accessing GON methods. The question is: is it better to reuse ExecutionOutputLink and GroundedSchemaNode. If we reuse ExecutionOutputLink, we have different syntactic and implementation choices here. E.g.,

ExecutionOutputLink
    DotLink
        GroundedObjectNode some_object
        ConceptNode field/method name
    ListLink arguments

or even without additional DotLink

ExecutionOutputLink
    GroundedObjectNode some_object
    ConceptNode field/method name
    ListLink arguments

Unfortunately, DotLink cannot return a GroundedSchemaNode pointing to the selected method without refactoring GroundedSchemaNode (e.g. without removing LibraryManager and storing pointers to functions inside GSN). Also, if we are accessing not methods, but fields, DotLink shouldn't return an Atom... So, the approach with DotLink needs more elaboration, although it might be very useful in terms of making "declarative API" to foreign atoms.
In any case, both these options require extending ExecutionOutputLink since its execute() method explicitly verifies that the first argument is GSN (and it is DotLink in the first case even if executing this DotLink will return GSN). This extension can cause some problems with future merging with opencog/atomspace.
Another approach would be to introduce some general GSN (written in C++ and registered in LibraryManager). This can look like:

ExecutionOutputLink
    GroundedSchemaNode "lib: call_object_method"
    ListLink
        GroundedObjectNode some_object
        ConceptNode field/method name
        ListLink arguments

Benefits of this approach is that it can be implemented without touching execute or do_execute of ExecutionOutputLink, and it will use both ExecutionOutputLink and GroundedSchemaNode, which are expected by URE (and possibly Pattern Matcher). But it's not too good in terms of interpretability. E.g. all formulas for URE (in some DNN-relate use case) will be grounded in same "lib: call_object_method" GSN...

2b) If we don't reuse ExecutionOutputLink, we can introduce either a special link to handle object method execution or a link which will handle both GSN and GON methods.
In the first case, we'll have class ExecutionObjectMethodLink: public FunctionLink and its execute() method will act like this method for ExecutionOutputLink (it will check if the first argument is GON and perform accordingly).
In the second case, we'll have class AMoreGeneralExecutionOutputLink: public ExecutionOutputLink and its execute() method will check if the first argument is GSN (and call ExecutionOutputLink::execute) or GroundedObjectNode/DotLink/etc. (and do its own stuff). Technical implications of this approach are not clear to me. Apparently, URE expects ExecutionOutputLink and GNSs, so it should be modified. Hopefully, it's not a problem. However, I'm not sure if everything else will work smoothly with new Execution link, and what else should be modified. We will experiment with this shortly.

@ngeiswei , @vsbogd , @noskill , @pennachin any comment or insight is welcome

[ngeiswei]

Thanks for the clear presentation. It seems to me

• 1. and the second part of 2.a. (call_object_method) are the most expedient.
• the first part of 2.a and 2.b are the most versatile and informative but also the most effortful. However I do not anticipate much problem with the URE if you go with 2.b.

I like 1. for its expedience, but I don't know to which extend it is gonna limit yourselves, due to not being able to decouple object and member.

I can't really find any that should be discarded outright, so I would say go with what you think is best.

[vsbogd]

@ngeiswei, as far as I understand second part of 2.a also requires changes in URE, because ExecutionOutputLink + GroundedObjectNode will not be supported for free, but may be effort is less then for the first part of 2.a?

I like 2.b and DotLink most, i.e. replacing ExecutionOutputLink by AMoreGeneralExecutionOutputLink and representing object method call as:

AMoreGeneralExecutionOutputLink
    DotLink
        GroundedObjectNode some_object
        ConceptNode field/method name
    ListLink arguments

[vsbogd]

I have tried to create custom ExecutionOutputLink. Small fixes in Instantiator class and ExecutionOutpuLink itself were required to make it possible. Main fix is replacing checks type == EXECUTION_OUTPUT_LINK by type isA EXECUTION_OUTPUT_LINK.

Now it is possible to create own ExecutionOutputLink by inheriting ExecutionOutputLink class and EXECUTION_OUTPUT_LINK type and overriding ExecutionOutputLink::execute() method.

Few facts about current implementation:

• In fact ExecutionOutputLink is executed using Instantiator class. Instantiator executes any FunctionLink link using child atomspace where all variables of FunctionLink are grounded. It contains list of ifs which recognizes specific atom types and does different things to execute them.
• FunctionLink::execute() signature

  atomspace/opencog/atoms/core/FunctionLink.h

  Line 78 in 8df878a

  virtual ValuePtr execute() const;

  is different to ExecutionOutputLink::execute()

  atomspace/opencog/atoms/execution/ExecutionOutputLink.h

  Line 61 in 8df878a

  virtual Handle execute(AtomSpace* as=nullptr, bool silent=false) const;

  ; so actually ExecutionOutpuLink doesn't reimplement FunctionLink interface;
• Instantiator can execute any subclass of FunctionLink which has no specific if yet; so one can create AMoreGeneralExecutionOutputLink inheriting its type from FunctionLink then Instantiator will execute it. I have tried to reimplement ExecutionOutputLink by this way for experiment and found that few URE unit tests fails;

[vsbogd]

I am going to implement GroundedObjectNode, AMoreGeneralExecutionOutputLink and DotLink to implement example from #43 (comment).

[ngeiswei]

I see, so apparently you don't even need to touch ExecutionOutputLink to enable ValuePtr support (though ultimately we might want to have ExecutionOutputLink support it, but it's not an immediate requirement).

[vsbogd]

Yes, Alexey and Anatoly have prepared first prototype version already. GroundedObjectNode at the moment seems like another option to implement which may have some advantages.

[Necr0x0Der]

Yes, my sketch is here:
https://github.com/singnet/semantic-vision/blob/master/experiments/opencog/cog_module/cog.py
and Anatoly is working on turning it into a full-fledged library (while Vitaly is working on GroundedObjectNode). The sketch is really light-weight, but some technical details (like smart caching of values calculated by grounded functions for avoiding their re-computation during "reasoning", which necessity was mentioned a long time ago, but was disregarded as a non-issue in opencog repo) can require more coding. They will be done inside python library rather than inside opencog for this implementation, but extending ExecutionOutputLink/EvaluationLink for GroundedObjectNode gives us room to do this on the opencog side......

[vsbogd]

Couple of obstacles so far:

(1) When atom is added into atomspace it is copied and copying procedure for nodes copies only node type and name which means that any other state will be lost.

Possible solutions are:
a) introduce new copying procedure for atoms and values, for instance Value::clone() method and reimplement it in GroundedObjectNode; I think there is no chance to merge it into opencog repo though;
b) keep pointer to the object inside of GroundedObjectNode as PtrValue, then it will be copied by standard AtomTable::add_atom method;

[Necr0x0Der]

@ngeiswei , @pennachin , what is your opinion? Is it better to use hacky PtrValue without introducing additional changes to the atomspace core, or to introduce such changes which will cause more divergence between opencog and singnet repos, but which might look less hacky and will enable introducing other stateful atom types (which is not too good in general, but might be convenient)?

[vsbogd]

(2) ExecutionOutputLink has internal check on its outgoing set type in constructor which child class cannot override due to C++ restrictions.
Possible solutions:

• inherit AMoreGeneralExecutionOutputLink from FunctionLink, it seems it will require more complex change in URE to support such link, and preliminary experiment shown that this implementation fails some backward chainer unit tests.
• commit check method overriding into singnet

[pennachin]

As I said before, I'm ok with whatever decision @ngeiswei and @vsbogd agree to, since I'm not personally doing merges back and forth. In a vacuum, I prefer to do the right thing from a design/architecture point of view at the cost of code divergence.

[vsbogd]

issue (2) is not actual anymore as there is PR to opencog/atomspace which disables type checks for ExecutionOutputLink children.

issue (1), I have implemented option (b) in my branch of singnet/atomspace/master hiding internal storage complexity behind API. In parallel I am working on OpenCog ClassServer improvements in opencog/atomspace.

Current result can be seen in my branch. I am going to discuss and improve API further and then raise PR to singnet/atomspace.

[vsbogd]

Implementation is done, closing it