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

execute() should return a set not a handle #2530

Closed
linas opened this issue Mar 26, 2020 · 9 comments
Closed

execute() should return a set not a handle #2530

linas opened this issue Mar 26, 2020 · 9 comments

Comments

@linas
Copy link
Member

linas commented Mar 26, 2020

Below is a partially-finished thought for a fundamental architectural change. It is motivated by the discussion seen in MOZI-AI/annotation-scheme#165 The issue was this:

A GSN is called to perform several computations. Rather than just depositing the results of those calculations in the AtomSpace, it was felt that the GSN should "return" several results. Since, by default, the GSN can only return a single Handle, the multiple results were wrapped in a ListLink. This lead to a proliferation of un-needed, pointless ListLinks in the AtomSpace.

This could be "fixed" by allowing the GSN to return a list (an ordered list) or a set (an unordered set), instead of returning a Handle. Under the covers, this suggests that the execute() method should return a list or set. All atomspace intrnals would have to be modified to deal with this. If some caller of execute() really, really needed a single atom, then they could explicitly wrap it in a ListLink/SetLink at that time.

The advantage of doing this is that there is less pollution of the AtomSpace with ListLinks, SetLinks. The end-user no longer would need to manage these. Another advantage is that the AtomSpace currently has multiple spots where ListLinks, SetLinks are implicitly unwrapped; perhaps these implicit unwrappings can be eased?

What might the actual solution look like? I see several possibilities:

  • Change execute() to return std::vector<ValuePtr> or maybe std::set<ValuePtr>
  • Create an InternalUseOnlySetListValue and return that. This way the C++ signature doesn't change. We just have to be carefully to make sure the user never sees this special value.
  • Create some other polymorphic return type, reminiscent of Vertex in the MOSES code-base. This kind of polymorphism is already a central underpinning concept in LISP and Scheme. This would make Atomese more lisp-like. :-/

Both the second and the third bullets enable result-polymorphism. The main difference between the second and the third bullets above is that InternalUseOnlySetListValue is supposed to be hidden from the end-user, while the polymorphism of Vertex is explicitly exposed to the user. I'm not sure I like making it explicit -- this leads to a snowball of design decisions that would turn Atomese into just another also-ran, poorly-designed dialect of LISP .. and that is not the intent...

Somewhat related is issue #1502 aka "Stop using SetLink for search results!" The current working prototype there is to declare an AnchorNode to which return values are attached, as they appear. That is, the return value is really more of a future or a promise, that can be polled, or blocked on, until the results appear. This suggests a fourth idea:

The best answer remains cloudy; this requires more ... prototyping and experimentation.

@linas linas changed the title eexecute() should return a set not a handle execute() should return a set not a handle Mar 26, 2020
@ngeiswei
Copy link
Member

What about just having the pattern matcher wrap its results in a LinkValue instead of ListLink?

@linas
Copy link
Member Author

linas commented Mar 27, 2020

wrap its results in a LinkValue

If we make that visible to the end-user, that's the third bullet; if we try to hide it, for backwards compat, its the second bullet. The current internal implementation is already almost that -- almost not quite, its sloppier and more scattered. But its a good idea, I'll take a look and clean up.

Also - not just he pattern matcher, but "everything" - PutLink, etc.

What's everything?

  • GetLink - fixed, use MeetLink instead
  • BindLink - fixed, use QueryLink instead
  • DualLink
  • PutLink
  • MapLink
  • what else? what did we forget?

@linas
Copy link
Member Author

linas commented May 4, 2020

#2569 implements a QueueValue which is a thread-safe FIFO. It allows some threads to place items on the queue, and other threads to remove them. In all other respects, it behaves like LinkValue. The various pattern matchers now use this, in #2571

@linas
Copy link
Member Author

linas commented May 4, 2020

Also, when I first opened this issue, I completely forgot about QueryLink
this is a base class for BindLink, and behaves exactly like BindLink
except that it returns a LinkValue instead of a SetLink.

Until now QueryLink has remained undocumented and unused,
it seemed "experimental" but maybe now its ready for general use.
It was added in pull req #2117 -- more than a year ago!

@linas
Copy link
Member Author

linas commented Aug 12, 2020

Related: the cog-execute-cache! function will return the result of execuntion, and will also "cache" the result by storing it under a user-provided key. It will also write some meta-data about when the caching was performed. Documentation is here:

cog-execute-cache! EXEC KEY [METADATA [FRESH]]
   Execute or return cached execution results. This is a caching version
   of the `cog-execute!` call.

   If the optional FRESH boolean flag is #f, then if there is a Value
   stored at KEY on EXEC, return that Value. The default value of FRESH
   is #f, so the default behavior is always to return the cached value.

   If the optional FRESH boolean flag is #t, or if there is no Value
   stored at KEY, then the `cog-execute!` function is called on EXEC,
   and the result is stored at KEY.

   The METADATA Atom is optional.  If it is specified, then metadata
   about the execution is placed on EXEC at the key METADATA.
   Currently, this is just a timestamp of when this execution was
   performed. The format of the meta-data is subject to change; this
   is currently an experimental feature, driven by user requirements.

   At this time, execution is synchronous. It may be worthwhile to have
   an asynchronous version of this call, where the execution is performed
   at some other time. This has not been done yet.

@linas
Copy link
Member Author

linas commented Aug 12, 2020

See also #2752

@linas
Copy link
Member Author

linas commented Dec 4, 2021

See also issue #2911 which wonders about the general problem, and proposes monads as a general solution.

@linas
Copy link
Member Author

linas commented Dec 15, 2022

As far as I can tell, GSN's can return general Values, not just Handles.

TODO would be a GroundedProcedureNode that would accept Values as arguments.

@linas
Copy link
Member Author

linas commented Dec 16, 2022

Closing. At this point in time, GSN's can return Values just fine, it all works. They can return QueueValues if desired. A more general framework to allow data to flow is described in issue #2911.

Anway, here's a demo. It works:

(use-modules (opencog))
(use-modules (opencog exec))
(define (msg atom btom)
  (format #t "duuude yo ~A and ~A\n" atom btom)
  (FloatValue 0 1 2 3 4))

(cog-execute!
   (ExecutionOutput (GroundedSchema "scm:msg")
      (List (Concept "a")(Concept "b"))))

@linas linas closed this as completed Dec 16, 2022
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