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

Add OR to exception in evaluatable checker #2622

Closed

Conversation

kasimebrahim
Copy link
Contributor

I needed this in asmoses when creating representation.

@linas
Copy link
Member

linas commented May 18, 2020

Why?

AndLink is treated differentlty due to a historical mistake; I'm in the process of slowly erasing that mistake. I do not want to spread that mistake to OrLink.

I'm guessing that perhaps you should be using ChoiceLink which is a non-boolean-logic Or useful for presenting a menu of choices?

@kasimebrahim
Copy link
Contributor Author

Yeah so do you remember the representation atomeses we were talking about on #2546?
some thing like

 
(define the-expr
  (AndLink
    (PredicateNode "$a")
    (CondLink
      (EqualLink (Variable "$knob-2") (NumberNode 1))
      (PredicateNode "$b")
      (NotLink (PredicateNode "$b")))))

I needed it to do the same when we have OrLink inplace of AndLink.

@linas
Copy link
Member

linas commented May 18, 2020

I think what you need is to add

+++ b/opencog/atoms/core/Checkers.cc
@@ -56,6 +56,7 @@ bool check_evaluatable(const Handle& bool_atom)
                if (VARIABLE_NODE == t) continue;
                if (GLOB_NODE == t) continue;
                if (DEFINED_PREDICATE_NODE == t) continue;
+               if (COND_LINK == t) continue;
 
                // Allow conjunction, disjunction and negation of
                // predicates. Since it cannot inherit from EVALUATABLE_LINK

I haven't tested, but I think that's the actual issue you are facing

@kasimebrahim
Copy link
Contributor Author

One more thing the CondLink is not actually CondLink, it is a KnobLink[gets converted to CondLink when execution though] which contains more information than CondLink when constructing representation.
example of an actual rep will be.

(OrLink
  (KnobLink
    (VariableNode "$knob-376fd81c")
    (PredicateNode "$2")
    (NumberNode "0 1 2")
    (FalseLink ) 
    (FalseLink )))

But I could check if it is FunctionLink?

@linas
Copy link
Member

linas commented May 18, 2020

Test the patch above, see if that fixes it for you. Also, the checker infrastructure should be updated to print a more informative error message: e.g "found a non-evaluatable atom XYZ in a link that will be evaluated".

@linas linas closed this May 18, 2020
@linas
Copy link
Member

linas commented May 19, 2020

Ho about if (nameserver().isA(t, COND_LINK)) ? I want to keep this as narrow/strict as possible, at least for now.

There are several meta-issues that haven't been solved; one is that static type-checking is ... OK but imperfect, and that dynamic type-checking is equally hap-hazard. Both are kind of a kludge, but I want to avoid making them even kludgier by having too many exceptions. The correct, long-term "perfect" solution is not yet clear.

@linas
Copy link
Member

linas commented May 19, 2020

I'm assuming KnobLink inherits from CondLink ...

@linas
Copy link
Member

linas commented May 19, 2020

Oh, well, you could just also make make KobLink inherit from EvaluatableLink, and then no changes to atomspace source are needed. So if you have some asmoses_atoms.script file then just change

KNOB_LINK <- WHATEVER

to

KNOB_LINK <- WHATEVER, EVALUATABLE_LINK

and it will all just work.

@kasimebrahim
Copy link
Contributor Author

Thanks that works.

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

Successfully merging this pull request may close these issues.

2 participants