Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add cond link #2066

Merged
merged 13 commits into from
Feb 12, 2019
16 changes: 16 additions & 0 deletions opencog/atoms/atom_types/atom_types.script
Original file line number Diff line number Diff line change
@@ -658,3 +658,19 @@ CONNECTOR <- LINK // Link-grammar connector
CONNECTOR_DIR_NODE <- NODE "ConnectorDir" // Connector direction
CONNECTOR_SEQ <- ORDERED_LINK // Sequence of connectors
SECTION <- ORDERED_LINK // Sheaf section, connector set

// Conditional based on crisp boolean value from TV.Might have two structure
linas marked this conversation as resolved.
Show resolved Hide resolved
//
// wrapped with ListLink
// CondLink
// ListLink
// GreaterThan ; any evaluable link [condition]
// Number 5
// Number 2
// Number 0 ;[expression]
//
// flattened
// CondLink
// TrueLink ;[condition]
// Number 1 ;[expression]
linas marked this conversation as resolved.
Show resolved Hide resolved
COND_LINK <- FUNCTION_LINK
2 changes: 2 additions & 0 deletions opencog/atoms/execution/CMakeLists.txt
Original file line number Diff line number Diff line change
@@ -14,6 +14,7 @@ ADD_LIBRARY (execution
Instantiator.cc
MapLink.cc
LibraryManager.cc
CondLink.cc
linas marked this conversation as resolved.
Show resolved Hide resolved
)

# Without this, parallel make will race and crap up the generated files.
@@ -54,5 +55,6 @@ ENDIF (HAVE_GUILE)
# are fixed, first.
INSTALL (FILES
MapLink.h
CondLink.h
DESTINATION "include/opencog/atoms/core"
)
99 changes: 99 additions & 0 deletions opencog/atoms/execution/CondLink.cc
Original file line number Diff line number Diff line change
@@ -0,0 +1,99 @@
/*
* opencog/atoms/execution/CondLink.cc
*
* Copyright (C) 2019 Kasim Ebrahim
*
* 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
*
* This program is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
* 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:
* Free Software Foundation, Inc.,
* 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
*/

#include <opencog/atoms/base/ClassServer.h>
#include <opencog/atoms/execution/Instantiator.h>
#include <opencog/atoms/core/FindUtils.h>

#include "CondLink.h"
#include "EvaluationLink.h"

using namespace opencog;

void CondLink::init(void)
{
if (0 == _outgoing.size())
throw SyntaxException(TRACE_INFO,
"CondLink is expected to be arity greater-than 0!");

if (1 == _outgoing.size()) {
default_exp = _outgoing[0];
return;
}

for (unsigned i = 0; i < _outgoing.size(); ++i) {
// If the conditions and expressions are wrapped in list_link
if (LIST_LINK == _outgoing[i]->get_type()) {
// The first item in the list_link holds the condition,
// and the second holds the expression.
conds.push_back(_outgoing[i]->getOutgoingSet()[0]);
exps.push_back(_outgoing[i]->getOutgoingSet()[1]);
continue;
}

// If cond_link starts wrapping conds and exps in list_link, then it is
// expected to be consistent. If one wants to have a default expression
// using true_link as condition should do it.
if (i != 0 && LIST_LINK == _outgoing[i - 1]->get_type())
throw SyntaxException(TRACE_INFO,
"CondLink is expected to wrap expressions in LIST_LINK.");
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of this check, and the checks below, it would be better to check if the even atoms are evaluatable, and the odd ones are executable. nameserver().isA(type, EVALUATABLE_LINK) and nameserver.isA(type, EXECUTABLE_LINK) The EVALUATABLE_LINK type already exists, and "does the right thing"; the EXECUTABLE_LINK does not yet exist) (I started adding it a few weeks ago, but did not finish. Perhaps you can add it, in a separate pull req?)


// If the conditions and expressions are flattened in even and odd
// positions respectively.
if (i % 2 == 0) {
if (i == _outgoing.size() - 1) {
default_exp = _outgoing[i];
return;
}
conds.push_back(_outgoing[i]);
} else {
exps.push_back(_outgoing[i]);
}
}
}

