RTXteam / RTX

Software repo for Team Expander Agent (Oregon State U., Institute for Systems Biology, and Penn State U.)
https://arax.ncats.io/
MIT License
33 stars 20 forks source link

New expand() error #748

Closed edeutsch closed 4 years ago

edeutsch commented 4 years ago

Hi everyone, I have just rolled out the latest code in master, beta, and test. And I'm finding a problem with something that worked before. Running this query:

add_qnode(name=babesia, id=n00) add_qnode(id=n01) add_qedge(source_id=n00, target_id=n01, id=e00) expand(edge_id=e00, kp=ARAX/KG2) resultify()

now results in this error at https://arax.rtx.ai/

2020-05-18 20:06:28.265511 INFO: Sending cypher query for edge e00 to KG2 neo4j 2020-05-18 20:06:35.311163 INFO: Query for edge e00 returned results (e00: 239, n00: 9, n01: 216) 2020-05-18 20:06:35.311188 DEBUG: Processing query results for edge e00 2020-05-18 20:06:35.435444 DEBUG: Merging results into Message.KnowledgeGraph 2020-05-18 20:06:35.435551 ERROR: Node CUI:C0004572 has been returned as an answer for multiple query graph nodes (n01 and n00)

saramsey commented 4 years ago

Hi @edeutsch I do not find this message text has been returned as an answer for multiple query graph nodes in ARAX_resultify.py. Are you sure it is coming from Resultify?

saramsey commented 4 years ago

see ARAX_expander line 474. reassigning to @amykglen

edeutsch commented 4 years ago

ah, you're right, my mistake.

amykglen commented 4 years ago

Yeah - I tried to explain in #742 that this query is (and should be) throwing this error in expand currently, because the specified node is returned for multiple query graph node ids, which we don't have a way of capturing at the moment.

However, I'm working today on having expand throw out self-edges, and since this multiple query graph ID situation is due to a self edge, it will no longer throw an error at that point.

saramsey commented 4 years ago

Hi all, I am working on a similar-sounding, but fundamentally unrelated issue (#743). But I am intervening in filter_kg_and_remap_predicates.py and I think @amykglen is addressing #742 inside Expander, right?

amykglen commented 4 years ago

Hi @amykglen are you working on #743 then? If so, great, I will assign it to you.

If you meant #742 - yes, I'm working on #742 (not #743).

Hi all, I am working on a similar-sounding, but fundamentally unrelated issue (#743). But I am intervening in filter_kg_and_remap_predicates.py and I think @amykglen is addressing #742 inside Expander, right?

Yes - after today (hopefully) expand will ignore any self-edges that may be present in KG2/other knowledge providers. (So it won't matter for ARAX purposes whether KG2 has self-edges - they'll always be ignored.)

saramsey commented 4 years ago

Hi @amykglen are you working on #743 then? If so, great, I will assign it to you.

If you meant #742 - yes, I'm working on #742 (not #743).

Hi all, I am working on a similar-sounding, but fundamentally unrelated issue (#743). But I am intervening in filter_kg_and_remap_predicates.py and I think @amykglen is addressing #742 inside Expander, right?

Yes - after today (hopefully) expand will ignore any self-edges that may be present in KG2/other knowledge providers. (So it won't matter for ARAX purposes whether KG2 has self-edges - they'll always be ignored.)

That makes sense. In #743 I am making filter_kg_and_remap_predicates.py drop self-edges with the subclass_of predicate, as it is kind of nonsensical (calls to mind de Morgan's poem about fleas).

amykglen commented 4 years ago

Ok - fix for this is pushed to master. Just need to confirm it's working once it's rolled out.

edeutsch commented 4 years ago

Latest master rolled out to production.

amykglen commented 4 years ago

Thanks! Original query appears to be running successfully in production now. https://arax.rtx.ai/?m=2073