From 2c031d8a56545e3ba243e9c8151be77f6662f3b6 Mon Sep 17 00:00:00 2001 From: Linas Vepstas Date: Sun, 14 Jun 2020 14:12:27 -0500 Subject: [PATCH 1/9] Remove if-then-else structure. --- opencog/atoms/pattern/PatternLink.cc | 23 +++++++++++++---------- 1 file changed, 13 insertions(+), 10 deletions(-) diff --git a/opencog/atoms/pattern/PatternLink.cc b/opencog/atoms/pattern/PatternLink.cc index 9bc1687e69..2ee160a7f4 100644 --- a/opencog/atoms/pattern/PatternLink.cc +++ b/opencog/atoms/pattern/PatternLink.cc @@ -444,10 +444,9 @@ void PatternLink::unbundle_clauses(const Handle& hbody) // list of clauses to be grounded. _pat.body = hbody; if (record_literal(hbody)) - { - /* no-op */ - } - else if (AND_LINK == t) + return; + + if (AND_LINK == t) { TypeSet connectives({AND_LINK, OR_LINK, NOT_LINK}); @@ -471,8 +470,10 @@ void PatternLink::unbundle_clauses(const Handle& hbody) _fixed.emplace_back(term); } } + return; } - else if (SEQUENTIAL_AND_LINK == t or SEQUENTIAL_OR_LINK == t) + + if (SEQUENTIAL_AND_LINK == t or SEQUENTIAL_OR_LINK == t) { TypeSet connectives({AND_LINK, SEQUENTIAL_AND_LINK, OR_LINK, SEQUENTIAL_OR_LINK, NOT_LINK}); @@ -487,27 +488,27 @@ void PatternLink::unbundle_clauses(const Handle& hbody) PatternTermPtr term(make_term_tree(hbody)); _pat.pmandatory.push_back(term); + return; } // A top-level OrLink with a single member. Unwrap it. // This interprets OrLink as a crisp boolean operator, // preventing alternate interpretations for it. - else if (OR_LINK == t and 1 == hbody->get_arity()) + if (OR_LINK == t and 1 == hbody->get_arity()) { - // BUG - XXX FIXME Handling of OrLink is incorrect, here. - // See also FIXME above. TypeSet connectives({AND_LINK, OR_LINK, NOT_LINK}); if (not unbundle_clauses_rec(hbody, connectives)) { PatternTermPtr term(make_term_tree(hbody)); _pat.pmandatory.push_back(term); } + return; } // A single top-level clause that is a NotLink. // This interprets NotLink as a crisp boolean operator, // preventing alternate interpretations for it. - else if (NOT_LINK == t) + if (NOT_LINK == t) { // XXX FIXME Handling of OrLink is incorrect, here. TypeSet connectives({AND_LINK, OR_LINK, NOT_LINK}); @@ -520,8 +521,10 @@ void PatternLink::unbundle_clauses(const Handle& hbody) if (not term->isVirtual()) _fixed.emplace_back(term); } + return; } - else if (not is_constant(_variables.varset, hbody)) + + if (not is_constant(_variables.varset, hbody)) { // There's just one single clause! PatternTermPtr term(make_term_tree(hbody)); From e86e5b1868b6aa5a4c22f10a08034b7e638c069c Mon Sep 17 00:00:00 2001 From: Linas Vepstas Date: Sun, 14 Jun 2020 14:54:06 -0500 Subject: [PATCH 2/9] Simplify the convoluted logic --- opencog/atoms/pattern/PatternLink.cc | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/opencog/atoms/pattern/PatternLink.cc b/opencog/atoms/pattern/PatternLink.cc index 2ee160a7f4..8c3f8964cb 100644 --- a/opencog/atoms/pattern/PatternLink.cc +++ b/opencog/atoms/pattern/PatternLink.cc @@ -569,12 +569,11 @@ bool PatternLink::unbundle_clauses_rec(const Handle& bdy, for (const Handle& ho : bdy->getOutgoingSet()) { if (record_literal(ho, reverse)) continue; + if (unbundle_clauses_rec(ho, connectives, reverse)) continue; - bool did_rec = unbundle_clauses_rec(ho, connectives, reverse); - recorded = recorded and did_rec; + recorded = false; Type ot = ho->get_type(); - if ((not did_rec and - not (ot == VARIABLE_NODE) and + if ((not (ot == VARIABLE_NODE) and not nameserver().isA(ot, EVALUATABLE_LINK)) or (ot == EVALUATION_LINK and 0 < ho->get_arity() and From 298bf702b5d2794cba96f9ef388269348b95ead9 Mon Sep 17 00:00:00 2001 From: Linas Vepstas Date: Sun, 14 Jun 2020 14:59:41 -0500 Subject: [PATCH 3/9] Remove dead code. --- opencog/atoms/pattern/PatternLink.cc | 10 ---------- 1 file changed, 10 deletions(-) diff --git a/opencog/atoms/pattern/PatternLink.cc b/opencog/atoms/pattern/PatternLink.cc index 8c3f8964cb..933ce33297 100644 --- a/opencog/atoms/pattern/PatternLink.cc +++ b/opencog/atoms/pattern/PatternLink.cc @@ -572,16 +572,6 @@ bool PatternLink::unbundle_clauses_rec(const Handle& bdy, if (unbundle_clauses_rec(ho, connectives, reverse)) continue; recorded = false; - Type ot = ho->get_type(); - if ((not (ot == VARIABLE_NODE) and - not nameserver().isA(ot, EVALUATABLE_LINK)) or - (ot == EVALUATION_LINK and - 0 < ho->get_arity() and - ho->getOutgoingAtom(0)->get_type() == PREDICATE_NODE)) - { - PatternTermPtr term(make_term_tree(ho)); - _pat.pmandatory.push_back(term); - } } return recorded; } From 8edd8bbb7aeebaf97ccddf223b5771ef0fef86af Mon Sep 17 00:00:00 2001 From: Linas Vepstas Date: Sun, 14 Jun 2020 15:10:37 -0500 Subject: [PATCH 4/9] Simplify the unbundle logic --- opencog/atoms/pattern/PatternLink.cc | 17 ++--------------- 1 file changed, 2 insertions(+), 15 deletions(-) diff --git a/opencog/atoms/pattern/PatternLink.cc b/opencog/atoms/pattern/PatternLink.cc index 933ce33297..e49f963514 100644 --- a/opencog/atoms/pattern/PatternLink.cc +++ b/opencog/atoms/pattern/PatternLink.cc @@ -473,7 +473,8 @@ void PatternLink::unbundle_clauses(const Handle& hbody) return; } - if (SEQUENTIAL_AND_LINK == t or SEQUENTIAL_OR_LINK == t) + if (SEQUENTIAL_AND_LINK == t or SEQUENTIAL_OR_LINK == t or + OR_LINK == t) { TypeSet connectives({AND_LINK, SEQUENTIAL_AND_LINK, OR_LINK, SEQUENTIAL_OR_LINK, NOT_LINK}); @@ -491,20 +492,6 @@ void PatternLink::unbundle_clauses(const Handle& hbody) return; } - // A top-level OrLink with a single member. Unwrap it. - // This interprets OrLink as a crisp boolean operator, - // preventing alternate interpretations for it. - if (OR_LINK == t and 1 == hbody->get_arity()) - { - TypeSet connectives({AND_LINK, OR_LINK, NOT_LINK}); - if (not unbundle_clauses_rec(hbody, connectives)) - { - PatternTermPtr term(make_term_tree(hbody)); - _pat.pmandatory.push_back(term); - } - return; - } - // A single top-level clause that is a NotLink. // This interprets NotLink as a crisp boolean operator, // preventing alternate interpretations for it. From 723db423e3ec6a0453c890b68ddd538eea43fe83 Mon Sep 17 00:00:00 2001 From: Linas Vepstas Date: Sun, 14 Jun 2020 15:16:34 -0500 Subject: [PATCH 5/9] More unbundle-clauses logic simplification --- opencog/atoms/pattern/PatternLink.cc | 16 +--------------- 1 file changed, 1 insertion(+), 15 deletions(-) diff --git a/opencog/atoms/pattern/PatternLink.cc b/opencog/atoms/pattern/PatternLink.cc index e49f963514..c0a3704c85 100644 --- a/opencog/atoms/pattern/PatternLink.cc +++ b/opencog/atoms/pattern/PatternLink.cc @@ -474,7 +474,7 @@ void PatternLink::unbundle_clauses(const Handle& hbody) } if (SEQUENTIAL_AND_LINK == t or SEQUENTIAL_OR_LINK == t or - OR_LINK == t) + OR_LINK == t or NOT_LINK == t) { TypeSet connectives({AND_LINK, SEQUENTIAL_AND_LINK, OR_LINK, SEQUENTIAL_OR_LINK, NOT_LINK}); @@ -485,20 +485,6 @@ void PatternLink::unbundle_clauses(const Handle& hbody) // the PresentLink is reached. Whereas the current design // of the clause-walking will run the PresentLink before // running the sequential. So that's a bug. - unbundle_clauses_rec(hbody, connectives); - - PatternTermPtr term(make_term_tree(hbody)); - _pat.pmandatory.push_back(term); - return; - } - - // A single top-level clause that is a NotLink. - // This interprets NotLink as a crisp boolean operator, - // preventing alternate interpretations for it. - if (NOT_LINK == t) - { - // XXX FIXME Handling of OrLink is incorrect, here. - TypeSet connectives({AND_LINK, OR_LINK, NOT_LINK}); if (not unbundle_clauses_rec(hbody, connectives)) { PatternTermPtr term(make_term_tree(hbody)); From 184c87af2f53a69ea9290c2a929abe2c91255a74 Mon Sep 17 00:00:00 2001 From: Linas Vepstas Date: Sun, 14 Jun 2020 15:24:11 -0500 Subject: [PATCH 6/9] More simplification of unbundling code --- opencog/atoms/pattern/PatternLink.cc | 37 +++++++++------------------- 1 file changed, 11 insertions(+), 26 deletions(-) diff --git a/opencog/atoms/pattern/PatternLink.cc b/opencog/atoms/pattern/PatternLink.cc index c0a3704c85..cb32b6c7f8 100644 --- a/opencog/atoms/pattern/PatternLink.cc +++ b/opencog/atoms/pattern/PatternLink.cc @@ -473,33 +473,18 @@ void PatternLink::unbundle_clauses(const Handle& hbody) return; } - if (SEQUENTIAL_AND_LINK == t or SEQUENTIAL_OR_LINK == t or - OR_LINK == t or NOT_LINK == t) + TypeSet connectives({AND_LINK, SEQUENTIAL_AND_LINK, + OR_LINK, SEQUENTIAL_OR_LINK, NOT_LINK}); + + // BUG - XXX FIXME. This extracts PresentLink's from the + // Sequentials. This is not really correct, because the + // evaluation of the sequential might terminate *before* + // the PresentLink is reached. Whereas the current design + // of the clause-walking will run the PresentLink before + // running the sequential. So that's a bug. + if (not unbundle_clauses_rec(hbody, connectives) and + not is_constant(_variables.varset, hbody)) { - TypeSet connectives({AND_LINK, SEQUENTIAL_AND_LINK, - OR_LINK, SEQUENTIAL_OR_LINK, NOT_LINK}); - - // BUG - XXX FIXME. This extracts PresentLink's from the - // Sequentials. This is not really correct, because the - // evaluation of the sequential might terminate *before* - // the PresentLink is reached. Whereas the current design - // of the clause-walking will run the PresentLink before - // running the sequential. So that's a bug. - if (not unbundle_clauses_rec(hbody, connectives)) - { - PatternTermPtr term(make_term_tree(hbody)); - _pat.pmandatory.push_back(term); - - // if (not term->hasAnyEvaluatable()) - if (not term->isVirtual()) - _fixed.emplace_back(term); - } - return; - } - - if (not is_constant(_variables.varset, hbody)) - { - // There's just one single clause! PatternTermPtr term(make_term_tree(hbody)); _pat.pmandatory.push_back(term); From 8229c9035361c2fd03807bb3a555e56ebb250274 Mon Sep 17 00:00:00 2001 From: Linas Vepstas Date: Sun, 14 Jun 2020 15:32:34 -0500 Subject: [PATCH 7/9] Re-organize flow of execution --- opencog/atoms/pattern/PatternLink.cc | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/opencog/atoms/pattern/PatternLink.cc b/opencog/atoms/pattern/PatternLink.cc index cb32b6c7f8..041ece8115 100644 --- a/opencog/atoms/pattern/PatternLink.cc +++ b/opencog/atoms/pattern/PatternLink.cc @@ -438,15 +438,10 @@ _pat.pmandatory.push_back(term); /// callback class to complete the matchig process. void PatternLink::unbundle_clauses(const Handle& hbody) { - Type t = hbody->get_type(); - - // Start by fishing out the PresentLink's, and adding them to the - // list of clauses to be grounded. _pat.body = hbody; - if (record_literal(hbody)) - return; - if (AND_LINK == t) + // A collection of clauses, all of which must be satisfied. + if (AND_LINK == hbody->get_type()) { TypeSet connectives({AND_LINK, OR_LINK, NOT_LINK}); @@ -473,6 +468,11 @@ void PatternLink::unbundle_clauses(const Handle& hbody) return; } + // Fish out the PresentLink's, and add them to the + // list of clauses to be grounded. + if (record_literal(hbody)) + return; + TypeSet connectives({AND_LINK, SEQUENTIAL_AND_LINK, OR_LINK, SEQUENTIAL_OR_LINK, NOT_LINK}); From eb964e339b39812873c7fa02aa8e9fdd7ae8862d Mon Sep 17 00:00:00 2001 From: Linas Vepstas Date: Sun, 14 Jun 2020 16:45:33 -0500 Subject: [PATCH 8/9] Remove constants at the point of recording --- opencog/atoms/pattern/PatternLink.cc | 2 ++ 1 file changed, 2 insertions(+) diff --git a/opencog/atoms/pattern/PatternLink.cc b/opencog/atoms/pattern/PatternLink.cc index 041ece8115..90ad6d9221 100644 --- a/opencog/atoms/pattern/PatternLink.cc +++ b/opencog/atoms/pattern/PatternLink.cc @@ -906,6 +906,8 @@ void PatternLink::make_term_tree_recursive(const PatternTermPtr& root, { for (const Handle& ho: h->getOutgoingSet()) { + if ((PRESENT_LINK == t or ABSENT_LINK == t or ALWAYS_LINK == t) + and is_constant(_variables.varset, ho)) continue; PatternTermPtr po(ptm->addOutgoingTerm(ho)); make_term_tree_recursive(root, po); } From 53018e591c1c1f6d35dcb24979a5d4922e855d83 Mon Sep 17 00:00:00 2001 From: Linas Vepstas Date: Sun, 14 Jun 2020 17:03:03 -0500 Subject: [PATCH 9/9] Debugging aids --- tests/query/executable-pattern.scm | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/tests/query/executable-pattern.scm b/tests/query/executable-pattern.scm index f393e378f6..45efc81d1a 100644 --- a/tests/query/executable-pattern.scm +++ b/tests/query/executable-pattern.scm @@ -2,7 +2,7 @@ ; Data and tests for Executable Pattern ; -(use-modules ((opencog exec))) +(use-modules (opencog) (opencog exec)) (PlusLink (NumberNode 3) (NumberNode 5)) @@ -12,3 +12,5 @@ (TypedVariableLink (VariableNode "$A") (TypeNode "NumberNode")) (TypedVariableLink (VariableNode "$B") (TypeNode "NumberNode"))) (Present (PlusLink (VariableNode "$A") (VariableNode "$B"))))) + +; (cog-execute! plus-pattern)