CondLink::CondLink(const HandleSeq &oset, Type t)
: FunctionLink(oset, t)
{
if (not nameserver().isA(t, COND_LINK)) {
const std::string &tname = nameserver().getTypeName(t);
throw SyntaxException(TRACE_INFO,
"Expecting a CondLink, got %s", tname.c_str());
}

// Derived types have a different initialization sequence.
if (COND_LINK != t) return;
init();
}

Handle CondLink::execute(AtomSpace *scratch) const
{
TruthValuePtr tvp;
Instantiator inst(scratch);
for (unsigned i = 0; i < conds.size(); ++i) {
tvp = EvaluationLink::do_evaluate(scratch, conds[i]);
if (tvp->get_mean() > 0.5)
return HandleCast(inst.instantiate(exps[i], HandleMap()));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is not wrong, but there is now a new and better way of doing this: call exps[i]->execute(scratch).

This generic execute() method is brand new, introduced just a week ago, and it is meant to resolve multiple issues with calling the instantiator directly.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @linas, In case of eager execution the expressions in the CondLink might have been already executed right? such as an ArithmeticLink might have been already reduced to a NumberNode and calling execute on that NumberNode should throw the "Not Executable" exception in Atom.h? or am I missing something?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, for eager evaluation, the arguments will already have been reduced. There should not be any exceptions as a result, as the reduction is (meant to be) performed only once, not twice.

Lay evaluation would be better; it almost works; there's some open issue saying "must finish this work. Anyway, toe CondLink code looks OK to me.

}
return HandleCast(inst.instantiate(default_exp, HandleMap()));
}

DEFINE_LINK_FACTORY(CondLink, COND_LINK)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
DEFINE_LINK_FACTORY(CondLink, COND_LINK)
DEFINE_LINK_FACTORY(CondLink, COND_LINK)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What Nil says. Your editor seems to forget to inert a newline.

60 changes: 60 additions & 0 deletions opencog/atoms/execution/CondLink.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,60 @@
/*
* opencog/atoms/execution/CondLink.h
*
* Copyright (C) 2019 Kasim Ebrahim
* All Rights Reserved
*
* 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
*
* This program is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
* 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:
* Free Software Foundation, Inc.,
* 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
*/

#ifndef _OPENCOG_COND_LINK_H
#define _OPENCOG_COND_LINK_H

#include <opencog/atoms/core/FunctionLink.h>
#include <opencog/atoms/core/ScopeLink.h>
#include <opencog/atoms/core/Quotation.h>

namespace opencog
{
class CondLink : public FunctionLink
{
protected:
HandleSeq conds;
HandleSeq exps;
Handle default_exp;

void init(void);

public:
CondLink(const HandleSeq&, Type=COND_LINK);

virtual Handle execute(AtomSpace* = nullptr) const;

static Handle factory(const Handle&);
};

typedef std::shared_ptr<CondLink> CondLinkPtr;
static inline CondLinkPtr CondLinkCast(const Handle& h)
{ AtomPtr a(h); return std::dynamic_pointer_cast<CondLink>(a); }
static inline CondLinkPtr CondLinkCast(AtomPtr a)
{ return std::dynamic_pointer_cast<CondLink>(a); }

#define createCondLink std::make_shared<CondLink>

/** @}*/
}

#endif // _OPENCOG_COND_LINK_H
16 changes: 16 additions & 0 deletions opencog/atoms/execution/Instantiator.cc
Original file line number Diff line number Diff line change
@@ -27,6 +27,7 @@
#include <opencog/atoms/execution/ExecutionOutputLink.h>
#include <opencog/atoms/execution/EvaluationLink.h>
#include <opencog/atoms/execution/MapLink.h>
#include <opencog/atoms/execution/CondLink.h>
#include <opencog/atoms/reduct/FoldLink.h>

#include "Instantiator.h"
@@ -436,6 +437,21 @@ Handle Instantiator::walk_tree(const Handle& expr, bool silent)
}
}

if (COND_LINK == t)
{
if (_eager)
{
HandleSeq oset_results;
walk_sequence(oset_results, expr->getOutgoingSet(), silent);
CondLinkPtr clp(CondLinkCast(createLink(oset_results, t)));
return clp->execute(_as);
}
else
{
CondLinkPtr clp(CondLinkCast(expr));
return clp->execute(_as);
}
}
linas marked this conversation as resolved.
Show resolved Hide resolved
// Fire any other function links, not handled above.
if (nameserver().isA(t, FUNCTION_LINK))
{
2 changes: 2 additions & 0 deletions tests/atoms/CMakeLists.txt
Original file line number Diff line number Diff line change
@@ -32,6 +32,8 @@ IF(HAVE_GUILE)
TARGET_LINK_LIBRARIES(QuotationUTest execution smob atomspace)
ADD_CXXTEST(FormulaUTest)
TARGET_LINK_LIBRARIES(FormulaUTest execution smob atomspace)
ADD_CXXTEST(CondLinkUTest)
TARGET_LINK_LIBRARIES(CondLinkUTest execution smob atomspace)
ENDIF(HAVE_GUILE)

ADD_CXXTEST(AlphaConvertUTest)
121 changes: 121 additions & 0 deletions tests/atoms/CondLinkUTest.cxxtest
Original file line number Diff line number Diff line change
@@ -0,0 +1,121 @@
/*
* tests/atoms/MapLinkUTest.cxxtest
linas marked this conversation as resolved.
Show resolved Hide resolved
*
* Copyright (C) 2019 Kasim Ebrahim
* All Rights Reserved
*
* 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
*
* This program is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
* 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:
* Free Software Foundation, Inc.,
* 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
*/

#include <opencog/guile/SchemeEval.h>
#include <opencog/atomspace/AtomSpace.h>
#include <opencog/atoms/execution/Instantiator.h>
#include <opencog/util/Logger.h>

using namespace opencog;

class CondLinkUTest : public CxxTest::TestSuite
{
private:
AtomSpace *as;
SchemeEval *eval;

public:
CondLinkUTest(void)
{
logger().set_level(Logger::DEBUG);
logger().set_print_to_stdout_flag(true);

as = new AtomSpace();
eval = new SchemeEval(as);
eval->eval("(add-to-load-path \"" PROJECT_SOURCE_DIR "\")");
eval->eval("(use-modules (opencog exec))");
}

~CondLinkUTest()
{
delete eval;
delete as;
// Erase the log file if no assertions failed.
if (!CxxTest::TestTracker::tracker().suiteFailed())
std::remove(logger().get_filename().c_str());
}

void setUp(void);

void tearDown(void);

void test_singleton(void);

void test_nondefault_exp(void);

void test_wrapped_exp();
};

void CondLinkUTest::tearDown(void)
{
as->clear();
}

void CondLinkUTest::setUp(void)
{
as->clear();
}

void CondLinkUTest::test_singleton()
{
logger().debug("BEGIN TEST: %s", __FUNCTION__);

eval->eval("(load-from-path \"tests/atoms/condlink.scm\")");

Handle result = eval->eval_h("(cog-execute! single)");

Handle baz = eval->eval_h("(Number -1)");
printf("got %s", result->to_string().c_str());
printf("expected %s\n", baz->to_string().c_str());

TS_ASSERT(result == baz);
}

void CondLinkUTest::test_nondefault_exp()
{
logger().debug("BEGIN TEST: %s", __FUNCTION__);

eval->eval("(load-from-path \"tests/atoms/condlink.scm\")");

Handle result = eval->eval_h("(cog-execute! nondefault)");

Handle baz = eval->eval_h("(Number 5)");
printf("got %s", result->to_string().c_str());
printf("expected %s\n", baz->to_string().c_str());

TS_ASSERT(result == baz);
}

void CondLinkUTest::test_wrapped_exp()
{
logger().debug("BEGIN TEST: %s", __FUNCTION__);

eval->eval("(load-from-path \"tests/atoms/condlink.scm\")");

Handle result = eval->eval_h("(cog-execute! listwrapped)");

Handle baz = eval->eval_h("(NumberNode 1)");
printf("got %s", result->to_string().c_str());
printf("expected %s\n", baz->to_string().c_str());

TS_ASSERT(result == baz);
}
linas marked this conversation as resolved.
Show resolved Hide resolved
Loading