Change OpenPsi rule definition

[ferrouswheel]

I need to change the layout of openpsi rules.

Currently the definition is:

(ImplicationLink  <TV>
                (AndLink
                    (context1)
                    (context2)
                    ...
                    (action))
                (goal))

but AndLink is unordered, so, as far as I can tell, there is no way to preserve the distinction between context and action when querying the atomspace.

I think the only way this "worked" in the current implementation is that OpenPsi didn't really use the AtomSpace, it just kept it's own internal index of rules, contexts, and actions.

Proposal is to change to:

(ImplicationLink  <TV>
                (ListLink
                    (AndLink
                       (context1)
                       (context2)
                       ...
                    )
                    (action))
                (goal))

But I'm not sure how much this is in keeping with current thinking around atom types.

[linas]

Perhaps SequentialAnd instead of ListLink might be more obvious. I too had needed this, at one point, for language.

[linas]

There's another thing openpsi doesn't currently do: allow variables in the contexts. I briefly had a version that allowed that (I needed them for language) but apparently this caused problems for Amen. It still seems like a good idea, but I have not thought about it recently.

[ferrouswheel]

Trying to use a SequentialAndLink instead of a ListLink gives me an "Invalid Atom syntax" error:

Invalid Atom syntax: (SequentialAndLink
  (AndLink
    (InheritanceLink
      (VariableNode "x1") ; [3075737219957315725][1]
      (VariableNode "z1") ; [7789777154810158308][1]
    ) ; [9620943575462120714][1]
    (EqualLink
      (VariableNode "x1") ; [3075737219957315725][1]
      (VariableNode "y1") ; [4509780737983488288][1]
    ) ; [11504100731908177554][1]
    (ListLink
      (VariableNode "x1") ; [3075737219957315725][1]
      (VariableNode "y1") ; [4509780737983488288][1]
      (ConceptNode "Required constant for DualLink-1") ; [6514141880010778379][1]
      (VariableNode "z1") ; [7789777154810158308][1]
    ) ; [13018497789937224059][1]
  ) ; [16988107234594482386][1]
  (ExecutionOutputLink
    (GroundedSchemaNode "scm: act-1") ; [8243806699251409518][1]
    (ListLink
      (VariableNode "$abc") ; [7271896597199682180][1]
    ) ; [16159871516354589992][1]
  ) ; [12776168277915291596][1]
) ; [11163037888953457227][-1]

Debugging where this is coming from I tracked it to the class server validator, and it appears that SequentialAndLink requires components to be evaluatable. AndLink has an explicit exception here.

Neither AndLink or ExecutionOutputLink are in the allowed types.

So I guess the options are to allow an exception for SequentialAndLink (same as AndLink) or a different type.

[ferrouswheel]

As I understand it, openpsi allows variables in the contexts, but they can't currently be referenced in the associated action... which is a somewhat severe limitation.

[linas]

OK .. so AndLinks are evaluatable. This one has a problem though: not all of it's contents are. So its kind-of syntactically broken. (The equal link is evaluatable, the inheritance link and the list link are not) For backwards compat, such brokenness is allowed, but is poor form.

