From 2189880f6e4cd470cf274b34045a9b76c82239c4 Mon Sep 17 00:00:00 2001 From: Linas Vepstas Date: Mon, 4 May 2020 13:11:11 -0500 Subject: [PATCH 01/12] Deliver search results into a (thread-safe) queue. --- opencog/atoms/pattern/QueryLink.cc | 14 ++++++++-- opencog/query/Implicator.cc | 33 ++++++++++++++---------- opencog/query/Implicator.h | 10 ++++--- tests/query/ExecutionOutputUTest.cxxtest | 4 +-- tests/query/imply.h | 11 ++++++-- 5 files changed, 49 insertions(+), 23 deletions(-) diff --git a/opencog/atoms/pattern/QueryLink.cc b/opencog/atoms/pattern/QueryLink.cc index fd2ce9b8a2..99fb896775 100644 --- a/opencog/atoms/pattern/QueryLink.cc +++ b/opencog/atoms/pattern/QueryLink.cc @@ -23,6 +23,7 @@ * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. */ +#include #include #include #include @@ -154,11 +155,20 @@ ValueSet QueryLink::do_execute(AtomSpace* as, bool silent) this->PatternLink::satisfy(impl); // If we got a non-empty answer, just return it. - if (0 < impl.get_result_set().size()) + QueueValue& qv(impl.get_result_queue()); + OC_ASSERT(qv.is_closed(), "Unexpected queue state!"); + std::queue vals(qv.wait_and_take_all()); + if (0 < vals.size()) { // The result_set contains a list of the grounded expressions. // (The order of the list has no significance, so it's really a set.) - return impl.get_result_set(); + ValueSet vset; + while (not vals.empty()) + { + vset.insert(vals.front()); + vals.pop(); + } + return vset; } // If we are here, then there were zero matches. diff --git a/opencog/query/Implicator.cc b/opencog/query/Implicator.cc index e19e5a7849..df3de05ca7 100644 --- a/opencog/query/Implicator.cc +++ b/opencog/query/Implicator.cc @@ -64,21 +64,26 @@ bool Implicator::grounding(const GroundingMap &var_soln, return (_result_set.size() >= max_results); } -void Implicator::insert_result(const ValuePtr& v) +void Implicator::insert_result(ValuePtr v) { - if (v and _result_set.end() == _result_set.find(v)) - { - // Insert atom into the atomspace immediately, so that - // it becomes visible in other threads. - if (v->is_atom()) - { - _result_set.insert(_as->add_atom(HandleCast(v))); - } - else - { - _result_set.insert(v); - } - } + if (nullptr == v) return; + if (_result_set.end() != _result_set.find(v)) return; + + // Insert atom into the atomspace immediately, so that + // it becomes visible in other threads. + if (v->is_atom()) + v = _as->add_atom(HandleCast(v)); + + if (_result_set.end() != _result_set.find(v)) return; + + _result_set.insert(v); + _result_queue.push (std::move(v)); +} + +bool Implicator::search_finished(bool done) +{ + _result_queue.close(); + return done; } /* ===================== END OF FILE ===================== */ diff --git a/opencog/query/Implicator.h b/opencog/query/Implicator.h index 090ab3da9e..0c796652e3 100644 --- a/opencog/query/Implicator.h +++ b/opencog/query/Implicator.h @@ -29,6 +29,7 @@ #include #include +#include #include @@ -58,7 +59,8 @@ class Implicator : AtomSpace* _as; ValueSet _result_set; - void insert_result(const ValuePtr&); + QueueValue _result_queue; + void insert_result(ValuePtr); public: Implicator(AtomSpace* as) : _as(as), inst(as), max_results(SIZE_MAX) {} @@ -69,8 +71,10 @@ class Implicator : virtual bool grounding(const GroundingMap &var_soln, const GroundingMap &term_soln); - virtual const ValueSet& get_result_set() const - { return _result_set; } + virtual bool search_finished(bool); + + virtual QueueValue& get_result_queue() + { return _result_queue; } }; }; // namespace opencog diff --git a/tests/query/ExecutionOutputUTest.cxxtest b/tests/query/ExecutionOutputUTest.cxxtest index 094a2c1ee6..9e2f902016 100644 --- a/tests/query/ExecutionOutputUTest.cxxtest +++ b/tests/query/ExecutionOutputUTest.cxxtest @@ -251,7 +251,7 @@ void ExecutionOutputUTest::test_varsub(void) situation->satisfy(simpl); // The result_list contains a list of the grounded expressions. - Handle res = HandleCast(*simpl.get_result_set().begin()); + Handle res = HandleCast(simpl.get_result_queue().value_pop()); printf("res is %s\n", res->to_short_string().c_str()); TSM_ASSERT_EQUALS("incorrect resolution", res, resolution); @@ -261,7 +261,7 @@ void ExecutionOutputUTest::test_varsub(void) predicament->satisfy(pimpl); // The result_list contains a list of the grounded expressions. - Handle del = HandleCast(*pimpl.get_result_set().begin()); + Handle del = HandleCast(pimpl.get_result_queue().value_pop()); printf("del is %s\n", del->to_short_string().c_str()); TSM_ASSERT_EQUALS("incorrect delivarance", del, deliverance); diff --git a/tests/query/imply.h b/tests/query/imply.h index 9699b70327..732ead8c82 100644 --- a/tests/query/imply.h +++ b/tests/query/imply.h @@ -1,4 +1,5 @@ +#include #include #include #include @@ -35,8 +36,14 @@ static inline Handle imply(AtomSpace* as, Handle hclauses, Handle himplicand) // The result_set contains a list of the grounded expressions. // Turn it into a true list, and return it. HandleSeq hlist; - for (const ValuePtr& v: impl.get_result_set()) - hlist.push_back(HandleCast(v)); + QueueValue& qv(impl.get_result_queue()); + OC_ASSERT(qv.is_closed(), "Unexpected queue state!"); + std::queue vals(qv.wait_and_take_all()); + while (not vals.empty()) + { + hlist.push_back(HandleCast(vals.front())); + vals.pop(); + } Handle gl = as->add_link(LIST_LINK, std::move(hlist)); return gl; } From eaad125b5f387dd1a0ae63df736dacd3165a8788 Mon Sep 17 00:00:00 2001 From: Linas Vepstas Date: Mon, 4 May 2020 15:21:22 -0500 Subject: [PATCH 02/12] Call the start and finish callbacks on virtual searches. --- opencog/query/Implicator.cc | 11 +++++++++++ opencog/query/Implicator.h | 3 ++- opencog/query/PatternLinkRuntime.cc | 6 +++++- tests/query/GreaterThanUTest.cxxtest | 1 - 4 files changed, 18 insertions(+), 3 deletions(-) diff --git a/opencog/query/Implicator.cc b/opencog/query/Implicator.cc index df3de05ca7..45f46b4ff3 100644 --- a/opencog/query/Implicator.cc +++ b/opencog/query/Implicator.cc @@ -28,6 +28,11 @@ using namespace opencog; +Implicator::Implicator(AtomSpace* as) + : _as(as), inst(as), max_results(SIZE_MAX) +{ +} + /** * This callback takes the reported grounding, runs it through the * instantiator, to create the implicand, and then records the result @@ -80,6 +85,12 @@ void Implicator::insert_result(ValuePtr v) _result_queue.push (std::move(v)); } +bool Implicator::start_search(void) +{ + _result_queue.open(); + return false; +} + bool Implicator::search_finished(bool done) { _result_queue.close(); diff --git a/opencog/query/Implicator.h b/opencog/query/Implicator.h index 0c796652e3..c04e72f310 100644 --- a/opencog/query/Implicator.h +++ b/opencog/query/Implicator.h @@ -63,7 +63,7 @@ class Implicator : void insert_result(ValuePtr); public: - Implicator(AtomSpace* as) : _as(as), inst(as), max_results(SIZE_MAX) {} + Implicator(AtomSpace*); Instantiator inst; HandleSeq implicand; size_t max_results; @@ -71,6 +71,7 @@ class Implicator : virtual bool grounding(const GroundingMap &var_soln, const GroundingMap &term_soln); + virtual bool start_search(void); virtual bool search_finished(bool); virtual QueueValue& get_result_queue() diff --git a/opencog/query/PatternLinkRuntime.cc b/opencog/query/PatternLinkRuntime.cc index 17bb013ade..915ee323ad 100644 --- a/opencog/query/PatternLinkRuntime.cc +++ b/opencog/query/PatternLinkRuntime.cc @@ -429,9 +429,13 @@ bool PatternLink::satisfy(PatternMatchCallback& pmcb) const GroundingMap empty_vg; GroundingMap empty_pg; pmcb.set_pattern(_variables, _pat); - return recursive_virtual(pmcb, _virtual, _pat.optionals, + bool done = pmcb.start_search(); + if (done) return done; + done = recursive_virtual(pmcb, _virtual, _pat.optionals, empty_vg, empty_pg, comp_var_gnds, comp_term_gnds); + done = pmcb.search_finished(done); + return done; } /* ===================== END OF FILE ===================== */ diff --git a/tests/query/GreaterThanUTest.cxxtest b/tests/query/GreaterThanUTest.cxxtest index ee736aede4..6be9984985 100644 --- a/tests/query/GreaterThanUTest.cxxtest +++ b/tests/query/GreaterThanUTest.cxxtest @@ -42,7 +42,6 @@ public: as = new AtomSpace(); eval = new SchemeEval(as); eval->eval("(add-to-load-path \"" PROJECT_SOURCE_DIR "\")"); - } ~GreaterThanUTest() From fd42f042b62479d6a4e08f22b1a8b249bf6a6523 Mon Sep 17 00:00:00 2001 From: Linas Vepstas Date: Mon, 4 May 2020 15:29:02 -0500 Subject: [PATCH 03/12] bug-fix usage of queues --- tests/query/ExecutionOutputUTest.cxxtest | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tests/query/ExecutionOutputUTest.cxxtest b/tests/query/ExecutionOutputUTest.cxxtest index 9e2f902016..37ec1318fb 100644 --- a/tests/query/ExecutionOutputUTest.cxxtest +++ b/tests/query/ExecutionOutputUTest.cxxtest @@ -251,6 +251,7 @@ void ExecutionOutputUTest::test_varsub(void) situation->satisfy(simpl); // The result_list contains a list of the grounded expressions. + simpl.get_result_queue().open(); Handle res = HandleCast(simpl.get_result_queue().value_pop()); printf("res is %s\n", res->to_short_string().c_str()); TSM_ASSERT_EQUALS("incorrect resolution", res, resolution); @@ -261,6 +262,7 @@ void ExecutionOutputUTest::test_varsub(void) predicament->satisfy(pimpl); // The result_list contains a list of the grounded expressions. + pimpl.get_result_queue().open(); Handle del = HandleCast(pimpl.get_result_queue().value_pop()); printf("del is %s\n", del->to_short_string().c_str()); TSM_ASSERT_EQUALS("incorrect delivarance", del, deliverance); From 35940308f080a84f518494d0de94b7e31bb8b04b Mon Sep 17 00:00:00 2001 From: Linas Vepstas Date: Mon, 4 May 2020 15:58:41 -0500 Subject: [PATCH 04/12] Need ability to pass around queue as independent object. --- opencog/atoms/pattern/QueryLink.cc | 6 +++--- opencog/atoms/value/QueueValue.h | 5 +++++ opencog/query/Implicator.cc | 7 ++++--- opencog/query/Implicator.h | 4 ++-- tests/query/ExecutionOutputUTest.cxxtest | 8 ++++---- tests/query/imply.h | 6 +++--- 6 files changed, 21 insertions(+), 15 deletions(-) diff --git a/opencog/atoms/pattern/QueryLink.cc b/opencog/atoms/pattern/QueryLink.cc index 99fb896775..705784939a 100644 --- a/opencog/atoms/pattern/QueryLink.cc +++ b/opencog/atoms/pattern/QueryLink.cc @@ -155,9 +155,9 @@ ValueSet QueryLink::do_execute(AtomSpace* as, bool silent) this->PatternLink::satisfy(impl); // If we got a non-empty answer, just return it. - QueueValue& qv(impl.get_result_queue()); - OC_ASSERT(qv.is_closed(), "Unexpected queue state!"); - std::queue vals(qv.wait_and_take_all()); + QueueValuePtr qv(impl.get_result_queue()); + OC_ASSERT(qv->is_closed(), "Unexpected queue state!"); + std::queue vals(qv->wait_and_take_all()); if (0 < vals.size()) { // The result_set contains a list of the grounded expressions. diff --git a/opencog/atoms/value/QueueValue.h b/opencog/atoms/value/QueueValue.h index da52b1ddc4..c01f1618e9 100644 --- a/opencog/atoms/value/QueueValue.h +++ b/opencog/atoms/value/QueueValue.h @@ -58,6 +58,11 @@ typedef std::shared_ptr QueueValuePtr; static inline QueueValuePtr QueueValueCast(ValuePtr& a) { return std::dynamic_pointer_cast(a); } +template +static inline std::shared_ptr createQueueValue(Type&&... args) { + return std::make_shared(std::forward(args)...); +} + /** @}*/ } // namespace opencog diff --git a/opencog/query/Implicator.cc b/opencog/query/Implicator.cc index 45f46b4ff3..29af2e78ca 100644 --- a/opencog/query/Implicator.cc +++ b/opencog/query/Implicator.cc @@ -31,6 +31,7 @@ using namespace opencog; Implicator::Implicator(AtomSpace* as) : _as(as), inst(as), max_results(SIZE_MAX) { + _result_queue = createQueueValue(); } /** @@ -82,18 +83,18 @@ void Implicator::insert_result(ValuePtr v) if (_result_set.end() != _result_set.find(v)) return; _result_set.insert(v); - _result_queue.push (std::move(v)); + _result_queue->push (std::move(v)); } bool Implicator::start_search(void) { - _result_queue.open(); + _result_queue->open(); return false; } bool Implicator::search_finished(bool done) { - _result_queue.close(); + _result_queue->close(); return done; } diff --git a/opencog/query/Implicator.h b/opencog/query/Implicator.h index c04e72f310..8510441f6c 100644 --- a/opencog/query/Implicator.h +++ b/opencog/query/Implicator.h @@ -59,7 +59,7 @@ class Implicator : AtomSpace* _as; ValueSet _result_set; - QueueValue _result_queue; + QueueValuePtr _result_queue; void insert_result(ValuePtr); public: @@ -74,7 +74,7 @@ class Implicator : virtual bool start_search(void); virtual bool search_finished(bool); - virtual QueueValue& get_result_queue() + virtual QueueValuePtr get_result_queue() { return _result_queue; } }; diff --git a/tests/query/ExecutionOutputUTest.cxxtest b/tests/query/ExecutionOutputUTest.cxxtest index 37ec1318fb..96c229df30 100644 --- a/tests/query/ExecutionOutputUTest.cxxtest +++ b/tests/query/ExecutionOutputUTest.cxxtest @@ -251,8 +251,8 @@ void ExecutionOutputUTest::test_varsub(void) situation->satisfy(simpl); // The result_list contains a list of the grounded expressions. - simpl.get_result_queue().open(); - Handle res = HandleCast(simpl.get_result_queue().value_pop()); + simpl.get_result_queue()->open(); + Handle res = HandleCast(simpl.get_result_queue()->value_pop()); printf("res is %s\n", res->to_short_string().c_str()); TSM_ASSERT_EQUALS("incorrect resolution", res, resolution); @@ -262,8 +262,8 @@ void ExecutionOutputUTest::test_varsub(void) predicament->satisfy(pimpl); // The result_list contains a list of the grounded expressions. - pimpl.get_result_queue().open(); - Handle del = HandleCast(pimpl.get_result_queue().value_pop()); + pimpl.get_result_queue()->open(); + Handle del = HandleCast(pimpl.get_result_queue()->value_pop()); printf("del is %s\n", del->to_short_string().c_str()); TSM_ASSERT_EQUALS("incorrect delivarance", del, deliverance); diff --git a/tests/query/imply.h b/tests/query/imply.h index 732ead8c82..942123339f 100644 --- a/tests/query/imply.h +++ b/tests/query/imply.h @@ -36,9 +36,9 @@ static inline Handle imply(AtomSpace* as, Handle hclauses, Handle himplicand) // The result_set contains a list of the grounded expressions. // Turn it into a true list, and return it. HandleSeq hlist; - QueueValue& qv(impl.get_result_queue()); - OC_ASSERT(qv.is_closed(), "Unexpected queue state!"); - std::queue vals(qv.wait_and_take_all()); + QueueValuePtr qv(impl.get_result_queue()); + OC_ASSERT(qv->is_closed(), "Unexpected queue state!"); + std::queue vals(qv->wait_and_take_all()); while (not vals.empty()) { hlist.push_back(HandleCast(vals.front())); From 3f23bf18bfa941fe9609ad5548c71e6325f7023e Mon Sep 17 00:00:00 2001 From: Linas Vepstas Date: Mon, 4 May 2020 16:04:11 -0500 Subject: [PATCH 05/12] All searches get a fresh new queue --- opencog/query/Implicator.cc | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/opencog/query/Implicator.cc b/opencog/query/Implicator.cc index 29af2e78ca..14cd3d779a 100644 --- a/opencog/query/Implicator.cc +++ b/opencog/query/Implicator.cc @@ -31,7 +31,6 @@ using namespace opencog; Implicator::Implicator(AtomSpace* as) : _as(as), inst(as), max_results(SIZE_MAX) { - _result_queue = createQueueValue(); } /** @@ -88,7 +87,10 @@ void Implicator::insert_result(ValuePtr v) bool Implicator::start_search(void) { - _result_queue->open(); + // *Every* search gets a brand new, fresh queue! + // This allows users to hang on to the old queue, holding + // previous results, if they need to. + _result_queue = createQueueValue(); return false; } From defcd1e10fc3efce2ddc6329ab67dc402890b46c Mon Sep 17 00:00:00 2001 From: Linas Vepstas Date: Mon, 4 May 2020 16:39:07 -0500 Subject: [PATCH 06/12] Use the QueuesValue for GetLink, too. --- opencog/atoms/pattern/GetLink.cc | 12 +++++++++++- opencog/query/Implicator.cc | 2 +- opencog/query/Satisfier.cc | 32 ++++++++++++++++++++++++++++++-- opencog/query/Satisfier.h | 17 ++++++++++++++--- 4 files changed, 56 insertions(+), 7 deletions(-) diff --git a/opencog/atoms/pattern/GetLink.cc b/opencog/atoms/pattern/GetLink.cc index 6be1c193c5..25fca0f044 100644 --- a/opencog/atoms/pattern/GetLink.cc +++ b/opencog/atoms/pattern/GetLink.cc @@ -21,6 +21,7 @@ * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. */ +#include #include #include #include @@ -55,7 +56,16 @@ HandleSet GetLink::do_execute(AtomSpace* as, bool silent) SatisfyingSet sater(as); this->satisfy(sater); - return sater._satisfying_set; + QueueValuePtr qv(sater.get_result_queue()); + OC_ASSERT(qv->is_closed(), "Unexpected queue state!"); + std::queue vals(qv->wait_and_take_all()); + HandleSet hset; + while (not vals.empty()) + { + hset.emplace(HandleCast(vals.front())); + vals.pop(); + } + return hset; } ValuePtr GetLink::execute(AtomSpace* as, bool silent) diff --git a/opencog/query/Implicator.cc b/opencog/query/Implicator.cc index 14cd3d779a..485985a645 100644 --- a/opencog/query/Implicator.cc +++ b/opencog/query/Implicator.cc @@ -82,7 +82,7 @@ void Implicator::insert_result(ValuePtr v) if (_result_set.end() != _result_set.find(v)) return; _result_set.insert(v); - _result_queue->push (std::move(v)); + _result_queue->push(std::move(v)); } bool Implicator::start_search(void) diff --git a/opencog/query/Satisfier.cc b/opencog/query/Satisfier.cc index 1299c90461..486dbc76a8 100644 --- a/opencog/query/Satisfier.cc +++ b/opencog/query/Satisfier.cc @@ -133,7 +133,14 @@ bool SatisfyingSet::grounding(const GroundingMap &var_soln, // std::map::at() can throw. Rethrow for easier deubugging. try { - _satisfying_set.emplace(var_soln.at(_varseq[0])); + // Insert atom into the atomspace immediately, so that + // it becomes visible in other threads. + Handle gnd(_as->add_atom(var_soln.at(_varseq[0]))); + if (_satisfying_set.end() == _satisfying_set.find(gnd)) + { + _satisfying_set.emplace(gnd); + _result_queue->push(std::move(gnd)); + } } catch (...) { @@ -163,10 +170,31 @@ bool SatisfyingSet::grounding(const GroundingMap &var_soln, vargnds.push_back(hv); } } - _satisfying_set.emplace(createLink(std::move(vargnds), LIST_LINK)); + Handle gnds(_as->add_atom(createLink(std::move(vargnds), LIST_LINK))); + + if (_satisfying_set.end() == _satisfying_set.find(gnds)) + { + _satisfying_set.emplace(gnds); + _result_queue->push(std::move(gnds)); + } // If we found as many as we want, then stop looking for more. return (_satisfying_set.size() >= max_results); } +bool SatisfyingSet::start_search(void) +{ + // *Every* search gets a brand new, fresh queue! + // This allows users to hang on to the old queue, holding + // previous results, if they need to. + _result_queue = createQueueValue(); + return false; +} + +bool SatisfyingSet::search_finished(bool done) +{ + _result_queue->close(); + return done; +} + /* ===================== END OF FILE ===================== */ diff --git a/opencog/query/Satisfier.h b/opencog/query/Satisfier.h index 44f5178963..bf7ebae777 100644 --- a/opencog/query/Satisfier.h +++ b/opencog/query/Satisfier.h @@ -27,6 +27,7 @@ #include #include +#include #include #include @@ -95,13 +96,17 @@ class SatisfyingSet : public virtual InitiateSearchCB, public virtual DefaultPatternMatchCB { + protected: + AtomSpace* _as; + HandleSeq _varseq; + HandleSet _satisfying_set; + QueueValuePtr _result_queue; + public: SatisfyingSet(AtomSpace* as) : InitiateSearchCB(as), DefaultPatternMatchCB(as), - max_results(SIZE_MAX) {} + _as(as), max_results(SIZE_MAX) {} - HandleSeq _varseq; - HandleSet _satisfying_set; size_t max_results; virtual void set_pattern(const Variables& vars, @@ -119,6 +124,12 @@ class SatisfyingSet : // groundings. virtual bool grounding(const GroundingMap &var_soln, const GroundingMap &term_soln); + + virtual bool start_search(void); + virtual bool search_finished(bool); + + virtual QueueValuePtr get_result_queue() + { return _result_queue; } }; }; // namespace opencog From f218fceddc6261d6d4a05bf149e192ff08f21eb5 Mon Sep 17 00:00:00 2001 From: Linas Vepstas Date: Mon, 4 May 2020 16:54:09 -0500 Subject: [PATCH 07/12] Use the QueueValue in all of the search links. --- opencog/atoms/pattern/BindLink.cc | 25 +++++++++++++++---------- opencog/atoms/pattern/GetLink.cc | 26 ++++++++++++-------------- opencog/atoms/pattern/GetLink.h | 2 +- opencog/atoms/pattern/QueryLink.cc | 30 +++++++++--------------------- opencog/atoms/pattern/QueryLink.h | 3 ++- 5 files changed, 39 insertions(+), 47 deletions(-) diff --git a/opencog/atoms/pattern/BindLink.cc b/opencog/atoms/pattern/BindLink.cc index 8019053894..fa5df16978 100644 --- a/opencog/atoms/pattern/BindLink.cc +++ b/opencog/atoms/pattern/BindLink.cc @@ -8,8 +8,7 @@ * This program is free software; you can redistribute it and/or modify * it under the terms of the GNU Affero General Public License v3 as * published by the Free Software Foundation and including the - * exceptions - * at http://opencog.org/wiki/Licenses + * exceptions at http://opencog.org/wiki/Licenses * * This program is distributed in the hope that it will be useful, * but WITHOUT ANY WARRANTY; without even the implied warranty of @@ -17,12 +16,12 @@ * GNU General Public License for more details. * * You should have received a copy of the GNU Affero General Public - * License - * along with this program; if not, write to: + * License along with this program; if not, write to: * Free Software Foundation, Inc., * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. */ +#include #include #include #include @@ -66,14 +65,22 @@ BindLink::BindLink(const HandleSeq&& hseq, Type t) /** Wrap query results in a SetLink, place them in the AtomSpace. */ ValuePtr BindLink::execute(AtomSpace* as, bool silent) { - ValueSet rslt(do_execute(as, silent)); + HandleSeq rslt; + QueueValuePtr qv(do_execute(as, silent)); + OC_ASSERT(qv->is_closed(), "Unexpected queue state!"); + std::queue vals(qv->wait_and_take_all()); + while (not vals.empty()) + { + rslt.push_back(HandleCast(vals.front())); + vals.pop(); + } // If there is an anchor, then attach results to the anchor. // Otherwise, create a SetLink and return that. if (_variables._anchor and as) { - for (const ValuePtr& v: rslt) - as->add_link(MEMBER_LINK, HandleCast(v), _variables._anchor); + for (const Handle& h: rslt) + as->add_link(MEMBER_LINK, h, _variables._anchor); return _variables._anchor; } @@ -81,9 +88,7 @@ ValuePtr BindLink::execute(AtomSpace* as, bool silent) // The result_set contains a list of the grounded expressions. // (The order of the list has no significance, so it's really a set.) // Put the set into a SetLink, cache it, and return that. - HandleSeq hlist; - for (const ValuePtr& v: rslt) hlist.emplace_back(HandleCast(v)); - Handle rewr(createUnorderedLink(std::move(hlist), SET_LINK)); + Handle rewr(createUnorderedLink(std::move(rslt), SET_LINK)); #define PLACE_RESULTS_IN_ATOMSPACE #ifdef PLACE_RESULTS_IN_ATOMSPACE diff --git a/opencog/atoms/pattern/GetLink.cc b/opencog/atoms/pattern/GetLink.cc index 25fca0f044..d6c38ed104 100644 --- a/opencog/atoms/pattern/GetLink.cc +++ b/opencog/atoms/pattern/GetLink.cc @@ -6,8 +6,7 @@ * This program is free software; you can redistribute it and/or modify * it under the terms of the GNU Affero General Public License v3 as * published by the Free Software Foundation and including the - * exceptions - * at http://opencog.org/wiki/Licenses + * exceptions at http://opencog.org/wiki/Licenses * * This program is distributed in the hope that it will be useful, * but WITHOUT ANY WARRANTY; without even the implied warranty of @@ -15,8 +14,7 @@ * GNU General Public License for more details. * * You should have received a copy of the GNU Affero General Public - * License - * along with this program; if not, write to: + * License along with this program; if not, write to: * Free Software Foundation, Inc., * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. */ @@ -49,32 +47,32 @@ GetLink::GetLink(const HandleSeq&& hseq, Type t) /* ================================================================= */ -HandleSet GetLink::do_execute(AtomSpace* as, bool silent) +QueueValuePtr GetLink::do_execute(AtomSpace* as, bool silent) { if (nullptr == as) as = _atom_space; SatisfyingSet sater(as); this->satisfy(sater); - QueueValuePtr qv(sater.get_result_queue()); + return sater.get_result_queue(); +} + +ValuePtr GetLink::execute(AtomSpace* as, bool silent) +{ + HandleSeq hs; + QueueValuePtr qv(do_execute(as, silent)); OC_ASSERT(qv->is_closed(), "Unexpected queue state!"); std::queue vals(qv->wait_and_take_all()); - HandleSet hset; while (not vals.empty()) { - hset.emplace(HandleCast(vals.front())); + hs.push_back(HandleCast(vals.front())); vals.pop(); } - return hset; -} -ValuePtr GetLink::execute(AtomSpace* as, bool silent) -{ // If there is an anchor, then attach results to the anchor. // Otherwise, create a SetLink and return that. if (_variables._anchor and as) { - HandleSet hs(do_execute(as, silent)); for (const Handle& h : hs) as->add_link(MEMBER_LINK, h, _variables._anchor); @@ -82,7 +80,7 @@ ValuePtr GetLink::execute(AtomSpace* as, bool silent) } // Create the satisfying set, and cache it. - Handle satset(createUnorderedLink(do_execute(as, silent), SET_LINK)); + Handle satset(createUnorderedLink(std::move(hs), SET_LINK)); #define PLACE_RESULTS_IN_ATOMSPACE #ifdef PLACE_RESULTS_IN_ATOMSPACE diff --git a/opencog/atoms/pattern/GetLink.h b/opencog/atoms/pattern/GetLink.h index 3c3d7c1f1c..3bf58aa65b 100644 --- a/opencog/atoms/pattern/GetLink.h +++ b/opencog/atoms/pattern/GetLink.h @@ -33,7 +33,7 @@ class GetLink : public PatternLink { protected: void init(void); - virtual HandleSet do_execute(AtomSpace*, bool silent); + virtual QueueValuePtr do_execute(AtomSpace*, bool silent); public: GetLink(const HandleSeq&&, Type=GET_LINK); diff --git a/opencog/atoms/pattern/QueryLink.cc b/opencog/atoms/pattern/QueryLink.cc index 705784939a..d676c64021 100644 --- a/opencog/atoms/pattern/QueryLink.cc +++ b/opencog/atoms/pattern/QueryLink.cc @@ -130,7 +130,7 @@ void QueryLink::extract_variables(const HandleSeq& oset) * atoms that could be a ground are found in the atomspace, then they * will be reported. */ -ValueSet QueryLink::do_execute(AtomSpace* as, bool silent) +QueueValuePtr QueryLink::do_execute(AtomSpace* as, bool silent) { if (nullptr == as) as = _atom_space; @@ -157,19 +157,8 @@ ValueSet QueryLink::do_execute(AtomSpace* as, bool silent) // If we got a non-empty answer, just return it. QueueValuePtr qv(impl.get_result_queue()); OC_ASSERT(qv->is_closed(), "Unexpected queue state!"); - std::queue vals(qv->wait_and_take_all()); - if (0 < vals.size()) - { - // The result_set contains a list of the grounded expressions. - // (The order of the list has no significance, so it's really a set.) - ValueSet vset; - while (not vals.empty()) - { - vset.insert(vals.front()); - vals.pop(); - } - return vset; - } + if (0 < qv->size()) + return qv; // If we are here, then there were zero matches. // @@ -190,20 +179,19 @@ ValueSet QueryLink::do_execute(AtomSpace* as, bool silent) if (0 == pat.mandatory.size() and 0 < pat.optionals.size() and not intu->optionals_present()) { - ValueSet result; + qv->open(); for (const Handle& himp: impl.implicand) - result.insert(impl.inst.execute(himp, true)); - return result; + qv->push(std::move(impl.inst.execute(himp, true))); + qv->close(); + return qv; } - return ValueSet(); + return qv; } ValuePtr QueryLink::execute(AtomSpace* as, bool silent) { - // The result_set contains a list of the grounded expressions. - // (The order of the list has no significance, so it's really a set.) - return createLinkValue(do_execute(as, silent)); + return do_execute(as, silent); } DEFINE_LINK_FACTORY(QueryLink, QUERY_LINK) diff --git a/opencog/atoms/pattern/QueryLink.h b/opencog/atoms/pattern/QueryLink.h index 929ff05af2..346914db27 100644 --- a/opencog/atoms/pattern/QueryLink.h +++ b/opencog/atoms/pattern/QueryLink.h @@ -23,6 +23,7 @@ #define _OPENCOG_QUERY_LINK_H #include +#include namespace opencog { @@ -42,7 +43,7 @@ class QueryLink : public PatternLink // will initialize the rewrite term _implicand. void extract_variables(const HandleSeq& oset); - virtual ValueSet do_execute(AtomSpace*, bool silent); + virtual QueueValuePtr do_execute(AtomSpace*, bool silent); public: QueryLink(const HandleSeq&&, Type=QUERY_LINK); From 083e203cc9519a56450673d21bacc39f7e442371 Mon Sep 17 00:00:00 2001 From: Linas Vepstas Date: Mon, 4 May 2020 17:12:23 -0500 Subject: [PATCH 08/12] Update unit test to match the new API --- tests/query/QueryUTest.cxxtest | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/tests/query/QueryUTest.cxxtest b/tests/query/QueryUTest.cxxtest index 25b0dbc496..7f40af186e 100644 --- a/tests/query/QueryUTest.cxxtest +++ b/tests/query/QueryUTest.cxxtest @@ -91,13 +91,14 @@ void QueryUTest::test_basic(void) // Can we execute it? ValuePtr sv = h->execute(as); - TS_ASSERT_EQUALS(sv->get_type(), LINK_VALUE); + TS_ASSERT(nameserver().isA(sv->get_type(), LINK_VALUE)); TS_ASSERT_EQUALS(LinkValueCast(sv)->value().size(), 0); // A more complex query ValuePtr v = eval->eval_v("(cog-execute! query)"); - TS_ASSERT_EQUALS(v->get_type(), LINK_VALUE); + TS_ASSERT(nameserver().isA(v->get_type(), LINK_VALUE)); TS_ASSERT_EQUALS(LinkValueCast(v)->value().size(), 3); + printf("Got: %s\n", v->to_string().c_str()); logger().debug("END TEST: %s", __FUNCTION__); } From 9e78aac641d6b65336841cd88d054f7eb5ab2e1a Mon Sep 17 00:00:00 2001 From: Linas Vepstas Date: Mon, 4 May 2020 17:15:32 -0500 Subject: [PATCH 09/12] Update test to verify the new API --- tests/query/ExecutionOutputUTest.cxxtest | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/tests/query/ExecutionOutputUTest.cxxtest b/tests/query/ExecutionOutputUTest.cxxtest index 96c229df30..6463bead6d 100644 --- a/tests/query/ExecutionOutputUTest.cxxtest +++ b/tests/query/ExecutionOutputUTest.cxxtest @@ -359,7 +359,7 @@ void ExecutionOutputUTest::test_value(void) eval.eval("(load-from-path \"tests/query/exec-gsn.scm\")"); ValuePtr result = eval.eval_v("(cog-execute! exec-query-handle)"); - TS_ASSERT_EQUALS(result->get_type(), LINK_VALUE); + TS_ASSERT(nameserver().isA(result->get_type(), LINK_VALUE)); ValueSeq vals = LinkValueCast(result)->value(); TS_ASSERT_EQUALS(vals.size(), 3); for (const ValuePtr& va : vals) @@ -367,7 +367,7 @@ void ExecutionOutputUTest::test_value(void) //------------ result = eval.eval_v("(cog-execute! exec-query-link-value)"); - TS_ASSERT_EQUALS(result->get_type(), LINK_VALUE); + TS_ASSERT(nameserver().isA(result->get_type(), LINK_VALUE)); vals = LinkValueCast(result)->value(); TS_ASSERT_EQUALS(vals.size(), 3); for (const ValuePtr& va : vals) @@ -375,7 +375,7 @@ void ExecutionOutputUTest::test_value(void) //------------ result = eval.eval_v("(cog-execute! exec-query-string-value)"); - TS_ASSERT_EQUALS(result->get_type(), LINK_VALUE); + TS_ASSERT(nameserver().isA(result->get_type(), LINK_VALUE)); vals = LinkValueCast(result)->value(); TS_ASSERT_EQUALS(vals.size(), 3); for (const ValuePtr& va : vals) @@ -383,7 +383,7 @@ void ExecutionOutputUTest::test_value(void) //------------ result = eval.eval_v("(cog-execute! exec-query-truth-value)"); - TS_ASSERT_EQUALS(result->get_type(), LINK_VALUE); + TS_ASSERT(nameserver().isA(result->get_type(), LINK_VALUE)); vals = LinkValueCast(result)->value(); TS_ASSERT_EQUALS(vals.size(), 3); for (const ValuePtr& va : vals) From 850c135f7ea736bac3d7c6f7787f284e0975eb9b Mon Sep 17 00:00:00 2001 From: Linas Vepstas Date: Mon, 4 May 2020 22:56:48 -0500 Subject: [PATCH 10/12] Provide a simpler, common method for getting the HandleSeq --- opencog/atoms/pattern/BindLink.cc | 8 +------- opencog/atoms/pattern/GetLink.cc | 8 +------- opencog/atoms/value/LinkValue.cc | 16 ++++++++++++++++ opencog/atoms/value/LinkValue.h | 2 ++ 4 files changed, 20 insertions(+), 14 deletions(-) diff --git a/opencog/atoms/pattern/BindLink.cc b/opencog/atoms/pattern/BindLink.cc index fa5df16978..6662ee1d55 100644 --- a/opencog/atoms/pattern/BindLink.cc +++ b/opencog/atoms/pattern/BindLink.cc @@ -65,15 +65,9 @@ BindLink::BindLink(const HandleSeq&& hseq, Type t) /** Wrap query results in a SetLink, place them in the AtomSpace. */ ValuePtr BindLink::execute(AtomSpace* as, bool silent) { - HandleSeq rslt; QueueValuePtr qv(do_execute(as, silent)); OC_ASSERT(qv->is_closed(), "Unexpected queue state!"); - std::queue vals(qv->wait_and_take_all()); - while (not vals.empty()) - { - rslt.push_back(HandleCast(vals.front())); - vals.pop(); - } + HandleSeq rslt(qv->to_handle_seq()); // If there is an anchor, then attach results to the anchor. // Otherwise, create a SetLink and return that. diff --git a/opencog/atoms/pattern/GetLink.cc b/opencog/atoms/pattern/GetLink.cc index d6c38ed104..eaf4650a9b 100644 --- a/opencog/atoms/pattern/GetLink.cc +++ b/opencog/atoms/pattern/GetLink.cc @@ -59,15 +59,9 @@ QueueValuePtr GetLink::do_execute(AtomSpace* as, bool silent) ValuePtr GetLink::execute(AtomSpace* as, bool silent) { - HandleSeq hs; QueueValuePtr qv(do_execute(as, silent)); OC_ASSERT(qv->is_closed(), "Unexpected queue state!"); - std::queue vals(qv->wait_and_take_all()); - while (not vals.empty()) - { - hs.push_back(HandleCast(vals.front())); - vals.pop(); - } + HandleSeq hs(qv->to_handle_seq()); // If there is an anchor, then attach results to the anchor. // Otherwise, create a SetLink and return that. diff --git a/opencog/atoms/value/LinkValue.cc b/opencog/atoms/value/LinkValue.cc index 08c254c3b4..88cdc9d189 100644 --- a/opencog/atoms/value/LinkValue.cc +++ b/opencog/atoms/value/LinkValue.cc @@ -20,11 +20,27 @@ * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. */ +#include #include #include using namespace opencog; + +HandleSeq LinkValue::to_handle_seq(void) const +{ + update(); + HandleSeq hs; + for (const ValuePtr& v : _value) + { + if (v->is_atom()) + hs.push_back(HandleCast(v)); + } + return hs; +} + +// ============================================================== + bool LinkValue::operator==(const Value& other) const { if (LINK_VALUE != other.get_type()) return false; diff --git a/opencog/atoms/value/LinkValue.h b/opencog/atoms/value/LinkValue.h index 7b4f6fe9e3..3b1ee3dfc6 100644 --- a/opencog/atoms/value/LinkValue.h +++ b/opencog/atoms/value/LinkValue.h @@ -25,6 +25,7 @@ #include #include +#include #include namespace opencog @@ -57,6 +58,7 @@ class LinkValue virtual ~LinkValue() {} const std::vector& value() const { update(); return _value; } + HandleSeq to_handle_seq(void) const; /** Returns a string representation of the value. */ virtual std::string to_string(const std::string& indent = "") const; From 9389d738273c201f7277023be961afb3b8b08c3c Mon Sep 17 00:00:00 2001 From: Linas Vepstas Date: Mon, 4 May 2020 22:59:52 -0500 Subject: [PATCH 11/12] Add new link dependencies --- tests/atoms/value/CMakeLists.txt | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tests/atoms/value/CMakeLists.txt b/tests/atoms/value/CMakeLists.txt index d4d60af4e9..f2731af517 100644 --- a/tests/atoms/value/CMakeLists.txt +++ b/tests/atoms/value/CMakeLists.txt @@ -1,5 +1,7 @@ LINK_LIBRARIES( value + atombase + exec ) # Tests in order of increasing functional complexity/dependency From 2563b6d7edad54505e9433f9e8873531c1d4117a Mon Sep 17 00:00:00 2001 From: Linas Vepstas Date: Mon, 4 May 2020 23:14:44 -0500 Subject: [PATCH 12/12] Temp hack for pattern miner Remove after opencog/miner#40 is merged ... --- opencog/query/Satisfier.h | 3 +++ 1 file changed, 3 insertions(+) diff --git a/opencog/query/Satisfier.h b/opencog/query/Satisfier.h index bf7ebae777..a5fe0446a4 100644 --- a/opencog/query/Satisfier.h +++ b/opencog/query/Satisfier.h @@ -99,7 +99,10 @@ class SatisfyingSet : protected: AtomSpace* _as; HandleSeq _varseq; +// XXX fixme tempt hack for pattern miner +public: HandleSet _satisfying_set; +protected: QueueValuePtr _result_queue; public: