opencog / atomspace

The OpenCog (hyper-)graph database and graph rewriting system
https://wiki.opencog.org/w/AtomSpace
Other
801 stars 225 forks source link

PatternLink::setup_components is likely buggy #110

Closed ngeiswei closed 9 years ago

ngeiswei commented 9 years ago

There is probably something wrong with PatternLink::setup_components, to see why you may do as follows

*1. Add the followning at l.93 of opencog/atoms/bind/PatternLink.cc

        std::cout << "PatternLink::setup_components h = " << h << std::endl;

in PatternLink::setup_components' loop after the definition of h.

*2. Run any pattern matcher example from examples/pattern-matcher, or any rule-engine example, you may see that h is always UNDEFINED.

That cannot be right, right? I think the problem is that the created patternLink is not added to the AtomSpace so no Handle can be associated to it. I'm not sure it should be added to atomspace, so maybe the solution is to have _component_patterns take PatternLinkPtrs instead of Handles.

ngeiswei commented 9 years ago

Hmm, I've changed _component_patterns to store a vector of PatternLinkPtrs instead of Handles but it makes no difference in the end (actually my problem is that some pattern matcher query crashes). It seems that the Handle associated to an atom not in the atomspace is still somewhat valid, although UNDEFINED it still points to the atom.

I guess I have no choice but to isolate the pattern matcher query that produces that crash.

linas commented 9 years ago

The code in there does many things that are not obvious at first. If I remember correctly, components are not used at all if the graph has a single component (from the point of view of a "virtual atom")

some other things which can be surprising and unexpected:
-- there are two lists of clauses; one of the lists has all NotLink and etc. stripped out from it. -- some of the intializers are called in a different order, depending on the atom type. The intialization sequence can be confusing, when you read through the code. If you try to simplify it, you will eventually find the spot where the extra complexity is needed. -- there are some circular shared library dependencies that prevent some atoms from being insterted into the atomspace; they will have valid pointers, but -1 as the UUID, so that the handle is technically invalid. This is annoying but I think very rare.

linas commented 9 years ago

Re components: if a graph has a single compontent, then "components" is not used.

If a graph has multiple components, then a bunch of atoms, each containing exactly one component, are created. These atoms must NOT be inserted into the atomspace, as they aren't "really there", are not whole and complete. Instead, they are used as holders for the unpacked data. These will thus be valid atom pointers, but will have an invalid UUID of -1.

Each of the sub-components are satisfied individually, and then the whole is stitched back together again from these parts; only the whole is "valid"

ngeiswei commented 9 years ago

I see. Reading the AtomSpace code I confirmed that invalid Handle doesn't mean invalid Atom, just atom not in the any atomspace, as I suspected. So the bug is elsewhere.

ngeiswei commented 9 years ago

Hmmm, handle comparison only works when the atoms are inserted in the AtomSpace, maybe that would be the problem...

ngeiswei commented 9 years ago

I cannot reproduce that bug anymore. I could go through the git history to identify what commit has fixed it but I've got other things to do. I guess I'll close it then.