-
Notifications
You must be signed in to change notification settings - Fork 725
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
Change OpenPsi rule definition #3548
Comments
Perhaps |
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. |
Trying to use a SequentialAndLink instead of a ListLink gives me an "Invalid Atom syntax" error:
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. |
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. |
OK .. so 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. |
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. |
I think OpenPsi marks which part is "context" and which part is "action" when a rule is created, and there is |
@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. |
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... |
@ferrouswheel Oh right I see |
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 :-) |
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. |
Those paragraphs belong either in a README, or (better yet?) the wiki. That way, you can just say RTFM. |
Regarding connecting variables in context and action I suggest to use Regarding distinguishing context and action, I suggest not to use a Also, ideally, as soon as temporal reasoning is supported rules should be represented with Please let me know if any of the above needs more elaboration. |
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!
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 |
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:
would become:
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? |
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... |
This does not make sense, as constants cannot be variables.
Note that VariabeLists are often optional: if not explicitly specified, all found variables will be assumed to be bound. Oddly, you did NOT specify 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 |
Thinking about it, the
Note, I'm not sure about the If adding this |
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 ;-) |
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. |
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. |
@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. |
@ferrouswheel The 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))) |
@ferrouswheel said:
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 (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
(ImplicationScopeLink <TV>
<variable-declaration>
(AndLink
<context>
(Execution
<action>
<action-arguments>
<success>))
<goal>) but we could also introduce a (ImplicationScopeLink <TV>
<variable-declaration>
(AndLink
<context>
(Do
<action>
<action-arguments>))
<goal>)
|
@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... |
I need to change the layout of openpsi rules.
Currently the definition is:
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:
But I'm not sure how much this is in keeping with current thinking around atom types.
The text was updated successfully, but these errors were encountered: