From d4a04da2148a1347b0d78e09f0bc7576c2e46eaf Mon Sep 17 00:00:00 2001 From: Linas Vepstas Date: Thu, 4 Nov 2021 15:25:37 -0500 Subject: [PATCH 01/10] Save of partly-finished, in-development work almost works right :-) --- opencog/atoms/pattern/PatternLink.cc | 51 ++++++++++++++++++++++++++-- opencog/atoms/pattern/PatternLink.h | 1 + 2 files changed, 50 insertions(+), 2 deletions(-) diff --git a/opencog/atoms/pattern/PatternLink.cc b/opencog/atoms/pattern/PatternLink.cc index f25e6980a9..e4e882fddf 100644 --- a/opencog/atoms/pattern/PatternLink.cc +++ b/opencog/atoms/pattern/PatternLink.cc @@ -384,6 +384,18 @@ bool PatternLink::record_literal(const PatternTermPtr& clause, bool reverse) const Handle& h = clause->getHandle(); Type typ = h->get_type(); + if (not reverse and EVALUATION_LINK == typ) + { + OC_ASSERT(0 < h->get_arity(), + "Don't now what to do with empty EvlauationLinks"); + if (PREDICATE_FORMULA_LINK != h->getOutgoingAtom(0)->get_type()) + { + pin_term(clause); + _pat.pmandatory.push_back(clause); + return true; + } + } + // Pull clauses out of a PresentLink if ((not reverse and PRESENT_LINK == typ) or (reverse and ABSENT_LINK == typ)) @@ -508,7 +520,7 @@ bool PatternLink::record_literal(const PatternTermPtr& clause, bool reverse) /// evaluatable, and will look at the TV on the EvaluationLink. /// * Other EvalutationLink styles (e.g. with GPN or DPN or Lambda /// as the predicate) are evaluatable, and cannot be treated as -/// Present. +/// Present. xxxxxxxxxxxx /// bool PatternLink::unbundle_clauses_rec(const PatternTermPtr& term, const TypeSet& connectives, @@ -774,6 +786,39 @@ bool PatternLink::is_virtual(const Handle& clause) /* ================================================================= */ +bool PatternLink::add_unaries(const PatternTermPtr& ptm) +{ + const Handle& h = ptm->getHandle(); + +// printf("duuude enter unaries for %s\n", h->to_string().c_str()); + Type t = h->get_type(); + if (CHOICE_LINK == t) return false; + + if (EVALUATION_LINK == t) + { + OC_ASSERT(0 < h->get_arity(), + "Don't now what to do with empty EvlauationLinks"); + + // PredicateFormulas cannot be found in the AtomSpace. + if (PREDICATE_FORMULA_LINK == h->getOutgoingAtom(0)->get_type()) + return false; + + size_t nfree = num_unquoted_unscoped_in_tree(h, _variables.varset); + if (1 < nfree) return false; + +printf("duuude add unary for %s\n", h->to_string().c_str()); + _pat.pmandatory.push_back(ptm); + return true; + } + + bool added = false; + for (const PatternTermPtr& sub: ptm->getOutgoingSet()) + { + added = added or add_unaries(sub); + } + return added; +} + /// Add dummy clauses for patterns that would otherwise not have any /// non-evaluatable clauses. One example of such is /// @@ -810,6 +855,8 @@ bool PatternLink::is_virtual(const Handle& clause) /// void PatternLink::add_dummies(const PatternTermPtr& ptm) { + if (add_unaries(ptm)) return; + const Handle& h = ptm->getHandle(); Type t = h->get_type(); @@ -890,7 +937,7 @@ void PatternLink::check_satisfiability(const HandleSet& vars, // (Concept "OK")) // and we could add `(Variable "X") (Variable "Y")` with the // `add_dummies()` method above. But if we did, it would be - // an info loop that blows out the stack, because X and Y + // an infinite loop that blows out the stack, because X and Y // are not constrained, and match everything, including the // temorary terms created during search... so we do NOT // `add_dummies()` this case. See issue #1420 for more. diff --git a/opencog/atoms/pattern/PatternLink.h b/opencog/atoms/pattern/PatternLink.h index b0a45e4f4c..50b0892a5e 100644 --- a/opencog/atoms/pattern/PatternLink.h +++ b/opencog/atoms/pattern/PatternLink.h @@ -134,6 +134,7 @@ class PatternLink : public PrenexLink void locate_cacheable(const PatternTermSeq& clauses); void add_dummies(const PatternTermPtr&); + bool add_unaries(const PatternTermPtr&); void make_connectivity_map(void); void make_map_recursive(const Handle&, const PatternTermPtr&); From 15a3538116b99e602105e219aad9735a50046e66 Mon Sep 17 00:00:00 2001 From: Linas Vepstas Date: Thu, 4 Nov 2021 16:11:54 -0500 Subject: [PATCH 02/10] iRestructure so that it actually works correctly. --- opencog/atoms/pattern/PatternLink.cc | 58 ++++++++++++++-------------- 1 file changed, 30 insertions(+), 28 deletions(-) diff --git a/opencog/atoms/pattern/PatternLink.cc b/opencog/atoms/pattern/PatternLink.cc index e4e882fddf..ba3345516b 100644 --- a/opencog/atoms/pattern/PatternLink.cc +++ b/opencog/atoms/pattern/PatternLink.cc @@ -384,18 +384,6 @@ bool PatternLink::record_literal(const PatternTermPtr& clause, bool reverse) const Handle& h = clause->getHandle(); Type typ = h->get_type(); - if (not reverse and EVALUATION_LINK == typ) - { - OC_ASSERT(0 < h->get_arity(), - "Don't now what to do with empty EvlauationLinks"); - if (PREDICATE_FORMULA_LINK != h->getOutgoingAtom(0)->get_type()) - { - pin_term(clause); - _pat.pmandatory.push_back(clause); - return true; - } - } - // Pull clauses out of a PresentLink if ((not reverse and PRESENT_LINK == typ) or (reverse and ABSENT_LINK == typ)) @@ -508,19 +496,13 @@ bool PatternLink::record_literal(const PatternTermPtr& clause, bool reverse) /// literal, groundable clauses. `record_literal` does this. /// /// If there weren't any literal Present, Absent or Choice Links, (i.e. -/// if `record_literal` didn't spot anything) then take some guesses. -/// This guessing is slightly convoluted, but seems to make sense. -/// So: +/// if `record_literal` didn't spot anything) then some later stages +/// will attempt to fish out any literal-like terms hidden inside of +/// of evaluatable clauses. +/// +/// What we do here is: /// * If a clause is not evaluatable, then assume `Present` was intended. -/// * If it is evaluatable, then assume some later stage will evaluate it. -/// * If it is a variable, then assume something else will ground it, and -/// that some later stage will evaluate it. -/// * If it is an EvalutationLink-PredicateNode combination, then demand -/// that it be Present. A later stage will *also* treat this as -/// evaluatable, and will look at the TV on the EvaluationLink. -/// * Other EvalutationLink styles (e.g. with GPN or DPN or Lambda -/// as the predicate) are evaluatable, and cannot be treated as -/// Present. xxxxxxxxxxxx +/// * If it is antyhing else, assume some later stage will evaluate it. /// bool PatternLink::unbundle_clauses_rec(const PatternTermPtr& term, const TypeSet& connectives, @@ -786,13 +768,31 @@ bool PatternLink::is_virtual(const Handle& clause) /* ================================================================= */ +/// * If it is an EvalutationLink-PredicateNode combination, then demand +/// that it be Present. A later stage will *also* treat this as +/// evaluatable, and will look at the TV on the EvaluationLink. +/// * Other EvalutationLink styles (e.g. with GPN or DPN or Lambda +/// as the predicate) are evaluatable, and cannot be treated as +/// Present. xxxxxxxxxxxx bool PatternLink::add_unaries(const PatternTermPtr& ptm) { const Handle& h = ptm->getHandle(); -// printf("duuude enter unaries for %s\n", h->to_string().c_str()); Type t = h->get_type(); - if (CHOICE_LINK == t) return false; + if (CHOICE_LINK == t or ALWAYS_LINK == t) return false; + if (ABSENT_LINK == t or PRESENT_LINK == t) return false; + if (SEQUENTIAL_AND_LINK == t or SEQUENTIAL_OR_LINK == t) return false; + + PatternTermPtr parnt = ptm->getParent(); + while (parnt) + { + const Handle& ph = parnt->getHandle(); + if (nullptr == ph) break; + Type pt = ph->get_type(); + if (SEQUENTIAL_AND_LINK == pt or SEQUENTIAL_OR_LINK == pt) return false; + parnt = parnt->getParent(); + } +printf("duuude enter unaries for %s\n", h->to_string().c_str()); if (EVALUATION_LINK == t) { @@ -855,8 +855,6 @@ printf("duuude add unary for %s\n", h->to_string().c_str()); /// void PatternLink::add_dummies(const PatternTermPtr& ptm) { - if (add_unaries(ptm)) return; - const Handle& h = ptm->getHandle(); Type t = h->get_type(); @@ -1112,6 +1110,10 @@ void PatternLink::make_term_tree_recursive(const PatternTermPtr& root, } } + // If the evaluatables have literal-ish subterms, add those. + if (add_unaries(ptm)) return; + + // If the above failed, then try adding dummy variables. add_dummies(ptm); return; } From 6637442b572b5b13757432eb4094e6663f343099 Mon Sep 17 00:00:00 2001 From: Linas Vepstas Date: Thu, 4 Nov 2021 16:44:53 -0500 Subject: [PATCH 03/10] Ongoing restructuring --- opencog/atoms/pattern/PatternLink.cc | 45 ++++++++++++++++++++-------- 1 file changed, 32 insertions(+), 13 deletions(-) diff --git a/opencog/atoms/pattern/PatternLink.cc b/opencog/atoms/pattern/PatternLink.cc index ba3345516b..20eb104598 100644 --- a/opencog/atoms/pattern/PatternLink.cc +++ b/opencog/atoms/pattern/PatternLink.cc @@ -768,16 +768,28 @@ bool PatternLink::is_virtual(const Handle& clause) /* ================================================================= */ -/// * If it is an EvalutationLink-PredicateNode combination, then demand -/// that it be Present. A later stage will *also* treat this as -/// evaluatable, and will look at the TV on the EvaluationLink. -/// * Other EvalutationLink styles (e.g. with GPN or DPN or Lambda -/// as the predicate) are evaluatable, and cannot be treated as -/// Present. xxxxxxxxxxxx +/// Add implicit fixed terms. The pattern might consist only of +/// evaluatable clauses. Unless we find some constant Atom inside of +/// it, we risk having to perform a very large search trying to find +/// something that makes the evaluatable clauses true. Thus, it is +/// highly advantageous to find some constant term on which to start +/// the search. That's what we try to do here. +/// +/// If there is an EvaluationLink somewhere inside the evaluatable, +/// it might provide a good starting point. So we loop, looking for +/// those. +/// bool PatternLink::add_unaries(const PatternTermPtr& ptm) { const Handle& h = ptm->getHandle(); + // Ignore literals inside of Choice and Always; these should have + // been handled earlier. Ditto for Present, Absent. + // The Sequentials are weird from a search perspective: they appear + // (should only appear) in SatisfactionLinks, and have thier own + // distinct logic to them. So if we're inside of one of these, + // we drop out. Probably should refactor the code so that we are + // not even called for any of these cases. Type t = h->get_type(); if (CHOICE_LINK == t or ALWAYS_LINK == t) return false; if (ABSENT_LINK == t or PRESENT_LINK == t) return false; @@ -792,24 +804,31 @@ bool PatternLink::add_unaries(const PatternTermPtr& ptm) if (SEQUENTIAL_AND_LINK == pt or SEQUENTIAL_OR_LINK == pt) return false; parnt = parnt->getParent(); } -printf("duuude enter unaries for %s\n", h->to_string().c_str()); if (EVALUATION_LINK == t) { OC_ASSERT(0 < h->get_arity(), "Don't now what to do with empty EvlauationLinks"); - // PredicateFormulas cannot be found in the AtomSpace. - if (PREDICATE_FORMULA_LINK == h->getOutgoingAtom(0)->get_type()) - return false; + if (PREDICATE_NODE == h->getOutgoingAtom(0)->get_type()) + { + _pat.pmandatory.push_back(ptm); + return true; + } - size_t nfree = num_unquoted_unscoped_in_tree(h, _variables.varset); - if (1 < nfree) return false; + // EvaluationLinks with evaluatable predicates cannot + // be found in the AtomSpace. But perhaps thier arguments + // might be. So fall through and look at those. + } -printf("duuude add unary for %s\n", h->to_string().c_str()); +#if 0 +// wtf + if (not nameserver().isA(t, EVALUATABLE_LINK)) + { _pat.pmandatory.push_back(ptm); return true; } +#endif bool added = false; for (const PatternTermPtr& sub: ptm->getOutgoingSet()) From fa589f947ed943a018f6aa607b9d95320aad6976 Mon Sep 17 00:00:00 2001 From: Linas Vepstas Date: Thu, 4 Nov 2021 17:27:55 -0500 Subject: [PATCH 04/10] Finally a working version. --- opencog/atoms/pattern/PatternLink.cc | 22 +++++++++++++++------- 1 file changed, 15 insertions(+), 7 deletions(-) diff --git a/opencog/atoms/pattern/PatternLink.cc b/opencog/atoms/pattern/PatternLink.cc index 20eb104598..bb2ac5802d 100644 --- a/opencog/atoms/pattern/PatternLink.cc +++ b/opencog/atoms/pattern/PatternLink.cc @@ -775,9 +775,12 @@ bool PatternLink::is_virtual(const Handle& clause) /// highly advantageous to find some constant term on which to start /// the search. That's what we try to do here. /// -/// If there is an EvaluationLink somewhere inside the evaluatable, -/// it might provide a good starting point. So we loop, looking for -/// those. +/// This recursively explores the term, attempting to find "ordinary" +/// links that have exactly one variable in them, and at least one +/// other atom. Since this other atom appears inside a non-evaluatable +/// "ordinary", it must necessarily appear in the AtomSpace. Thus, this +/// kind of "ordinary" link counts as a fixed term. (A link is +/// "ordinary" if it's not evaluatable and not a function.) /// bool PatternLink::add_unaries(const PatternTermPtr& ptm) { @@ -821,14 +824,19 @@ bool PatternLink::add_unaries(const PatternTermPtr& ptm) // might be. So fall through and look at those. } -#if 0 -// wtf - if (not nameserver().isA(t, EVALUATABLE_LINK)) + // Try to add any kind of "ordinary" link that contains exactly + // one variable in it, and at least one non-variable term in it. + // (thus, an arity of 2 or more.) It's "ordinary" if it is not + // evaluatable or executable. + if (not nameserver().isA(t, NODE) and + 1 < h->get_arity() and + not nameserver().isA(t, EVALUATABLE_LINK) and + not nameserver().isA(t, FREE_LINK) and + 1 == num_unquoted_unscoped_in_tree(h, _variables.varset)) { _pat.pmandatory.push_back(ptm); return true; } -#endif bool added = false; for (const PatternTermPtr& sub: ptm->getOutgoingSet()) From e6763ece4a13f2cc7e89485381d171b2aa316299 Mon Sep 17 00:00:00 2001 From: Linas Vepstas Date: Thu, 4 Nov 2021 17:38:32 -0500 Subject: [PATCH 05/10] Extend the unit test a bit more --- tests/query/or-link-test.scm | 22 ++++++++++++++++++++-- 1 file changed, 20 insertions(+), 2 deletions(-) diff --git a/tests/query/or-link-test.scm b/tests/query/or-link-test.scm index 2fdbc3b348..2b9b609142 100644 --- a/tests/query/or-link-test.scm +++ b/tests/query/or-link-test.scm @@ -25,8 +25,26 @@ (IsTrue (Evaluation (Predicate "cold") (Variable "someone"))))))) -; (cog-execute! qr4) (test-assert "thirsty or cold" - (equal? (cog-execute! qr4) (Set (ConceptNode "you") (ConceptNode "me")))) + (equal? (cog-execute! qr4) (Set (Concept "you") (Concept "me")))) + +; ------------ +(define qr5 + (Get (TypedVariable (Variable "someone") (Type 'Concept)) + (Or + (Present (State (Variable "someone") (Concept "thirsty"))) + (IsTrue (Evaluation (Predicate "cold") (Variable "someone"))) + (IsTrue (Evaluation (Predicate "tired") (Variable "someone")))))) + +(test-assert "thirsty or cold but not tired" + (equal? (cog-execute! qr5) (Set (Concept "you") (Concept "me")))) + +; ------------ +; Add the stv to force it to be strictly true. +(Evaluation (stv 1 1) (Predicate "tired") (Concept "her")) + +(test-assert "thirsty or cold or tired" + (equal? (cog-execute! qr5) + (Set (Concept "you") (Concept "me") (Concept "her")))) (test-end tname) From 4710f23ead3297ab65e97e638f609b104c546621 Mon Sep 17 00:00:00 2001 From: Linas Vepstas Date: Thu, 4 Nov 2021 17:40:26 -0500 Subject: [PATCH 06/10] Check more cases. --- tests/query/or-link-test.scm | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-) diff --git a/tests/query/or-link-test.scm b/tests/query/or-link-test.scm index 2b9b609142..34a5c21ab1 100644 --- a/tests/query/or-link-test.scm +++ b/tests/query/or-link-test.scm @@ -29,7 +29,19 @@ (equal? (cog-execute! qr4) (Set (Concept "you") (Concept "me")))) ; ------------ +; Same as above, but with implcit PresentLink (define qr5 + (Get (TypedVariable (Variable "someone") (Type 'Concept)) + (Or + (Present (State (Variable "someone") (Concept "thirsty"))) + (And + (IsTrue (Evaluation (Predicate "cold") (Variable "someone"))))))) + +(test-assert "thirsty or cold" + (equal? (cog-execute! qr5) (Set (Concept "you") (Concept "me")))) + +; ------------ +(define qr6 (Get (TypedVariable (Variable "someone") (Type 'Concept)) (Or (Present (State (Variable "someone") (Concept "thirsty"))) @@ -37,14 +49,14 @@ (IsTrue (Evaluation (Predicate "tired") (Variable "someone")))))) (test-assert "thirsty or cold but not tired" - (equal? (cog-execute! qr5) (Set (Concept "you") (Concept "me")))) + (equal? (cog-execute! qr6) (Set (Concept "you") (Concept "me")))) ; ------------ ; Add the stv to force it to be strictly true. (Evaluation (stv 1 1) (Predicate "tired") (Concept "her")) (test-assert "thirsty or cold or tired" - (equal? (cog-execute! qr5) + (equal? (cog-execute! qr6) (Set (Concept "you") (Concept "me") (Concept "her")))) (test-end tname) From b5d19847fd3700823f978d92212324edd9869b96 Mon Sep 17 00:00:00 2001 From: Linas Vepstas Date: Thu, 4 Nov 2021 18:02:23 -0500 Subject: [PATCH 07/10] Deduplicate multiple terms --- opencog/atoms/pattern/PatternLink.cc | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/opencog/atoms/pattern/PatternLink.cc b/opencog/atoms/pattern/PatternLink.cc index bb2ac5802d..af546cde46 100644 --- a/opencog/atoms/pattern/PatternLink.cc +++ b/opencog/atoms/pattern/PatternLink.cc @@ -815,7 +815,9 @@ bool PatternLink::add_unaries(const PatternTermPtr& ptm) if (PREDICATE_NODE == h->getOutgoingAtom(0)->get_type()) { - _pat.pmandatory.push_back(ptm); + // deduplicate on the fly. + if (not ptm->contained_in(_pat.pmandatory)) + _pat.pmandatory.push_back(ptm); return true; } @@ -834,7 +836,9 @@ bool PatternLink::add_unaries(const PatternTermPtr& ptm) not nameserver().isA(t, FREE_LINK) and 1 == num_unquoted_unscoped_in_tree(h, _variables.varset)) { - _pat.pmandatory.push_back(ptm); + // deduplicate on the fly. + if (not ptm->contained_in(_pat.pmandatory)) + _pat.pmandatory.push_back(ptm); return true; } From cb91a49e6c058a4fe18111f8a8a13acefaa26c64 Mon Sep 17 00:00:00 2001 From: Linas Vepstas Date: Thu, 4 Nov 2021 18:12:10 -0500 Subject: [PATCH 08/10] Test IdenticalLink as well as EqualLink --- tests/query/PredicateFormulaUTest.cxxtest | 26 +++++++++++++ tests/query/predicate-formula.scm | 45 +++++++++++++++++++++++ 2 files changed, 71 insertions(+) diff --git a/tests/query/PredicateFormulaUTest.cxxtest b/tests/query/PredicateFormulaUTest.cxxtest index 90260e96ab..91f6eb987f 100644 --- a/tests/query/PredicateFormulaUTest.cxxtest +++ b/tests/query/PredicateFormulaUTest.cxxtest @@ -102,6 +102,14 @@ void PredicateFormulaUTest::do_predicate_test(HandleSeq ans) hget = LinkValueCast(get)->to_handle_seq(); TSM_ASSERT_EQUALS("Expect list", hget, ans); + // ------------------ + get = eval->eval_v("(cog-execute! qe2i)"); + printf("Got qe2i: %s\n", get->to_string().c_str()); + + TS_ASSERT(nameserver().isA(get->get_type(), LINK_VALUE)); + hget = LinkValueCast(get)->to_handle_seq(); + TSM_ASSERT_EQUALS("Expect list", hget, ans); + // ------------------ get = eval->eval_v("(cog-execute! qe3)"); printf("Got qe3: %s\n", get->to_string().c_str()); @@ -110,6 +118,14 @@ void PredicateFormulaUTest::do_predicate_test(HandleSeq ans) hget = LinkValueCast(get)->to_handle_seq(); TSM_ASSERT_EQUALS("Expect list", hget, ans); + // ------------------ + get = eval->eval_v("(cog-execute! qe3i)"); + printf("Got qe3i: %s\n", get->to_string().c_str()); + + TS_ASSERT(nameserver().isA(get->get_type(), LINK_VALUE)); + hget = LinkValueCast(get)->to_handle_seq(); + TSM_ASSERT_EQUALS("Expect list", hget, ans); + // ------------------ get = eval->eval_v("(cog-execute! qe4)"); printf("Got qe4: %s\n", get->to_string().c_str()); @@ -166,6 +182,16 @@ void PredicateFormulaUTest::test_accept(void) TSM_ASSERT_EQUALS("Expect one answer", hget.size(), 1); TSM_ASSERT_EQUALS("Expect list", hget[0], ans); + // ------------------ + ValuePtr geti = eval->eval_v("(cog-execute! qi-basic)"); + printf("Got basic-i: %s\n", geti->to_string().c_str()); + + TS_ASSERT(nameserver().isA(geti->get_type(), LINK_VALUE)); + LinkValuePtr lgeti(LinkValueCast(geti)); + HandleSeq hgeti(lgeti->to_handle_seq()); + TSM_ASSERT_EQUALS("Expect one answer", hgeti.size(), 1); + TSM_ASSERT_EQUALS("Expect list", hgeti[0], ans); + // Run the rest of the test. do_predicate_test({ans}); diff --git a/tests/query/predicate-formula.scm b/tests/query/predicate-formula.scm index 59a3afd73f..12c948ee94 100644 --- a/tests/query/predicate-formula.scm +++ b/tests/query/predicate-formula.scm @@ -24,6 +24,16 @@ (List (Variable "N") (Concept "name1")))) (Variable "Y"))) +; Same as above but with IdenticalLink +(define qi-basic (Query + (And + (Member + (Evaluation (Predicate "has_name") (Variable "Y")) + (Concept "node2")) + (Identical (Variable "Y") + (List (Variable "N") (Concept "name1")))) + (Variable "Y"))) + (define qe1 (Query (And (Member @@ -67,6 +77,28 @@ ; (cog-execute! qe2) +; Same as above, but with IdenticalLink +(define qe2i (Query + (And + (Member + (Evaluation (Predicate "has_name") (Variable "Y")) + (Concept "node2")) + (Identical (Variable "Y") + (List (Variable "N") (Concept "name1"))) + (Evaluation + (PredicateFormula + (Minus (Number 1) + (Times + (StrengthOf (Variable "$X")) + (StrengthOf (Variable "$Y")))) + (Times + (ConfidenceOf (Variable "$X")) + (ConfidenceOf (Variable "$Y")))) + (Variable "Y"))) + (Variable "Y"))) + +; (cog-execute! qe2i) + (define qe3 (Query (And (Member @@ -79,6 +111,19 @@ (Variable "Y"))) (Variable "Y"))) +; Same as above, but with Identical +(define qe3i (Query + (And + (Member + (Evaluation (Predicate "has_name") (Variable "Y")) + (Concept "node2")) + (Identical (Variable "Y") + (List (Variable "N") (Concept "name1"))) + (Evaluation + (DefinedPredicate "pred1") + (Variable "Y"))) + (Variable "Y"))) + ; The definition needed for the above. (DefineLink (DefinedPredicate "pred1") From f8c1b1f2001ff4504c261602fa0ff4e6e33580d8 Mon Sep 17 00:00:00 2001 From: Linas Vepstas Date: Thu, 4 Nov 2021 18:21:03 -0500 Subject: [PATCH 09/10] Extend the unit test some more ... --- tests/query/or-link-test.scm | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/tests/query/or-link-test.scm b/tests/query/or-link-test.scm index 34a5c21ab1..800cee33f4 100644 --- a/tests/query/or-link-test.scm +++ b/tests/query/or-link-test.scm @@ -1,6 +1,6 @@ ; ; or-link-test.scm -- Verify that OrLink produces sums during search. -; +; Reflects the discussion in issue opencog/atomspace#2644 (use-modules (opencog) (opencog exec)) (use-modules (opencog test-runner)) @@ -13,9 +13,21 @@ ; will fail with the default TruthValue of (stv 1 0) (true but not ; confident). (State (Concept "you") (Concept "thirsty")) +(State (Concept "me") (Concept "hungry")) (Evaluation (stv 1 1) (Predicate "cold") (Concept "me")) (Evaluation (Predicate "tired") (Concept "her")) +(define qr2 + (Get (TypedVariable (Variable "someone") (Type 'Concept)) + (Or + (Present (State (Variable "someone") (Concept "hungry"))) + (Present (State (Variable "someone") (Concept "thirsty")))))) + +(test-assert "hungry or thirsty" + (equal? (cog-execute! qr2) (Set (Concept "you") (Concept "me")))) + +; ------------ +; As above, but with EvaulationLink (define qr4 (Get (TypedVariable (Variable "someone") (Type 'Concept)) (Or From f51389909a99ce769d307aa1f58dd812bdf334da Mon Sep 17 00:00:00 2001 From: Linas Vepstas Date: Thu, 4 Nov 2021 20:48:36 -0500 Subject: [PATCH 10/10] Nasty hack to pass the ForwadChainerUTest This is not really acceptable, so the next pull req attempts to remove it. --- opencog/atoms/pattern/PatternLink.cc | 17 ++++++++++++++--- 1 file changed, 14 insertions(+), 3 deletions(-) diff --git a/opencog/atoms/pattern/PatternLink.cc b/opencog/atoms/pattern/PatternLink.cc index af546cde46..718017c5e6 100644 --- a/opencog/atoms/pattern/PatternLink.cc +++ b/opencog/atoms/pattern/PatternLink.cc @@ -1083,7 +1083,7 @@ void PatternLink::make_term_tree_recursive(const PatternTermPtr& root, } } - // Recurse down to the tips. ... after the evaluatable markup below. + // Recurse down to the tips. ... after the evaluatable markup above. if (h->is_link()) { // Remove constants from PresentLink, as they are pointless. @@ -1128,10 +1128,21 @@ void PatternLink::make_term_tree_recursive(const PatternTermPtr& root, if ((parent->getHandle() == nullptr or parent->hasEvaluatable()) and not ptm->isQuoted() and can_evaluate(h)) { + + // Yuck. Ugly, ugly hack to pass the FowardChainerUTest. + // Need to remove this ASAP. + bool already_have_fixed = false; + for (const PatternTermPtr& man : _pat.pmandatory) + if (not man->hasAnyEvaluatable()) + { + already_have_fixed = true; + break; + } + // If its an AndLink, make sure that all of the children are // evaluatable. The problem is .. users insert AndLinks into // random places... - if (AND_LINK == t) + if (not already_have_fixed and AND_LINK == t) { for (const PatternTermPtr& ptc : ptm->getOutgoingSet()) if (ptc->isQuoted() or not can_eval_or_present(ptc->getHandle())) @@ -1142,7 +1153,7 @@ void PatternLink::make_term_tree_recursive(const PatternTermPtr& root, } // If the evaluatables have literal-ish subterms, add those. - if (add_unaries(ptm)) return; + if (not already_have_fixed and add_unaries(ptm)) return; // If the above failed, then try adding dummy variables. add_dummies(ptm);