From b87ea5784934c56290379ad2a4ee9082c7fc355b Mon Sep 17 00:00:00 2001 From: Linas Vepstas Date: Wed, 25 Dec 2019 21:15:55 -0600 Subject: [PATCH 1/9] Split out the search loop from the setup loop. --- opencog/query/InitiateSearchCB.cc | 21 ++++++++++++++------- opencog/query/InitiateSearchCB.h | 11 +++++++---- 2 files changed, 21 insertions(+), 11 deletions(-) diff --git a/opencog/query/InitiateSearchCB.cc b/opencog/query/InitiateSearchCB.cc index e8f4df8a90..e4491d4186 100644 --- a/opencog/query/InitiateSearchCB.cc +++ b/opencog/query/InitiateSearchCB.cc @@ -630,7 +630,7 @@ bool InitiateSearchCB::link_type_search(PatternMatchEngine *pme) * variables, then you probably don't want to use this method, either; * you should create something more clever. */ -bool InitiateSearchCB::variable_search(PatternMatchEngine *pme) +bool InitiateSearchCB::setup_variable_search(void) { const HandleSeq& clauses = _pattern->mandatory; @@ -788,19 +788,26 @@ bool InitiateSearchCB::variable_search(PatternMatchEngine *pme) } } - HandleSeq handle_set; if (ptypes.empty()) - _as->get_handles_by_type(handle_set, ATOM, true); + _as->get_handles_by_type(_search_set, ATOM, true); else for (Type ptype : ptypes) - _as->get_handles_by_type(handle_set, ptype); + _as->get_handles_by_type(_search_set, ptype); - DO_LOG({LAZY_LOG_FINE << "Atomspace reported " << handle_set.size() << " atoms";}) + return true; +} + +bool InitiateSearchCB::variable_search(PatternMatchEngine *pme) +{ + if (not setup_variable_search()) return false; + + DO_LOG({LAZY_LOG_FINE << "Search-set size: " + << _search_set.size() << " atoms";}) #ifdef DEBUG - size_t i = 0, hsz = handle_set.size(); + size_t i = 0, hsz = _search_set.size(); #endif - for (const Handle& h : handle_set) + for (const Handle& h : _search_set) { DO_LOG({LAZY_LOG_FINE << "zzzzzzzzzzz variable_search zzzzzzzzzzz\n" << "Loop candidate (" << ++i << "/" << hsz << "):\n" diff --git a/opencog/query/InitiateSearchCB.h b/opencog/query/InitiateSearchCB.h index ff3f1cb740..86a8757a30 100644 --- a/opencog/query/InitiateSearchCB.h +++ b/opencog/query/InitiateSearchCB.h @@ -71,6 +71,7 @@ class InitiateSearchCB : public virtual PatternMatchCallback Handle _root; Handle _starter_term; + HandleSeq _search_set; struct Choice { @@ -90,11 +91,13 @@ class InitiateSearchCB : public virtual PatternMatchCallback virtual void find_rarest(const Handle&, Handle&, size_t&, Quotation quotation=Quotation()); + bool setup_variable_search(void); + bool _search_fail; - virtual bool neighbor_search(PatternMatchEngine *); - virtual bool link_type_search(PatternMatchEngine *); - virtual bool variable_search(PatternMatchEngine *); - virtual bool no_search(PatternMatchEngine *); + bool neighbor_search(PatternMatchEngine *); + bool link_type_search(PatternMatchEngine *); + bool variable_search(PatternMatchEngine *); + bool no_search(PatternMatchEngine *); AtomSpace *_as; }; From 1c0a3201df61fb94ee47e986d248966e8a27e4af Mon Sep 17 00:00:00 2001 From: Linas Vepstas Date: Wed, 25 Dec 2019 21:23:51 -0600 Subject: [PATCH 2/9] Make the search loop generic --- opencog/query/InitiateSearchCB.cc | 15 ++++++++------- opencog/query/InitiateSearchCB.h | 2 +- 2 files changed, 9 insertions(+), 8 deletions(-) diff --git a/opencog/query/InitiateSearchCB.cc b/opencog/query/InitiateSearchCB.cc index e4491d4186..4d3f15dfa1 100644 --- a/opencog/query/InitiateSearchCB.cc +++ b/opencog/query/InitiateSearchCB.cc @@ -513,7 +513,9 @@ bool InitiateSearchCB::initiate_search(PatternMatchEngine *pme) // method. DO_LOG({logger().fine("Cannot use link-type search, use variable-type search");}) _search_fail = false; - found = variable_search(pme); + bool setup = setup_variable_search(); + if (setup) + found = search_loop(pme, "zzzzzzzzzzz variable_search zzzzzzzzzzz"); return found; } @@ -797,10 +799,9 @@ bool InitiateSearchCB::setup_variable_search(void) return true; } -bool InitiateSearchCB::variable_search(PatternMatchEngine *pme) +bool InitiateSearchCB::search_loop(PatternMatchEngine *pme, + const std::string dbg_banner) { - if (not setup_variable_search()) return false; - DO_LOG({LAZY_LOG_FINE << "Search-set size: " << _search_set.size() << " atoms";}) @@ -809,9 +810,9 @@ bool InitiateSearchCB::variable_search(PatternMatchEngine *pme) #endif for (const Handle& h : _search_set) { - DO_LOG({LAZY_LOG_FINE << "zzzzzzzzzzz variable_search zzzzzzzzzzz\n" - << "Loop candidate (" << ++i << "/" << hsz << "):\n" - << h->to_string();}) + DO_LOG({LAZY_LOG_FINE << dbg_banner + << "\nLoop candidate (" << ++i << "/" << hsz << "):\n" + << h->to_string();}) bool found = pme->explore_neighborhood(_root, _starter_term, h); if (found) return true; } diff --git a/opencog/query/InitiateSearchCB.h b/opencog/query/InitiateSearchCB.h index 86a8757a30..74ce5f082c 100644 --- a/opencog/query/InitiateSearchCB.h +++ b/opencog/query/InitiateSearchCB.h @@ -96,9 +96,9 @@ class InitiateSearchCB : public virtual PatternMatchCallback bool _search_fail; bool neighbor_search(PatternMatchEngine *); bool link_type_search(PatternMatchEngine *); - bool variable_search(PatternMatchEngine *); bool no_search(PatternMatchEngine *); + bool search_loop(PatternMatchEngine *, const std::string); AtomSpace *_as; }; From a76724109e616d81c8186f4ea8cee9654c30240b Mon Sep 17 00:00:00 2001 From: Linas Vepstas Date: Wed, 25 Dec 2019 21:30:20 -0600 Subject: [PATCH 3/9] Refactor the link-type search loop too. --- opencog/query/InitiateSearchCB.cc | 24 ++++++------------------ opencog/query/InitiateSearchCB.h | 1 + 2 files changed, 7 insertions(+), 18 deletions(-) diff --git a/opencog/query/InitiateSearchCB.cc b/opencog/query/InitiateSearchCB.cc index 4d3f15dfa1..dbdfd38eaf 100644 --- a/opencog/query/InitiateSearchCB.cc +++ b/opencog/query/InitiateSearchCB.cc @@ -501,7 +501,8 @@ bool InitiateSearchCB::initiate_search(PatternMatchEngine *pme) // types that occur in the atomspace. DO_LOG({logger().fine("Cannot use no-var search, use link-type search");}) _search_fail = false; - found = link_type_search(pme); + if (setup_link_type_search()) + found = search_loop(pme, "yyyyyyyyyy link_type_search yyyyyyyyyy"); if (found) return true; if (not _search_fail) return false; @@ -513,8 +514,7 @@ bool InitiateSearchCB::initiate_search(PatternMatchEngine *pme) // method. DO_LOG({logger().fine("Cannot use link-type search, use variable-type search");}) _search_fail = false; - bool setup = setup_variable_search(); - if (setup) + if (setup_variable_search()) found = search_loop(pme, "zzzzzzzzzzz variable_search zzzzzzzzzzz"); return found; } @@ -561,7 +561,7 @@ void InitiateSearchCB::find_rarest(const Handle& clause, * type which has the smallest number of atoms of that type in the * AtomSpace. */ -bool InitiateSearchCB::link_type_search(PatternMatchEngine *pme) +bool InitiateSearchCB::setup_link_type_search() { const HandleSeq& clauses = _pattern->mandatory; @@ -603,20 +603,8 @@ bool InitiateSearchCB::link_type_search(PatternMatchEngine *pme) Type ptype = _starter_term->get_type(); HandleSeq handle_set; - _as->get_handles_by_type(handle_set, ptype); - -#ifdef DEBUG - size_t i = 0, hsz = handle_set.size(); -#endif - for (const Handle& h : handle_set) - { - DO_LOG({LAZY_LOG_FINE << "yyyyyyyyyy link_type_search yyyyyyyyyy\n" - << "Loop candidate (" << ++i << "/" << hsz << "):\n" - << h->to_string();}) - bool found = pme->explore_neighborhood(_root, _starter_term, h); - if (found) return true; - } - return false; + _as->get_handles_by_type(_search_set, ptype); + return true; } /* ======================================================== */ diff --git a/opencog/query/InitiateSearchCB.h b/opencog/query/InitiateSearchCB.h index 74ce5f082c..f9403f847d 100644 --- a/opencog/query/InitiateSearchCB.h +++ b/opencog/query/InitiateSearchCB.h @@ -91,6 +91,7 @@ class InitiateSearchCB : public virtual PatternMatchCallback virtual void find_rarest(const Handle&, Handle&, size_t&, Quotation quotation=Quotation()); + bool setup_link_type_search(void); bool setup_variable_search(void); bool _search_fail; From 576adf8f0af58f69f1ad308fae3aabf2848618dd Mon Sep 17 00:00:00 2001 From: Linas Vepstas Date: Wed, 25 Dec 2019 21:40:05 -0600 Subject: [PATCH 4/9] refactor the neighbor-search loop. --- opencog/query/InitiateSearchCB.cc | 21 ++++++++------------- opencog/query/InitiateSearchCB.h | 3 +-- 2 files changed, 9 insertions(+), 15 deletions(-) diff --git a/opencog/query/InitiateSearchCB.cc b/opencog/query/InitiateSearchCB.cc index dbdfd38eaf..53d0213270 100644 --- a/opencog/query/InitiateSearchCB.cc +++ b/opencog/query/InitiateSearchCB.cc @@ -280,7 +280,7 @@ Handle InitiateSearchCB::find_thinnest(const HandleSeq& clauses, * if the search was not performed, due to a failure to find a sutiable * starting point. */ -bool InitiateSearchCB::neighbor_search(PatternMatchEngine *pme) +bool InitiateSearchCB::neighbor_search(PatternMatchEngine* pme) { // If there are no non-constant clauses, abort; will use // no_search() instead. @@ -365,18 +365,13 @@ bool InitiateSearchCB::neighbor_search(PatternMatchEngine *pme) // get_incoming_set(), so that, e.g. it gets sorted by attentional // focus in the AttentionalFocusCB class... IncomingSet iset = get_incoming_set(best_start); - size_t sz = iset.size(); - for (size_t i = 0; i < sz; i++) - { - Handle h(iset[i]); - DO_LOG({LAZY_LOG_FINE << "xxxxxxxxxx neighbor_search xxxxxxxxxx\n" - << "Loop candidate (" << i+1 << "/" << sz << "):\n" - << h->to_string();}) - bool found = pme->explore_neighborhood(_root, _starter_term, h); - - // Terminate search if satisfied. - if (found) return true; - } + _search_set.clear(); + for (const auto& lptr: iset) + _search_set.emplace_back(HandleCast(lptr)); + + bool found = search_loop(pme, "xxxxxxxxxx neighbor_search xxxxxxxxxx"); + // Terminate search if satisfied. + if (found) return true; } // If we are here, we have searched the entire neighborhood, and diff --git a/opencog/query/InitiateSearchCB.h b/opencog/query/InitiateSearchCB.h index f9403f847d..856115da50 100644 --- a/opencog/query/InitiateSearchCB.h +++ b/opencog/query/InitiateSearchCB.h @@ -91,12 +91,11 @@ class InitiateSearchCB : public virtual PatternMatchCallback virtual void find_rarest(const Handle&, Handle&, size_t&, Quotation quotation=Quotation()); + bool neighbor_search(PatternMatchEngine *); bool setup_link_type_search(void); bool setup_variable_search(void); bool _search_fail; - bool neighbor_search(PatternMatchEngine *); - bool link_type_search(PatternMatchEngine *); bool no_search(PatternMatchEngine *); bool search_loop(PatternMatchEngine *, const std::string); From 9f9e4be65b47a1a72c141d6cb434735b69cd55f5 Mon Sep 17 00:00:00 2001 From: Linas Vepstas Date: Wed, 25 Dec 2019 21:47:32 -0600 Subject: [PATCH 5/9] Start ripping out the _search_fail flag --- opencog/query/InitiateSearchCB.cc | 21 ++++++--------------- 1 file changed, 6 insertions(+), 15 deletions(-) diff --git a/opencog/query/InitiateSearchCB.cc b/opencog/query/InitiateSearchCB.cc index 53d0213270..aced123b1e 100644 --- a/opencog/query/InitiateSearchCB.cc +++ b/opencog/query/InitiateSearchCB.cc @@ -495,11 +495,8 @@ bool InitiateSearchCB::initiate_search(PatternMatchEngine *pme) // the LoopUTest), and so instead, we search based on the link // types that occur in the atomspace. DO_LOG({logger().fine("Cannot use no-var search, use link-type search");}) - _search_fail = false; if (setup_link_type_search()) - found = search_loop(pme, "yyyyyyyyyy link_type_search yyyyyyyyyy"); - if (found) return true; - if (not _search_fail) return false; + return search_loop(pme, "yyyyyyyyyy link_type_search yyyyyyyyyy"); // The URE Reasoning case: if we found nothing, then there are no // links! Ergo, every clause must be a lone variable, all by @@ -508,10 +505,10 @@ bool InitiateSearchCB::initiate_search(PatternMatchEngine *pme) // and that's all. We deal with this in the variable_search() // method. DO_LOG({logger().fine("Cannot use link-type search, use variable-type search");}) - _search_fail = false; if (setup_variable_search()) - found = search_loop(pme, "zzzzzzzzzzz variable_search zzzzzzzzzzz"); - return found; + return search_loop(pme, "zzzzzzzzzzz variable_search zzzzzzzzzzz"); + + return false; } /* ======================================================== */ @@ -584,10 +581,7 @@ bool InitiateSearchCB::setup_link_type_search() // and that's all. We deal with this in the variable_search() // method. if (nullptr == _root) - { - _search_fail = true; return false; - } DO_LOG({LAZY_LOG_FINE << "Start clause is: " << std::endl << _root->to_string();}) @@ -740,13 +734,10 @@ bool InitiateSearchCB::setup_variable_search(void) #endif } + // There are no clauses. This is kind-of weird, but it can happen + // if all clauses are optional. if (0 == clauses.size()) - { - // This is kind-of weird, but it can happen if all clauses - // are optional. - _search_fail = true; return false; - } // The pattern body might be of the form // (And (Present (Variable "$x")) (Evaluation ...)) From dd397da4b1c993cf0ea5558bd37f205e9b9c5566 Mon Sep 17 00:00:00 2001 From: Linas Vepstas Date: Wed, 25 Dec 2019 21:52:58 -0600 Subject: [PATCH 6/9] Refactor the no-search branch. --- opencog/query/InitiateSearchCB.cc | 17 ++++------------- opencog/query/InitiateSearchCB.h | 5 ++--- 2 files changed, 6 insertions(+), 16 deletions(-) diff --git a/opencog/query/InitiateSearchCB.cc b/opencog/query/InitiateSearchCB.cc index aced123b1e..3c201560e0 100644 --- a/opencog/query/InitiateSearchCB.cc +++ b/opencog/query/InitiateSearchCB.cc @@ -484,10 +484,8 @@ bool InitiateSearchCB::initiate_search(PatternMatchEngine *pme) // we want to quickly rule out this case before moving to more // complex searches, below. DO_LOG({logger().fine("Cannot use node-neighbor search, use no-var search");}) - _search_fail = false; - found = no_search(pme); - if (found) return true; - if (not _search_fail) return false; + if (setup_no_search()) + return pme->explore_constant_evaluatables(_pattern->mandatory); // If we are here, then we could not find a clause at which to // start, which can happen if the clauses consist entirely of @@ -806,16 +804,9 @@ bool InitiateSearchCB::search_loop(PatternMatchEngine *pme, * inefficient to use the pattern matcher for this, so if you want it * to run fast, re-work the below to not use the PME. */ -bool InitiateSearchCB::no_search(PatternMatchEngine *pme) +bool InitiateSearchCB::setup_no_search(void) { - if (0 < _variables->varset.size()) - { - _search_fail = true; - return false; - } - - // Evaluate all evaluatable clauses - return pme->explore_constant_evaluatables(_pattern->mandatory); + return (0 == _variables->varset.size()); } /* ======================================================== */ diff --git a/opencog/query/InitiateSearchCB.h b/opencog/query/InitiateSearchCB.h index 856115da50..136dbe66c7 100644 --- a/opencog/query/InitiateSearchCB.h +++ b/opencog/query/InitiateSearchCB.h @@ -91,13 +91,12 @@ class InitiateSearchCB : public virtual PatternMatchCallback virtual void find_rarest(const Handle&, Handle&, size_t&, Quotation quotation=Quotation()); + bool _search_fail; bool neighbor_search(PatternMatchEngine *); + bool setup_no_search(void); bool setup_link_type_search(void); bool setup_variable_search(void); - bool _search_fail; - bool no_search(PatternMatchEngine *); - bool search_loop(PatternMatchEngine *, const std::string); AtomSpace *_as; }; From 568dc28e80565e871f8ada1549157e5115ab5c92 Mon Sep 17 00:00:00 2001 From: Linas Vepstas Date: Wed, 25 Dec 2019 22:21:56 -0600 Subject: [PATCH 7/9] Refactor neighbor search .. and clause setup. A bit of domino-effect here; the full refactor required getting rid of the int index into the clauses. --- opencog/query/InitiateSearchCB.cc | 56 +++++++++++++------------------ opencog/query/InitiateSearchCB.h | 10 +++--- 2 files changed, 28 insertions(+), 38 deletions(-) diff --git a/opencog/query/InitiateSearchCB.cc b/opencog/query/InitiateSearchCB.cc index 3c201560e0..1094f4eb70 100644 --- a/opencog/query/InitiateSearchCB.cc +++ b/opencog/query/InitiateSearchCB.cc @@ -55,17 +55,14 @@ InitiateSearchCB::InitiateSearchCB(AtomSpace* as) : _root = Handle::UNDEFINED; _starter_term = Handle::UNDEFINED; - _curr_clause = 0; + _curr_clause = Handle::UNDEFINED; _choices.clear(); - _search_fail = false; _as = as; } void InitiateSearchCB::set_pattern(const Variables& vars, const Pattern& pat) { - _search_fail = false; - _variables = &vars; _pattern = &pat; _dynamic = &pat.evaluatable_terms; @@ -224,23 +221,21 @@ InitiateSearchCB::find_starter_recursive(const Handle& h, size_t& depth, Handle InitiateSearchCB::find_thinnest(const HandleSeq& clauses, const HandleSet& evl, Handle& starter_term, - size_t& bestclause) + Handle& bestclause) { size_t thinnest = SIZE_MAX; size_t deepest = 0; - bestclause = 0; + bestclause = Handle::UNDEFINED; Handle best_start(Handle::UNDEFINED); starter_term = Handle::UNDEFINED; _choices.clear(); - size_t nc = clauses.size(); - for (size_t i=0; i < nc; i++) + for (const Handle& h: clauses) { // Cannot start with an evaluatable clause! - if (0 < evl.count(clauses[i])) continue; + if (0 < evl.count(h)) continue; - _curr_clause = i; - Handle h(clauses[i]); + _curr_clause = h; size_t depth = 0; size_t width = SIZE_MAX; Handle term(Handle::UNDEFINED); @@ -251,7 +246,7 @@ Handle InitiateSearchCB::find_thinnest(const HandleSeq& clauses, { thinnest = width; deepest = depth; - bestclause = i; + bestclause = h; best_start = start; starter_term = term; } @@ -276,18 +271,15 @@ Handle InitiateSearchCB::find_thinnest(const HandleSeq& clauses, * * The return value is true if a grounding was found, else it returns * false. That is, this return value works just like all the other - * satisfiability callbacks. The flag '_search_fail' is set to true - * if the search was not performed, due to a failure to find a sutiable - * starting point. + * satisfiability callbacks. Returns false, if no sutiable starting + * points were found. */ -bool InitiateSearchCB::neighbor_search(PatternMatchEngine* pme) +bool InitiateSearchCB::setup_neighbor_search(void) { // If there are no non-constant clauses, abort; will use // no_search() instead. - if (_pattern->mandatory.empty() and _pattern->optionals.empty()) { - _search_fail = true; + if (_pattern->mandatory.empty() and _pattern->optionals.empty()) return false; - } // Sometimes, the number of mandatory clauses can be zero... // or they might all be evaluatable. In this case, its OK to @@ -319,7 +311,7 @@ bool InitiateSearchCB::neighbor_search(PatternMatchEngine* pme) // Note also: the user is allowed to specify patterns that have // no constants in them at all. In this case, the search is // performed by looping over all links of the given types. - size_t bestclause; + Handle bestclause; Handle best_start = find_thinnest(clauses, _pattern->evaluatable_holders, _starter_term, bestclause); @@ -329,12 +321,9 @@ bool InitiateSearchCB::neighbor_search(PatternMatchEngine* pme) // Somewhat unusual, but it can happen. For this, we need // some other, alternative search strategy. if (nullptr == best_start and 0 == _choices.size()) - { - _search_fail = true; return false; - } - // If only a single choice, fake it for the loop below. + // If only a single choice, fake it for the choice_loop. if (0 == _choices.size()) { Choice ch; @@ -347,14 +336,18 @@ bool InitiateSearchCB::neighbor_search(PatternMatchEngine* pme) { // TODO -- weed out duplicates! } + return true; +} +bool InitiateSearchCB::choice_loop(PatternMatchEngine* pme, + const std::string dbg_banner) +{ for (const Choice& ch : _choices) { - bestclause = ch.clause; - best_start = ch.best_start; + _root = ch.clause; _starter_term = ch.start_term; + Handle best_start = ch.best_start; - _root = clauses[bestclause]; DO_LOG({LAZY_LOG_FINE << "Search start node: " << best_start->to_string();}) DO_LOG({LAZY_LOG_FINE << "Start term is: " << (_starter_term == (Atom*) nullptr ? @@ -369,7 +362,7 @@ bool InitiateSearchCB::neighbor_search(PatternMatchEngine* pme) for (const auto& lptr: iset) _search_set.emplace_back(HandleCast(lptr)); - bool found = search_loop(pme, "xxxxxxxxxx neighbor_search xxxxxxxxxx"); + bool found = search_loop(pme, dbg_banner); // Terminate search if satisfied. if (found) return true; } @@ -473,10 +466,8 @@ bool InitiateSearchCB::initiate_search(PatternMatchEngine *pme) jit_analyze(pme); DO_LOG({logger().fine("Attempt to use node-neighbor search");}) - _search_fail = false; - bool found = neighbor_search(pme); - if (found) return true; - if (not _search_fail) return false; + if (setup_neighbor_search()) + return choice_loop(pme, "xxxxxxxxxx neighbor_search xxxxxxxxxx"); // If we are here, then we could not find a clause at which to // start, which can happen if the clauses hold no variables, and @@ -920,7 +911,6 @@ std::string InitiateSearchCB::to_string(const std::string& indent) const << oc_to_string(ch.start_term, indent_ppp) << std::endl; } } - ss << indent << "_search_fail = " << _search_fail; return ss.str(); } diff --git a/opencog/query/InitiateSearchCB.h b/opencog/query/InitiateSearchCB.h index 136dbe66c7..bebfe77d6e 100644 --- a/opencog/query/InitiateSearchCB.h +++ b/opencog/query/InitiateSearchCB.h @@ -75,11 +75,11 @@ class InitiateSearchCB : public virtual PatternMatchCallback struct Choice { - size_t clause; + Handle clause; Handle best_start; Handle start_term; }; - size_t _curr_clause; + Handle _curr_clause; std::vector _choices; virtual Handle find_starter(const Handle&, size_t&, Handle&, size_t&); @@ -87,16 +87,16 @@ class InitiateSearchCB : public virtual PatternMatchCallback size_t&); virtual Handle find_thinnest(const HandleSeq&, const HandleSet&, - Handle&, size_t&); + Handle&, Handle&); virtual void find_rarest(const Handle&, Handle&, size_t&, Quotation quotation=Quotation()); - bool _search_fail; - bool neighbor_search(PatternMatchEngine *); + bool setup_neighbor_search(void); bool setup_no_search(void); bool setup_link_type_search(void); bool setup_variable_search(void); + bool choice_loop(PatternMatchEngine *, const std::string); bool search_loop(PatternMatchEngine *, const std::string); AtomSpace *_as; }; From 7b7ce90d627a8a832e9da43414f6be0736f318c8 Mon Sep 17 00:00:00 2001 From: Linas Vepstas Date: Wed, 25 Dec 2019 22:41:41 -0600 Subject: [PATCH 8/9] Revise documentation to match the new code. --- opencog/query/InitiateSearchCB.cc | 96 +++++++++++++++++++------------ 1 file changed, 59 insertions(+), 37 deletions(-) diff --git a/opencog/query/InitiateSearchCB.cc b/opencog/query/InitiateSearchCB.cc index 1094f4eb70..06821a85a3 100644 --- a/opencog/query/InitiateSearchCB.cc +++ b/opencog/query/InitiateSearchCB.cc @@ -257,10 +257,11 @@ Handle InitiateSearchCB::find_thinnest(const HandleSeq& clauses, /* ======================================================== */ /** - * Given a set of clauses, find a neighborhood to search, and perform - * the search. A `neighborhood` is defined as all of the atoms that - * can be reached from a given (non-variable) atom, by following either - * it's incoming or its outgoing set. + * Given a set of clauses, create a list of starting points for a + * search. This set of starting points is called a `neghborhood`; + * it is defined as all of the atoms that can be reached from a + * given (non-variable) atom, by following either it's incoming or + * its outgoing set. * * A neighborhood search is guaranteed to find all possible groundings * for the set of clauses. The reason for this is that, given a @@ -269,10 +270,19 @@ Handle InitiateSearchCB::find_thinnest(const HandleSeq& clauses, * grounding must be contained in that neighborhood. It is sufficient * to walk that graph until a suitable grounding is encountered. * - * The return value is true if a grounding was found, else it returns - * false. That is, this return value works just like all the other - * satisfiability callbacks. Returns false, if no sutiable starting - * points were found. + * The starting points or `neighborhood` is chosen so that the initial + * search space is as small as possible (thus, hopefully, resulting in + * a fast search.) + * + * Due to the ChoiceLink, there may be multiple such neighborhoods. + * Each neighborhood is placed into a `struct Choice`, and the search + * loop will look at the choices. + * + * This method returns true if suitable starting points were found, + * else it returns false. There are rare cases where this will fail + * to find starting points: if, for example, all clauses are evaluatable, + * or if all clauses consist only of VariableNodes or GlobNodes, so + * that there's nowhere to start the search. */ bool InitiateSearchCB::setup_neighbor_search(void) { @@ -339,6 +349,8 @@ bool InitiateSearchCB::setup_neighbor_search(void) return true; } +/* ======================================================== */ + bool InitiateSearchCB::choice_loop(PatternMatchEngine* pme, const std::string dbg_banner) { @@ -372,6 +384,29 @@ bool InitiateSearchCB::choice_loop(PatternMatchEngine* pme, return false; } +/* ======================================================== */ + +bool InitiateSearchCB::search_loop(PatternMatchEngine *pme, + const std::string dbg_banner) +{ + DO_LOG({LAZY_LOG_FINE << "Search-set size: " + << _search_set.size() << " atoms";}) + +#ifdef DEBUG + size_t i = 0, hsz = _search_set.size(); +#endif + for (const Handle& h : _search_set) + { + DO_LOG({LAZY_LOG_FINE << dbg_banner + << "\nLoop candidate (" << ++i << "/" << hsz << "):\n" + << h->to_string();}) + bool found = pme->explore_neighborhood(_root, _starter_term, h); + if (found) return true; + } + + return false; +} + /* ======================================================== */ /** * Search for solutions/groundings over all of the AtomSpace, using @@ -537,10 +572,15 @@ void InitiateSearchCB::find_rarest(const Handle& clause, /* ======================================================== */ /** - * Initiate a search by looping over all Links of the same type as one - * of the links in the set of clauses. This attempts to pick the link - * type which has the smallest number of atoms of that type in the + * Set up a list of starting points to search by making a list of all + * Links of the same type as one of the links in the set of clauses. + * This attempts to minimize the search space by picking the link type + * which has the smallest number of atoms of that type in the * AtomSpace. + * + * The list of starting points is placed into `_search_set` and this + * method returns true. If it cannot find any starting points, this + * returns false. */ bool InitiateSearchCB::setup_link_type_search() { @@ -580,23 +620,26 @@ bool InitiateSearchCB::setup_link_type_search() // Get type of the rarest link Type ptype = _starter_term->get_type(); - HandleSeq handle_set; _as->get_handles_by_type(_search_set, ptype); return true; } /* ======================================================== */ /** - * Initiate a search by looping over all atoms of the allowed - * variable types (as set with the set_type_restrictions() method). - * This assumes that the varset contains the variables to be searched - * over, and that the type restrictions are set up appropriately. + * Set up a list of search starting points consisting of all atoms of + * the allowed variable types (as set with the `set_type_restrictions()` + * method). This assumes that the varset contains the variables to be + * searched over, and that the type restrictions are set up appropriately. * * If the varset is empty, or if there are no variables, then the * entire atomspace will be searched. Depending on the pattern, * many, many duplicates might be reported. If you are not using * variables, then you probably don't want to use this method, either; * you should create something more clever. + * + * The list of starting points is placed into `_search_set` and this + * method returns true. If it cannot find any starting points, this + * returns false. */ bool InitiateSearchCB::setup_variable_search(void) { @@ -762,27 +805,6 @@ bool InitiateSearchCB::setup_variable_search(void) return true; } -bool InitiateSearchCB::search_loop(PatternMatchEngine *pme, - const std::string dbg_banner) -{ - DO_LOG({LAZY_LOG_FINE << "Search-set size: " - << _search_set.size() << " atoms";}) - -#ifdef DEBUG - size_t i = 0, hsz = _search_set.size(); -#endif - for (const Handle& h : _search_set) - { - DO_LOG({LAZY_LOG_FINE << dbg_banner - << "\nLoop candidate (" << ++i << "/" << hsz << "):\n" - << h->to_string();}) - bool found = pme->explore_neighborhood(_root, _starter_term, h); - if (found) return true; - } - - return false; -} - /* ======================================================== */ /** * No search -- no variables, only constant, possibly evaluatable From 91f509c637dbaa74b4fda4a909d7be0f3af033f2 Mon Sep 17 00:00:00 2001 From: Linas Vepstas Date: Wed, 25 Dec 2019 22:50:28 -0600 Subject: [PATCH 9/9] Minimize the speard of the PME in this code-base. --- opencog/query/InitiateSearchCB.cc | 42 +++++++++++++++---------------- opencog/query/InitiateSearchCB.h | 2 +- 2 files changed, 22 insertions(+), 22 deletions(-) diff --git a/opencog/query/InitiateSearchCB.cc b/opencog/query/InitiateSearchCB.cc index 06821a85a3..bda2e0d017 100644 --- a/opencog/query/InitiateSearchCB.cc +++ b/opencog/query/InitiateSearchCB.cc @@ -440,55 +440,55 @@ bool InitiateSearchCB::search_loop(PatternMatchEngine *pme, * but this has no effect on the thoroughness of the search. The search * will proceed by exploring the entire incoming-set for this node. * - * This is ideal, when the node_match() callback accepts a match only + * This is ideal, when the `node_match()` callback accepts a match only * when the pattern and suggested nodes are identical (i.e. are - * exactly the same atom). If the node_match() callback is willing to - * accept a broader range of node matches, then other possible + * exactly the same atom). If the `node_match()` callback is willing + * to accept a broader range of node matches, then other possible * solutions might be missed. Just how to fix this depends sharpely - * on what node_match() is willing to accept as a match. + * on what `node_match()` is willing to accept as a match. * * Anyway, this seems like a very reasonable limitation: if you - * really want a lenient node_match(), then use variables instead. - * Don't overload node-match with something weird, and you should be - * OK. Otherwise, you'll have to implement your own initiate_search() - * callback. + * really want a lenient `node_match()`, then use variables instead. + * Don't overload `node_match` with something weird, and you should + * be OK. Otherwise, you'll have to implement your own + * `initiate_search()` callback. * * 2) If the clauses consist entirely of variables, i.e. if there is not * even one single non-variable node in the pattern, then a search is * driven by looking for all links that are of the same type as one * of the links in one of the clauses. * - * If the link_match() callback is willing to accept a broader range + * If the `link_match()` callback is willing to accept a broader range * of types, then this search method may fail to find some possible * patterns. * - * Lets start by noting that this situation is very rare: most - * patterns will not consist entirely if Links and VariableNodes. + * Let's start by noting that this situation is very rare: most + * patterns will not consist entirely of `Links` and `VariableNodes`. * Almost surely, most reasonable people will have at least one * non-variable node in the pattern. So the disucssion below almost * surely does not apply. * * But if you really want this, there are several possible remedies. - * One is to modify the link_type_search() callback to try each + * One is to modify the `link_type_search()` callback to try each * possible link type that is considered to be equivalent by - * link_match(). Another alternative is to just leave the - * link_match() callback alone, and use variables for links, instead. + * `link_match()`. Another alternative is to just leave the + * `link_match()` callback alone, and use variables for links, instead. * This is probably the best strategy, because then the fairly * standard reasoning can be used when thinking about the problem. - * Of course, you can always write your own initiate_search() callback. + * Of course, you can always write your own `initiate_search()` callback. * * If the constraint 1) can be met, (which is always the case for * "standard, canonical" searches, then the pattern match should be * quite rapid. Incoming sets tend to be small; in addition, the - * implemnentation here picks the smallest, "tinnest" incoming set to + * implemnentation here picks the smallest, "thinnest" incoming set to * explore. * - * The default implementation of node_match() and link_match() in this + * The default implementation of `node_match()` and `link_match()` in this * class does satisfy both 1) and 2), so this algo will work correctly, * if these two methods are not overloaded with more callbacks that are * lenient about matching. * - * If you overload node_match(), and do so in a way that breaks + * If you overload `node_match()`, and do so in a way that breaks * assumption 1), then you will scratch your head, thinking * "why did my search fail to find this obvious solution?" The answer * will be for you to create a new search algo, in a new class, that @@ -498,7 +498,8 @@ bool InitiateSearchCB::search_loop(PatternMatchEngine *pme, */ bool InitiateSearchCB::initiate_search(PatternMatchEngine *pme) { - jit_analyze(pme); + jit_analyze(); + pme->set_pattern(*_variables, *_pattern); DO_LOG({logger().fine("Attempt to use node-neighbor search");}) if (setup_neighbor_search()) @@ -828,7 +829,7 @@ bool InitiateSearchCB::setup_no_search(void) * earlier, because the definitions for them might not have been * present, or may have changed since the pattern was initially created. */ -void InitiateSearchCB::jit_analyze(PatternMatchEngine* pme) +void InitiateSearchCB::jit_analyze(void) { // If there are no definitions, there is nothing to do. if (0 == _pattern->defined_terms.size()) @@ -889,7 +890,6 @@ void InitiateSearchCB::jit_analyze(PatternMatchEngine* pme) _dynamic = &_pattern->evaluatable_terms; - pme->set_pattern(*_variables, *_pattern); set_pattern(*_variables, *_pattern); DO_LOG({logger().fine("JIT expanded!"); _pl->debug_log();}) diff --git a/opencog/query/InitiateSearchCB.h b/opencog/query/InitiateSearchCB.h index bebfe77d6e..c71154694f 100644 --- a/opencog/query/InitiateSearchCB.h +++ b/opencog/query/InitiateSearchCB.h @@ -67,7 +67,7 @@ class InitiateSearchCB : public virtual PatternMatchCallback const HandleSet* _dynamic; PatternLinkPtr _pl; - void jit_analyze(PatternMatchEngine *); + void jit_analyze(void); Handle _root; Handle _starter_term;