Migration to TRAPI 1.4
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.
Branch NewFmt is now completely synced with master
@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, 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?
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)
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
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!
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
oops, forgot to commit that one. just pushed. try again?
fixed, thanks!
@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.
Maybe, I really don't understand these parts of the system sufficiently to have a recommendation.
a few notes for the ARAX implementation. we need to remember to:
- [x] update Overlay to use the new
edge_bindingslocation - [x] update Overlay to add knowledge source information into the new
Edge.sourcesstructure (instead ofEdge.attributes) - [x] update Filter to use the new
Edge.sourcesstructure - [x] update Filter to use the new
edge_bindingslocation - [x] add a new 'transform results' step (to be called after ranking), that removes virtual edges from
Analysis.edge_bindingsand stuffs them intoAnalysis.support_graphs
another thing to remember to do:
- [ ] Expand should be updated to consider
MetaEdge.qualifierswhen it's selecting KPs based on their meta knowledge graphs (for queries that involve qualifiers)...
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 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.
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...
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)
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.
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...
This was completed.