Support implicit present terms

opencog/atoms/pattern/PatternLink.cc

@@ -496,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.
+/// * If it is antyhing else, assume some later stage will evaluate it.
 ///
 bool PatternLink::unbundle_clauses_rec(const PatternTermPtr& term,
                                        const TypeSet& connectives,
@@ -774,6 +768,88 @@ bool PatternLink::is_virtual(const Handle& clause)
 
 /* ================================================================= */
 
+/// 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.
+///
+/// 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)
+{
+	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;
+	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();
+	}
+
+	if (EVALUATION_LINK == t)
+	{
+		OC_ASSERT(0 < h->get_arity(),
+			"Don't now what to do with empty EvlauationLinks");
+
+		if (PREDICATE_NODE == h->getOutgoingAtom(0)->get_type())
+		{
+			// deduplicate on the fly.
+			if (not ptm->contained_in(_pat.pmandatory))
+				_pat.pmandatory.push_back(ptm);
+			return true;
+		}
+
+		// EvaluationLinks with evaluatable predicates cannot
+		// be found in the AtomSpace. But perhaps thier arguments
+		// might be. So fall through and look at those.
+	}
+
+	// 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))
+	{
+		// deduplicate on the fly.
+		if (not ptm->contained_in(_pat.pmandatory))
+			_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
 ///
@@ -890,7 +966,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.
@@ -1007,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.
@@ -1052,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()))
@@ -1065,6 +1152,10 @@ void PatternLink::make_term_tree_recursive(const PatternTermPtr& root,
 				}
 		}
 
+		// If the evaluatables have literal-ish subterms, add those.
+		if (not already_have_fixed and add_unaries(ptm)) return;
+
+		// If the above failed, then try adding dummy variables.
 		add_dummies(ptm);
 		return;
 	}

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&);

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});
 

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
@@ -25,8 +37,38 @@
 				(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"))))
+
+; ------------
+; 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")))
+			(IsTrue (Evaluation (Predicate "cold") (Variable "someone")))
+			(IsTrue (Evaluation (Predicate "tired") (Variable "someone"))))))
+
+(test-assert "thirsty or cold but not tired"
+	(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! qr6)
+		(Set (Concept "you") (Concept "me") (Concept "her"))))
 
 (test-end tname)

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")