RTX icon indicating copy to clipboard operation
RTX copied to clipboard

Migration to TRAPI 1.4

Open edeutsch opened this issue 2 years ago • 19 comments

The migration to TRAPI 1.4 is about to begin. The 1.4-beta release is scheduled for tomorrow March 23. Here is the TRAPI 1.4 ChangeLog to get a sense of what's new: https://github.com/NCATSTranslator/ReasonerAPI/blob/edeutsch-draft-changelog/ChangeLog.md

TRAPI 1.4 specific work should occur in the NewFmt branch. I will sync it with master and then we can begin.

edeutsch avatar Mar 22 '23 18:03 edeutsch

Branch NewFmt is now completely synced with master

edeutsch avatar Mar 22 '23 18:03 edeutsch

@edeutsch and @chunyuma talking with Andy Crouse, for the xDTD results, it would be most helpful for the UI team if we separate out the individual paths as individual supporting graphs. They can reassemble to the full graph if needed, but not conversely. Let me know if that makes sense or if you see any problems with this. I figure since it's a "one off" we can do TRAPI 1.4 for xDTD separately from the rest of the code base

dkoslicki avatar Mar 29 '23 14:03 dkoslicki

@dkoslicki, It makes sense to me. But I am confused about how to implement it. Should this individual supporting graphs still be generated via infer_utilities.py script? In other words, is this separate graph still following the DSL structure/TRAPI format?

chunyuma avatar Mar 29 '23 17:03 chunyuma

This is an important and complex topic. I have no easy answers. I had originally suggested that these "analyses" (individual supporting graphs) be organized within xDTD. But it seems like we decided at the AHM last week that we would continue to do things as we do them now and then convert things at the end? So I'm under the impression that we are on a trajectory not to separate out the individual paths (unless xDTD can provide hints that the transformer can use)

edeutsch avatar Mar 29 '23 19:03 edeutsch

just tagging some sub-issues for our records: I created three separate issues for the three main parts of getting the RTX-KG2 KP on TRAPI 1.4: #1995, #1996, #1997

amykglen avatar Mar 29 '23 21:03 amykglen

I have just pushed a series of commits to branch NewFmt that I think should update our data model to TRAPI 1.4. Which means that there will need to be substantial additional code changes to return to a working state with the new model. Here are some notable changes:

New classes:

  • New Analysis class
  • New AuxiliaryGraph class
  • New MetaQualifier class
  • New RetrievalSoruce
  • New ResourceRoleEnum

Classes that changed:

  • Message adds auxiliary_graphs
  • Result removes edge_bindings and add analyses
  • Edge add sources
  • MetaEdge add qualifiers

There's more, but that other stuff probably won't affect anyone other than me for now.

I think we are ready for everyone else to jump into NewFmt!

edeutsch avatar Mar 29 '23 22:03 edeutsch

ok @edeutsch - I'm getting started on TRAPI 1.4 KG2 KP changes, and currently I'm seeing this kind of error when trying to run pytests in the NewFmt branch:

____________________________________________________ ERROR collecting test_ARAX_query.py ____________________________________________________
ImportError while importing test module '/Users/amyglen/Projects/RTX/code/ARAX/test/test_ARAX_query.py'.
Hint: make sure your test modules/packages have valid Python names.
Traceback:
/Users/amyglen/.pyenv/versions/3.9.7/lib/python3.9/importlib/__init__.py:127: in import_module
    return _bootstrap._gcd_import(name[level:], package, level)
test_ARAX_query.py:9: in <module>
    from ARAX_query import ARAXQuery
../ARAXQuery/ARAX_query.py:29: in <module>
    from ARAX_query_graph_interpreter import ARAXQueryGraphInterpreter
../ARAXQuery/ARAX_query_graph_interpreter.py:15: in <module>
    from ARAX_messenger import ARAXMessenger
../ARAXQuery/ARAX_messenger.py:24: in <module>
    from openapi_server.models.response import Response
../../UI/OpenAPI/python-flask-server/openapi_server/models/__init__.py:56: in <module>
    from openapi_server.models.operation_lookup_and_score import OperationLookupAndScore
../../UI/OpenAPI/python-flask-server/openapi_server/models/operation_lookup_and_score.py:9: in <module>
    from openapi_server.models.one_ofobjectobject import OneOfobjectobject
E   ModuleNotFoundError: No module named 'openapi_server.models.one_ofobjectobject'

seems like most pytests trigger this, but I specifically ran: pytest -vsk test09 --runslow

amykglen avatar Mar 29 '23 23:03 amykglen

oops, forgot to commit that one. just pushed. try again?

