-
Notifications
You must be signed in to change notification settings - Fork 8
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
remove imports from go-cams #345
Comments
Should check that
|
@vanaukenk @ukemi could you take some time with models on noctua-dev to verify that all is good? I just made a change under the hood and want to make sure the engine is still running.. Thanks. |
@kltm everything seems to be good to go with this change on dev. Do you want to make the change to master so we can make everything consistent ? |
We'll need to coordinate time with @vanaukenk to stop the system, get the PR/update in, restart. |
@vanaukenk @ukemi
(- [ ] models are coherent in graph editor) Issues, if there are any, would likely be quite serious. |
Created basic complete model in graph editor that includes MF-BP-CC-CL-EMAPA. gomodel:5f4d6ed700000063 looks good and reasoner runs without crashing. Able to add and delete individuals. Used add annoton and add individuals. PASS. Created model in graph resulting in inference of regulation process. gomodel:5f4d6ed700000077. Inference is correct. Tested clone evidence. Model looks good. PASS Created model in graph using BP only. gomodel:5f4d6ed700000089. Could not use EMAPA:17412 in occurs_in statement from the 'ADD Process' menu. It appears that EMAPA is missing from the autocomplete from the 'Add process' menu. Although it is available from the 'Add Individual' menu. Should 'Add Process' occurs_in be restricted to Cellular Component? FAIL Created individuals with UBERON, EMAPA, ZFIN, XAO and WBbt anatomical structures. gomodel:5f4d6ed700000089. Correct inferences were generated from UBERON and WBbt, but not ZFIN, EMAPA or XAO. See the intestine terms and heart terms. FAIL I suspect the failures are due to lack of bridging axioms (@balhoff )? |
@ukemi it might be useful to chat on Skype about the anatomy bridging you were expecting. I thought those were set up so we might need something more thorough. |
Testing in the form: I am able to create a model with extensions that renders in the form and the graph. @tmushayahama One thing that is not working properly in the form on noctua-dev is 'Remove evidence' . This now deletes all of the evidence boxes instead of just the values within them. This does not happen on production. Based on our discussions yesterday, I don't think this would be a result of the minerva changes, but would like to confirm what will happen when we move to production. |
Either one is good for me. I thought the EMAPA inferences used to work. But I figured that since I was testing, I would run through a bunch. |
I have questions about the C. elegans cell and anatomy inferences, too, and there are still some issues with typing cell and anatomical entities for the purposes of the form and table. We're off-topic a bit for this ticket, I suspect, but let's try to schedule a time when we can discuss this with all necessary people on board. |
Sorry @vanaukenk. I wasn't sure exactly what these changes would affect. I figured that sine the ticket was called remove imports I should try using imported ontologies. |
No worries @ukemi You may well be right. |
@ukemi @vanaukenk whether terms appear in certain type-aheads is a separate issue from the minerva changes as the type aheads are controlled via the javascript and the SOLR service - they don't touch minerva. For the anatomy inferences, it would be helpful to know specifically what you were expecting and what actually happened. It would also be useful to see if the same thing happened on production versus master. What are the correct inferences that you are not seeing? For clarity, the 'remove imports' thing here does NOT mean that we should lose access to any terms or inferences within the noctua/minerva system. It is a change internally regarding how Minerva specifically handles the binding of ontology term definitions to instances in the models. It seems that we have made the mistake here of changing a lot of things at the same time.. It would have been nice to test the UI code changes independently of the server side changes. I suspect there are probably also ontology changes. So yeah.. an automated test suite that ran on top of the UI so the entire stack could be tested at once would be very awesome - perhaps it could be called necessary - and necessary to execute every time there is a merge to dev for any of the client or server code bases. |
@ukemi @balhoff and I discussed this morning some of the inference issues we're seeing and don't believe this relates to the minerva changes on this ticket. Wrt the UI changes, is it possible to roll those back so we can just test any possible impact due to the minerva changes? +1 for the automated test suite. |
From 2020-09-23 call: @tmushayahama will investigate the behavior with 'Remove evidence'; appears to be on the client side. |
The no imports noctua stack has been working fine for a while now. |
Following discussion with @balhoff , we concluded that we should:
The reasoning underlying this move:
Minerva handles reasoning over models using an independent, system wide configuration controlled globally as two start up parameters (1 ontology (for Arachne) and 2 ontology journal (for tbox queries and labels)). Having ontology imports declared in models can conflict with the system-level ontologies, causing confusion. By leaving them out, the user of the go-cam can decide what ontologies to import and when. e.g. this allows users to open go-cams in protege without editing a catalogue file. If users want to access to missing tbox information, they can always import the ontologies.
The text was updated successfully, but these errors were encountered: