PatternMatcherCallBack grounding var_soln corrupted?

[williampma]

This one is tough. Need to write some custom code to test this.

First, assume the following are in the AtomSpace

(EvaluationLink
   (PredicateNode "chirps")
   (ConceptNode "Tweety")
)
(EvaluationLink
   (PredicateNode "eats_flies")
   (ConceptNode "Tweety")
)
(InheritanceLink
   (ConceptNode "Tweety")
   (ConceptNode "Yello")
)
(BindLink (av 0 0 0)
   (VariableList (av 0 0 0)
      (VariableNode "$A")
      (VariableNode "$B")
   )
   (AndLink
      (VariableNode "$A")
      (ImplicationLink
         (VariableNode "$A")
         (VariableNode "$B")
      )
   )
   (ExecutionOutputLink
      (GroundedSchemaNode "scm: pln-formula-simple-modus-ponens")
      (ListLink
         (VariableNode "$B")
         (ImplicationLink
            (VariableNode "$A")
            (VariableNode "$B")
         )
      )
   )
)
(ListLink
   (InheritanceLink
      (ConceptNode "Fritz")
      (ConceptNode "green")
   )
   (ImplicationLink
      (InheritanceLink
         (ConceptNode "Fritz")
         (ConceptNode "Frog")
      )
      (InheritanceLink
         (ConceptNode "Fritz")
         (ConceptNode "green")
      )
   )
)
(ListLink
   (InheritanceLink
      (ConceptNode "Fritz")
      (ConceptNode "Frog")
   )
   (ImplicationLink
      (AndLink
         (EvaluationLink
            (PredicateNode "croaks")
            (ConceptNode "Fritz")
         )
         (EvaluationLink
            (PredicateNode "eats_flies")
            (ConceptNode "Fritz")
         )
      )
      (InheritanceLink
         (ConceptNode "Fritz")
         (ConceptNode "Frog")
      )
   )
)
(ImplicationLink
   (AndLink
      (EvaluationLink
         (PredicateNode "chirps")
         (VariableNode "$Y")
      )
      (EvaluationLink
         (PredicateNode "sings")
         (VariableNode "$Y")
      )
   )
   (InheritanceLink
      (VariableNode "$Y")
      (ConceptNode "Canary")
   )
)
(ImplicationLink
   (AndLink
      (EvaluationLink
         (PredicateNode "croaks")
         (VariableNode "$X")
      )
      (EvaluationLink
         (PredicateNode "eats_flies")
         (VariableNode "$X")
      )
   )
   (InheritanceLink
      (VariableNode "$X")
      (ConceptNode "Frog")
   )
)
(ImplicationLink
   (InheritanceLink
      (VariableNode "$Z")
      (ConceptNode "Frog")
   )
   (InheritanceLink
      (VariableNode "$Z")
      (ConceptNode "green")
   )
)
(ImplicationLink
   (InheritanceLink
      (VariableNode "$A")
      (ConceptNode "Canary")
   )
   (InheritanceLink
      (VariableNode "$A")
      (ConceptNode "yellow")
   )
)

Then, write a new PatternMatcherCallBack with a new grounding method that just print the var_soln mapping

bool NewPMCB::grounding(const std::map<Handle, Handle> &var_soln,
                               const std::map<Handle, Handle> &pred_soln)
{
    for (auto& p : var_soln)
        logger().debug("var_soln map " + p.first->toShortString() + " to " + p.second->toShortString());

    return false;
}

Then have some variable Handle htarget then contains the following additional atom

(AndLink (stv 1.000000 0.000000)
  (InheritanceLink (stv 1.000000 0.000000)
    (VariableNode "$Z") ; [142]
    (ConceptNode "Frog") ; [115]
  ) ; [150]
  (ImplicationLink (stv 1.000000 0.000000)
    (InheritanceLink (stv 1.000000 0.000000)
      (VariableNode "$Z") ; [142]
      (ConceptNode "Frog") ; [115]
    ) ; [150]
    (InheritanceLink (stv 1.000000 0.000000)
      (VariableNode "$Z") ; [142]
      (ConceptNode "green") ; [113]
    ) ; [151]
  ) ; [152]
) ; [477]

And call the Pattern Matcher code like this

    // Get all VariableNodes (unquoted)
    FindAtoms fv(VARIABLE_NODE);
    fv.search_set(htarget);

    HandleSeq vars;
    for (auto& h : fv.varset)
        vars.push_back(h);

    Handle htarget_vardecl = _as->addAtom(createVariableList(vars));

    PatternLinkPtr sl(createPatternLink(htarget_vardecl, htarget));
    NewPMCB pmcb(_as);

    logger().debug("Before patterm matcher");

    sl->satisfy(pmcb);

    logger().debug("After running pattern matcher");

cogserver crash at the line printing var_soln (in particular, when printing p.second. Printing p.first was fine).

In my case, the crash code is like this

[2015-06-04 04:18:41:696] [ERROR] Caught signal 11 (Segmentation fault) on thread 140051551868672
    Stack Trace:
    2: basic_string.h:539   ~basic_string()
    3: CogServerMain.cc:79  _Z7sighandi()
    4: ??:0 killpg()
    5: BackwardChainerPMCB.cc:57      opencog::BackwardChainerPMCB::grounding(std::map<opencog::Handle, opencog::Handle, std::less<opencog::Handle>, std::allocator<std::pair<opencog::Handle const, opencog::Handle> > > const&, std::map<opencog::Handle, opencog::Handle, std::less<opencog::Handle>, std::allocator<std::pair<opencog::Handle const, opencog::Handle> > > const&)
    6: PatternMatchEngine.cc:1295     opencog::PatternMatchEngine::do_next_clause()
    7: PatternMatchEngine.cc:1104     opencog::PatternMatchEngine::do_term_up(opencog::Handle const&, opencog::Handle const&, opencog::Handle const&)
    8: PatternMatchEngine.cc:1048     opencog::PatternMatchEngine::explore_single_branch(opencog::Handle const&, opencog::Handle const&, opencog::Handle const&)
    9: shared_ptr_base.h:545    ~__shared_count()
    10: PatternMatchEngine.cc:1208    opencog::PatternMatchEngine::do_term_up(opencog::Handle const&, opencog::Handle const&, opencog::Handle const&)
    11: PatternMatchEngine.cc:1048    opencog::PatternMatchEngine::explore_single_branch(opencog::Handle const&, opencog::Handle const&, opencog::Handle const&)
    12: PatternMatchEngine.cc:1335    opencog::PatternMatchEngine::do_next_clause()
    13: PatternMatchEngine.cc:1104    opencog::PatternMatchEngine::do_term_up(opencog::Handle const&, opencog::Handle const&, opencog::Handle const&)
    14: PatternMatchEngine.cc:1048    opencog::PatternMatchEngine::explore_single_branch(opencog::Handle const&, opencog::Handle const&, opencog::Handle const&)
    15: shared_ptr_base.h:545   ~__shared_count()
    16: PatternMatchEngine.cc:1208    opencog::PatternMatchEngine::do_term_up(opencog::Handle const&, opencog::Handle const&, opencog::Handle const&)
    17: PatternMatchEngine.cc:1048    opencog::PatternMatchEngine::explore_single_branch(opencog::Handle const&, opencog::Handle const&, opencog::Handle const&)
    18: InitiateSearchCB.cc:286   opencog::InitiateSearchCB::neighbor_search(opencog::PatternMatchEngine*)
    19: InitiateSearchCB.cc:481   opencog::InitiateSearchCB::disjunct_search(opencog::PatternMatchEngine*)
    20: PatternMatch.cc:341   opencog::PatternLink::satisfy(opencog::PatternMatchCallback&) const
    21: BackwardChainer.cc:112    opencog::BackwardChainer::do_until(unsigned int)
    22: InferenceSCM.cc:134   opencog::InferenceSCM::do_backward_chaining(opencog::Handle)
    23: shared_ptr_base.h:545   ~__shared_count()
    24: SchemePrimitive.cc:158    opencog::PrimitiveEnviron::do_call(scm_unused_struct*, scm_unused_struct*)
    25: ??:0    scm_vm_engine()
    26: ??:0    scm_call_1()
    27: ??:0    scm_vm_engine()
    28: ??:0    scm_call_3()
    29: ??:0    scm_vm_engine()
    30: ??:0    scm_call_4()
    31: SchemeEval.cc:606     opencog::SchemeEval::do_eval(std::string const&)
    32: SchemeEval.cc:535     opencog::SchemeEval::c_wrap_eval(void*)
    33: ??:0    scm_at_abort()
    34: ??:0    scm_vm_engine()
    35: ??:0    scm_call_4()
    36: ??:0    scm_at_abort()
    37: ??:0    scm_c_with_continuation_barrier()
    38: ??:0    GC_call_with_gc_active()
    39: ??:0    scm_current_processor_count()
    40: ??:0    GC_call_with_stack_base()
    41: ??:0    scm_with_guile()
    42: SchemeEval.cc:502     opencog::SchemeEval::eval_expr(std::string const&)
    43: basic_string.h:293    std::string::_M_data() const
    44: ??:0      std::this_thread::__sleep_for(std::chrono::duration<long, std::ratio<1l, 1l> >, std::chrono::duration<long, std::ratio<1l, 1000000000l> >)
    45: pthread_create.c:312    start_thread()
    46: clone.S:113 clone()

[williampma]

Here's a crashing UTest file.

https://github.com/williampma/atomspace/blob/PMTest/tests/query/PatternCrashUTest.cxxtest

[linas]

Please push and commit the unit test.

Here's some guesses: p.second is probably Handle::UNDEFINED so p.second->toShortStrng crashed because its a null pointer dereference.

At some point, I'll bet that p.second was pointing at a real, valid atom that was not in the atomspace. Since its not in the atomsapce, it will have a UUID=-1 and both scheme and python get tripped up on atoms with UUID=-1 (even if they are otherwise valid atoms).

Ohh, here I think I see it:

PatternLinkPtr sl(createPatternLink(htarget_vardecl, htarget))

you're creating an atom here, but you are not putting it in the atomspace. That atom will have the variable-decls in its outgoing set, and so as the pattern matcher walks the incoming set of the variable decls, it will, at some point, look at the atom pointed at by PatternLinkPtr .. which is not in any atomspace!

I'm guessing this eventually leads to the crash. if its not this, it could be some other atom(s) that weren't inserted. I suppose that perhaps using mutliple atomspaces and addings/removing/deleteing in a weird order could also results in atoms that are not in atomspaces. ...

[williampma]

I just tried changing that part you mentioned to

    Handle htarget_vardecl = _as->addAtom(createVariableList(vars));
    Handle hpattern = _as->addAtom(createPatternLink(htarget_vardecl, htarget));

    PatternLinkPtr sl = PatternLinkCast(hpattern);
    PMCB pmcb(_as);
    sl->satisfy(pmcb);

It still crashes with a Segmentation fault, so I guess it's not that.

[linas]

Bummer. thought I got lucky. I wont have time to look at this for many days. try adding a #define DEBUG 1 to Pattern.h and compiling, and working backwards from the crash. somewhere is an atom that is not in the atomspace.

This bug might be my fault .. I did re-organize how atoms are placed into the atomspace, recently, and maybe something is not being inserted. That's my best guess.

[williampma]

Ok, I will take a look.

I just verified that indeed p.second was Handle::UNDEFINED when it crashes.

[ngeiswei]

BTW, @williampma why do you use

PatternLinkPtr sl(createPatternLink(htarget_vardecl, htarget))

instead of

PatternLinkPtr sl(htarget_vardecl, htarget)

I'm especially asking because I've seen

// XXX temporary hack ...
#define createPatternLink std::make_shared<PatternLink>

in PatternLink.h, though I don't understand why it is a hack...

[ngeiswei]

Hmmm, this temporary hack is used in AtomTable.cc, but that doesn't mean it should be used by users, I would assume. Users should use instead as->addLink(PATTERN_LINK, ...), that way the atom is added no matter what.

@linas do you know why it is a hack?

[linas]

@linas https://github.com/linas do you know why it is a hack?

No. That warning should be removed It is cut-n-pasted across all Atom files. it was going to be replaced by something clever and pretty, but never was. Maybe some constructor overloading magic. Unrelated to this problem.

[linas]

OK, so I looked at this. I think its a "feature" not a "bug". The code was written before c++11x came out, and that is a few places where the map uses [] instead of at() to look up an element. This causes a null handle to be inserted. It doesn't mean anything -- it just means that, during recursion, it tried looking at atoms that aren't variables. Such atoms are grounded by themselves, and weren't recorded.

Note also: the pattern matcher was written before VariableNodes existed; it assumed the user would deal with these things. I'll try to fix all the spots, meanwhile, just check for null, and don';t use it. Thats what thedebug prints do.

[linas]

I just pushed a fix for this at fb41305e4f207a87694b3b8305299a5c43af43ee -- it was simply old code that was "working as designed", but maybe working a bit ugly.

[williampma]

I see. Thanks, Linas!