edeutsch avatar Mar 29 '23 23:03 edeutsch

fixed, thanks!

amykglen avatar Mar 29 '23 23:03 amykglen

@edeutsch and @chunyuma: my understanding is that xDTD "does it's own thing" in that it doesn't call expand nor resultify; please correct me if I am wrong. I was thinking, since this is the "main" creative mode query to be shown, it would seem reasonable to present it in a way that the UI can show nicely (i.e. the individual paths, each as a separate "supporting graph"), and wouldn't require a whole code-base re-factoring like it would to stuff everything in the "supporting graph" would require.

dkoslicki avatar Mar 30 '23 16:03 dkoslicki

Maybe, I really don't understand these parts of the system sufficiently to have a recommendation.

edeutsch avatar Mar 30 '23 16:03 edeutsch

a few notes for the ARAX implementation. we need to remember to:

  • [x] update Overlay to use the new edge_bindings location
  • [x] update Overlay to add knowledge source information into the new Edge.sources structure (instead of Edge.attributes)
  • [x] update Filter to use the new Edge.sources structure
  • [x] update Filter to use the new edge_bindings location
  • [x] add a new 'transform results' step (to be called after ranking), that removes virtual edges from Analysis.edge_bindings and stuffs them into Analysis.support_graphs

amykglen avatar Mar 30 '23 18:03 amykglen

another thing to remember to do:

  • [ ] Expand should be updated to consider MetaEdge.qualifiers when it's selecting KPs based on their meta knowledge graphs (for queries that involve qualifiers)...

amykglen avatar Apr 12 '23 02:04 amykglen

ok, I added a ResultTransformer that is called at the very end of processing, after ranking... it works for ARAXInfer as well as all our other virtual edges (from Overlay and etc.), so technically this (I think) makes us compliant with TRAPI 1.4. it is automatically run for all queries, including creative mode/Infer. it could use more testing.

later on we may want to refine how virtual edges are divided into separate support graphs; right now, for a given result, any virtual edges that belong to the same option group are lumped into one support graph, and all virtual edges that do not belong to an option group are lumped together into one support graph. this might not be ideal (I know David mentioned potentially putting the separate paths in creative mode results into separate support graphs), but it was the easiest starting point to get us compliant.

@kvnthomas98 - these tests appear to be failing in NewFmt for reasons related to Overlay - can you take a look and adjust things as needed? note that now that the message.query_graph returned in the final ARAX response does not contain virtual qedges/qnodes, some tests are likely failing because they are asserting that a virtual qedge should be in the QG.

FAILED test_ARAX_overlay.py::test_FET_ex1 - assert 0 == 2
FAILED test_ARAX_expand.py::test_xdtd_expand - AssertionError: assert ('ERROR' == 'OK'
FAILED test_ARAX_expand.py::test_xdtd_multiple_categories - AssertionError: assert ('ERROR' == 'OK'
FAILED test_ARAX_expand.py::test_xdtd_different_predicates - AssertionError: assert ('ERROR' == 'OK'

there may be other tests failing as well related to Overlay and/or Infer - it'd be great if you could check for any others.

amykglen avatar Apr 20 '23 23:04 amykglen

@amykglen I've modified those testcases, currently only one test case is failing(test_chemical_substances_that_down_regulate_STK11_issue_28) because the edge bindings in the result returned by molepro is None here and I'm not too sure what to do here.

kvnthomas98 avatar Apr 24 '23 16:04 kvnthomas98

awesome, thanks @kvnthomas98!!

hmm, we'll have to dig in to see why MolePro isn't returning edge_bindings there... maybe their TRAPI 1.4 endpoint has a bug? I think I need to do a little more refinement to that trapi_querier.py module to make it work better with 1.4 - I'll take a look later today...

amykglen avatar Apr 24 '23 16:04 amykglen

hey @edeutsch - could you roll out master when you have a chance, so we can do more testing? I fixed most of the issues we identified earlier (think I got everything except the missing EPC on XDTD KG2 edges, and the UI issue in #2032)

amykglen avatar May 17 '23 23:05 amykglen

master is now rolled out in /test, /devED, /devLM. But kg2beta is not

NewFmt and kg2NewFmt should be. Maybe in a state where you can test in NewFmt now? not sure.

edeutsch avatar May 17 '23 23:05 edeutsch

yes, thanks!

awesome, so it looks like I actually did fix the missing edge attributes on XDTD KG2 edges!

now I think just XDTD KG2 edges' sources need to be sorted out... I'm going to create a new issue for that one...

amykglen avatar May 18 '23 00:05 amykglen

This was completed.

edeutsch avatar Apr 16 '25 17:04 edeutsch