MOZI-AI / annotation-scheme

Human Gene annotation service backend
GNU General Public License v3.0
3 stars 4 forks source link

Change location of where ListLink wrapping occurs. #165

Open linas opened 4 years ago

linas commented 4 years ago

As discussed in issue #164.

Untested -- I have not tried running this code; I think it's correct but I might have made a mistake, or missed a spot where a change was needed.

tanksha commented 4 years ago

@linas I think its better to get rid of them at all. As the GroundedSchemaNodes are removed, there is no need for extra ListLink to wrap the output. I will be working on this

linas commented 4 years ago

I don't understand at all. The ListLink here has nothing at all to do with GroundedSchemaNodes.... they are completely unrelated. The GroundedSchema never needed these... The ListLink here is grouping together a bunch of clauses, for some other unknown reason.

tanksha commented 4 years ago

@linas yes, the GroundedSchemaNodes have nothing to do with this but moving the ListLink up will create a much bigger number of outgoing nodes for the ListLink which the Pattern miner code is complains.

linas commented 4 years ago

moving the ListLink up will create a much bigger number of outgoing nodes

This does not change the number of nodes/links created, at all: exactly the same structures are created, either way. All that this does is refactor the code-base. It does not change the output of the codebase.

tanksha commented 4 years ago

@Habush I will merge this PR as it fixes the issue with having an empty ListLink you were getting from Biogrid. can you check?

linas commented 4 years ago

empty ListLink

This pull req does not change the output: it still produces empty list links in all the same places as before.

linas commented 4 years ago

Yes, exactly, it replaces 9 different locations by only two. That should make it easier to remove those two, if/when you are ready to do that, rather than struggling with all 9.

tanksha commented 4 years ago

The ListLink here is grouping together a bunch of clauses, for some other unknown reason.

To be clear on why we were using ListLink

at first, the generate-result function was called through a GroundedSchemaNode, and when there is no result to be returned, we were having #unspecified variable error because the pattern matching query expects an opencog atom as a result, thats why we send an empty ListLink.

and when there is a result, since we were sending a bunch of clauses (more than one EvaluationLinks) as a result, we need to wrap all in one ListLink and return.

But now, we don't have a GroundedSchemaNode ... but when you refactor the code, since you dont want to modify the output, you keep the ListLink but we dont need them anymore as it was for the above reason we used them HERE in the first place.

linas commented 4 years ago

unspecified

It is impossible for the pattern matcher to ever see *unspecified*, since this is a scheme-only object. Atomese also never-ever generates *unspecified*. It is only generated by guile - for example, (if #f "foo") evaluates to *unspecified* because one of the two if-statement branches is unspecified. There are lots of if's in the code-base, and the *unspecified* is coming from one of them.

since we were sending a bunch of clauses (more than one EvaluationLinks) as a result, we need to wrap all in one ListLink and return.

Why? That is the original question: why do you need to wrap them? Oh, I understand now.

GroundedSchemaNode

The use of the ListLinks has nothing at all to do with the GroundedSchema. They are completely unrelated to one-another. The code generates the same results before, and after. One could have eliminated the ListLinks entirely, and still used GSN's for everything; it would have made no difference. Oh I understand now.

linas commented 4 years ago

Here's a better example:

(define ttt #t)
(define u (if (not ttt) "foo"))
(equal? u *unspecified*)
(display u)(newline)
linas commented 4 years ago

Oh... I understand now ...