The SequentialLink is stricter in checking (there is no backwards-compat requirement): it knows the And could be evaluatable, but the ExOut link is not (ExOut generates atoms when executed, but SequentialLink wants TV's to operate on). Thus, sequential-and throws an error.

[linas]

BTW when I said "OpenPsi doesn't allow variables in contexts", I meant "it doesn't know how do do anything useful with variables in contexts, and will probably generate garbage as a result". I did not mean that it actually checks and throws an error if you accidentally use them.

[leungmanhin]

I think OpenPsi marks which part is "context" and which part is "action" when a rule is created, and there is psi-get-context and psi-get-action to get them.

[ferrouswheel]

@leungmanhin yeah, I need to change how openpsi works because its "index" is independent from the atomspace.. which is why it gets in trouble when you start pushing and popping atomspaces.

This is why I'm running into the problem with AndLink being unordered. I'm surprised it hasn't come up before, since anyone using the AtomSpace won't see an accurate representation of the psi rules. It'll be a random mix of context and actions.

[leungmanhin]

Regarding variables, I recall there is some special handling in the code. Though context and action are all in one AndLink, they are actually evaluated separately, as the action will only be triggered when the context is satisfied (and the rule is selected). But sometimes one may want to reuse some of the variable groundings in the action, and it wasn't possible unless they were all evaluated at the same time. So I think OpenPsi actually holds the groundings of variables somewhere for the actions, and that may have been the main reason why part of OpenPsi is written in C++, correct me if I'm wrong...

[leungmanhin]

@ferrouswheel Oh right I see

[linas]

But sometimes one may want to reuse some of the variable groundings in the action, and it wasn't possible unless they were all evaluated at the same time.

Um, but of course, this describes the patter-matcher, doesn't it? This is what the pattern matcher is designed to do. However, I'm guessing that "there is more to it than just that", but that is because I never had a very clear idea of how open-psi worked. Which is why I'm asking for a detailed explanation :-)

[leungmanhin]

Yeah, let me try to explain the situation... Let's say we have a few psi-rules in AtomSpace and want to select and trigger one of them in order to satisfy certain goals. The current selection criteria takes at least a few things into account -- satisfiability of the context, TV of the rule, STI of the rule, and the urges of the goals that it can potentially satisfy. A weight will then be calculated and assigned to each of the rule, and the action/rule selector will pick accordingly so that the higher-weighted ones will more likely be selected.

So in order to calculate that weight, OpenPsi has to take out and evaluate the contexts of all the rules first, calculate the weights, select one of them, and then trigger its action. Although everything is in the same ImplicationLink, the context evaluation and action triggering are two separate processes within OpenPsi so the groundings of variables cannot be passed to the action directly like what a BindLink does. I believe that's what some of those Pattern Matcher custom callbacks in OpenPsi try to do -- to keep the groundings and pass them to the action when needed.

[linas]

let me try to explain the situation...

Those paragraphs belong either in a README, or (better yet?) the wiki. That way, you can just say RTFM.

[ngeiswei]

Regarding connecting variables in context and action I suggest to use ImplicationScopeLink, see https://wiki.opencog.org/w/ImplicationScopeLink.

Regarding distinguishing context and action, I suggest not to use a ListLink to make that distinction, rules must be semantically correct to allow PLN reason about them (and likely some form of planning can be achieved with reasoning so this is a rather important point). Instead this should be doable by merely parsing the rule because the action is a specialized and known kind of context, I suggest to introduce a predicate "do" as in Judea Pearl https://www.quora.com/What-is-Judea-Pearls-work-on-causality-in-a-nutshell (or https://arxiv.org/pdf/1305.5506v1.pdf for a thorough introduction) to make the distinction clear.

Also, ideally, as soon as temporal reasoning is supported rules should be represented with PredictiveImplicationLink (or PredictiveImplicationScopeLink, or anything that captures temporal knowledge) as to properly manage time during planning.

Please let me know if any of the above needs more elaboration.

[ngeiswei]

BTW, the URE control mechanism is a specialized form of OpenPsi that could be more broadly used, inspired, in particular it performs rule combination and action selection in a somewhat optimal fashion, in theory at least!

1. Rule combination is done with Baysian Model Averaging, actually some limited form of Solomonoff Operator Induction (https://github.com/ngeiswei/papers/blob/master/PartialBetaOperatorInduction/PartialBetaOperatorInduction.pdf).
2. Action selection is done with Thompson Sampling (http://auai.org/uai2016/proceedings/papers/20.pdf).

For more detail please see https://github.com/opencog/opencog/blob/master/examples/pln/inference-control-learning/README.md#inference-control-rule

For the model combination code see https://github.com/opencog/atomspace/blob/master/opencog/ure/MixtureModel.h

For the action selection code see https://github.com/opencog/atomspace/blob/master/opencog/ure/ActionSelection.h

[ferrouswheel]

Thanks @ngeiswei. It seems that ImplicationScopeLink might be all that is needed? Does this work with the pattern matcher?

As an example, from OpenPsi's test case:

(ImplicationLink (stv 1 1)
     (AndLink
        (ListLink
           (VariableNode "x2")
           (ConceptNode "Required constant for DualLink-2")
           (VariableNode "z2")
        )
        (InheritanceLink
           (VariableNode "x2")
           (VariableNode "z2")
        )
        (NotLink
           (EqualLink
              (VariableNode "x2")
              (VariableNode "z2")
           )
        )
        (ExecutionOutputLink
           (GroundedSchemaNode "scm: act-2")
           (ListLink
              (VariableNode "$abc")
           )
        )
     )
     (ConceptNode "goal-2")
  )

would become:

(ImplicationLink (stv 1 1)
     (ImplicationScopeLink
         (VariableList
           (VariableNode "x2")
           (ConceptNode "Required constant for DualLink-2")
           (VariableNode "z2")
         )
        (AndLink
           (InheritanceLink
              (VariableNode "x2")
              (VariableNode "z2")
           )
           (NotLink
              (EqualLink
                 (VariableNode "x2")
                 (VariableNode "z2")
              )
           )
        )
        (ExecutionOutputLink
           (GroundedSchemaNode "scm: act-2")
           (ListLink
              (VariableNode "$abc")
           )
        )
     )
     (ConceptNode "goal-2")
  )

However I'm unsure about the validity of nested Implications.

If the alternative is to have ImplicationScopeLink as the root of the rule. And that the action be nested in some kind of "do" link as per your suggestion. Then I could do that too.

Regarding PredictiveImplication, it feels like this should be the general case, and ImplicationLink is a PredictiveImplicationLink with a time delta of 0?

[ferrouswheel]

If OpenPsi could be made to run with URE, this would be great, but the refactor I'm doing is for a specific project and I think such a migration may be too big a task at this stage. On the plus side, after delving into OpenPsi I should have a good enough understanding to convert it to a new backend...

[linas]

This does not make sense, as constants cannot be variables.

         (VariableList
           (VariableNode "x2")
           (ConceptNode "Required constant for DualLink-2")
           (VariableNode "z2")
         )

Note that VariabeLists are often optional: if not explicitly specified, all found variables will be assumed to be bound.

Oddly, you did NOT specify (VariableNode "$abc") which would leave it free, and that is probably not what you want.

The mention of "DualLiink" here suggests that ... well, please review the wiki page for DualLink, as it might not work the way you are imagining it to. In short: instead of searching for all patterns that satisfy a query, it does the opposite: given a fixed pattern (all constants) it searches for all queries that might be satisfied by that pattern.

It works for the above, because anything scoped is a constant. (well, but you had $abc free, so that a little bit broken)

[ferrouswheel]

Thinking about it, the Do predicate with a scope link does sound appealing. Would that look like the following?

(ImplicationScopeLink (stv 1 1)
  (VariableList
    (VariableNode "x2")
    (ConceptNode "Required constant for DualLink-2")
    (VariableNode "z2"))
  (AndLink
    (InheritanceLink
      (VariableNode "x2")
      (VariableNode "z2"))
    (NotLink
      (EqualLink
        (VariableNode "x2")
        (VariableNode "z2")))
    (Do
      (ExecutionOutputLink
        (GroundedSchemaNode "scm: act-2")
        (ListLink
          (VariableNode "$abc"))))
  (ConceptNode "goal-2")
)

Note, I'm not sure about the ConceptNode in the VariableList, or the action variable having no correspondance to the context variables. This is just what is in the psi unit test.

If adding this Do type is the agreed course of action, is there documentation that describes what is necessary to add a new atom type?

[ferrouswheel]

Thanks Linas. It looked weird to me, but yeah.. it's just what's in the unit tests so I have to assume it means something until I discover it doesn't ;-)

[amebel]

As I understand it, openpsi allows variables in the contexts, but they can't currently be referenced in the associated action... which is a somewhat severe limitation.

What Manhin described in #3548 (comment) is right. The grounding result of the context is stored in the cache.

ImplicationScopeLink doesn't handle storage of variable grounding, that is the job of the pattern-matcher callbacks, which the openpsi code does. URE's chaining algorithm most likely does this as well, as it is a requirement for the explore vs exploit choice.

[amebel]

Thinking about it, the Do predicate with a scope link does sound appealing. Would that look like the follow?

(ImplicationScopeLink (stv 1 1)
  (VariableList
    (VariableNode "x2")
    (ConceptNode "Required constant for DualLink-2")
    (VariableNode "z2"))
  (AndLink
    (InheritanceLink
      (VariableNode "x2")
      (VariableNode "z2"))
    (NotLink
      (EqualLink
        (VariableNode "x2")
        (VariableNode "z2")))
    (Do
      (ExecutionOutputLink
        (GroundedSchemaNode "scm: act-2")
        (ListLink
          (VariableNode "$abc"))))
  (ConceptNode "goal-2")
)

Note, I'm not sure about the ConceptNode in the VariableList, or the action variable having no correspondance to the context variables. This is just what is in the psi unit test.

If adding this Do type is the agreed course of action, is there documentation that describes what is necessary to add a new atom type?

This does solve the typing of an action, but doesn't solve the problem of having to parse the context, goal, and action during each loop. The index might still be needed, for performance reasons, if we seek to minimize the number of atom types. We can use values for that, so as to keep it "closer" to the atomspace.

[ferrouswheel]

@amebel I'm talking about the singleton that is explicitly called the "openpsi_cache", not the implicator. https://github.com/opencog/opencog/blob/master/opencog/openpsi/OpenPsiRules.h#L163

Eventually I'll have to deal with the implicator too, since it also permanently attaches itself to an AtomSpace. But the rule cache should be easy enough once we decide an alternative rule structure.

[amebel]

@ferrouswheel The openpsi_cache is used for saving on pattern matching time in search of context, action, and goal. between every openpsi loop. So, when an openpsi rule is added using OpenPsiRules::add_rule the elements of the rules are cached.

At the time of authoring the code, I think values were not mature. Using values for replacing PsiTuple should be a solution for removing the singleton, regardless of what representation is accepted for openpsi rules. The scheme version is like,

(define rule (ImplicationLink ...))
(define context-atoms (LinkValue atom-1 atom-2 ...))
(cog-set-value! (Predicate "psi-context") context-atoms)
(cog-set-value! (Predicate "psi-action") action-atom)
(cog-set-value! (Predicate "psi-goal") goal-atom)
(cog-set-value! (Predicate "psi-query") (SatisfactionLink (cog-value->list context-atoms)))

[ngeiswei]

@ferrouswheel said:

If the alternative is to have ImplicationScopeLink as the root of the rule. And that the action be nested in some kind of "do" link as per your suggestion. Then I could do that too.

yes, that is what I mean, which is what you attempted here

(ImplicationScopeLink (stv 1 1)
  (VariableList
    (VariableNode "x2")
    (ConceptNode "Required constant for DualLink-2")
    (VariableNode "z2"))
  (AndLink
    (InheritanceLink
      (VariableNode "x2")
      (VariableNode "z2"))
    (NotLink
      (EqualLink
        (VariableNode "x2")
        (VariableNode "z2")))
    (Do
      (ExecutionOutputLink
        (GroundedSchemaNode "scm: act-2")
        (ListLink
          (VariableNode "$abc"))))
  (ConceptNode "goal-2")
)

however, if you use do then you wouldn't need ExecutionOutputLink any longer (in fact it would be wrong, unless your action is actually obtained the result of act-2 (i.e. act-2 would be a meta-action). What you'd write (ignoring the variable declaration shenanigan pointed out by Linas) is

(ImplicationScopeLink (stv 1 1)
  (VariableList
    (VariableNode "x2")
    (ConceptNode "Required constant for DualLink-2")
    (VariableNode "z2"))
  (AndLink
    (InheritanceLink
      (VariableNode "x2")
      (VariableNode "z2"))
    (NotLink
      (EqualLink
        (VariableNode "x2")
        (VariableNode "z2")))
    (Do
        (GroundedSchemaNode "scm: act-2")
        (ListLink
          (VariableNode "$abc")))
  (ConceptNode "goal-2")
)

We actually almost already have a do link! It is https://wiki.opencog.org/w/ExecutionLink

ExecutionLink seems to expect a result, which could be in the case of an action success or failure, success meaning that the action was performed (regardless of whether the goal is achieved), in such case we'd have:

(ImplicationScopeLink <TV>
 <variable-declaration>
 (AndLink
    <context>
    (Execution
        <action>
        <action-arguments>
        <success>))
  <goal>)

but we could also introduce a Do link as well, which would give us

(ImplicationScopeLink <TV>
 <variable-declaration>
 (AndLink
    <context>
    (Do
        <action>
        <action-arguments>))
  <goal>)

ExecutionLink could be used when the action has more feedback than just success or failure, for instance we could have cognitive schematics (i.e. a rule, see https://wiki.opencog.org/w/Cognitive_Schematic) predicting the probability of achieving the goal if the action was a failure, half a failure, and a success, etc. But I kinda like the DoLink, it's simple and likely just what we need most of the time.

[linas]

@ngeiswei The AndLink is unordered. But when we write examples of it, we pretend like it's ordered. And that was the original complaint: it would be nice to cleanly split out contexts from actions, so that one can tell which-is-which. Currently, openpsi keeps its own private-secret-list so that it magically knows which is which, while everyone else is kept in the dark...