Skip to content
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

Add/expose go-cam reasoning/validation service #200

Closed
goodb opened this issue Aug 29, 2018 · 16 comments
Closed

Add/expose go-cam reasoning/validation service #200

goodb opened this issue Aug 29, 2018 · 16 comments
Assignees
Labels

Comments

@goodb
Copy link
Contributor

goodb commented Aug 29, 2018

Input is an OWL model representing a GO-CAM (expressed using the json format preferred by the Noctua stack)
Output

  1. boolean is_valid (check OWL consistency)
  2. list of errors if any (nodes that = OWL:Nothing
  3. Any explanations of the errors that can be provided
  4. Inferred types (all the way up to root) for each node in the model

@kltm @tmushayahama @lpalbou @cmungall

@goodb
Copy link
Contributor Author

goodb commented Aug 29, 2018

@tmushayahama needs tp get the inferred types for each of the nodes in the model.

@goodb
Copy link
Contributor Author

goodb commented Aug 29, 2018

assign to @goodb

@goodb
Copy link
Contributor Author

goodb commented Aug 29, 2018

@kltm to help set up testing environment (call the service from node libraries used by actual client code (probably using command line interface) ). Provide specifications for request/response structure.

@goodb goodb changed the title Add/expose go-cam validation service Add/expose go-cam reasoning/validation service Aug 30, 2018
@goodb
Copy link
Contributor Author

goodb commented Aug 30, 2018

@kltm @balhoff I think I must have missed something when we discussed this. Why can't @tmushayahama call the existing reasoning service from the form? It seems like that would be sufficient for his needs - with the addition of returning the root type (MF, BP, CC, etc.) for each individual.

@vanaukenk
Copy link

One of the requests we've had from curators is to provide some explanation of what is invalid in a model when they use the reasoner and the screen turns red. Currently, there is little, if anything to provide hints. Could this service be used to help with this?

@cmungall
Copy link
Member

cmungall commented Sep 4, 2018

Of course, anyone can use the inference explanations view but this is probably really appropriate for ontology editors who have undergone OWL training.

I believe we can iterate over this, adding server side functionality that will look at patterns in the explanation and summarizing these in a curator friendly way. As an example, as a first pass the server would look at the explanation axioms and see if one looks like a TC. If so, it can inject an explanation text into the payload that says "it looks like TC was violated somewhere".

This same server-side logic could potentially be reused e.g. in gaferencer (when processing non-GO-CAM submitted GAFs) (cc @dougli1sqrd )

@goodb
Copy link
Contributor Author

goodb commented Sep 4, 2018

I think the UI for explaining errors (or correct inferences) could be significantly improved without any immediate updates to the server side. One of the problems with the explanation view is searching through it to find the which nodes are inferred to be OWL:Nothing. Reading about why that inference happened is a second step. I suggest:

  1. Have the UI visually highlight any nodes with type OWL:Nothing
  2. For any inferred type (including Nothing), link directly to the explanation for that particular inference (isolated from the rest of the explanations).

The first could be done now. The second may require some minor adjustments to the way the explanation data is sent back, but not much.

From that point, we could iterate on the text of the explanations.

@cmungall
Copy link
Member

cmungall commented Sep 5, 2018

I think @goodb's suggestions make sense. How much work would it be? And if the answer is that this would take a lot of plumbing and UI work, it's not clear we should invest the time vs spending time on a more curator-focused interface.

@kltm
Copy link
Member

kltm commented Sep 5, 2018

Re: #200 (comment)
As a practical matter, it may be easier to spitball what the API looks like in the "How the Minerva server works" doc, especially the return.
For the use case of testing using noctua-repl, commands for tests on current models should like get_model('gomodel:XYZ') should work.
I will need to quickly write a serializer to make the test environment work out; barring interruption, I should have that in about a day.

@kltm
Copy link
Member

kltm commented Sep 5, 2018

@cmungall UI things like that would not be a lot of work, but it would be enough to slow things down, even if other things were dropped and it was focused on. That said, simple things like highlighting and color changes are approachable.

@goodb
Copy link
Contributor Author

goodb commented Sep 5, 2018

@kltm if you want to define the inputs and expected outputs for the service as simple blobs of json that would probably be sufficient to get the core of the service working, though of course running more realistic tests directly from the client API would be best. I have a test built now that uses the Java/gson class that @cmungall wrote to generate requests but it would be better to be working from a json structure that was already understood by the client to start out with.

That being said.
Given the bandwidth limitations we are operating under I think being clear about why or why not this is needed right now, as I asked about in the Noctua chat room, is very important. Right now I see interesting use cases for a reasoning service that was to some extent independent of the rest of the Noctua/Minerva stack but I don't see any clear case for why this is crucial in the @tmushayahama form-editor use case that initiated the idea. If he made use of the existing service it already provides the core information he needs - and if it needs to extended, for example by appending the root inferred classes (MF, BP, etc.) to the output, that would be a straightforward addition.

In fact, if he started developing with this new service, which operates quite differently from the rest of the stack (whole models versus atomic actions on models), it could actually lead to greater splintering and more wasted resources moving forward.

@kltm
Copy link
Member

kltm commented Sep 6, 2018

@goodb Sorry, but I thought it might be good to retread some of the gitter comments before they get lost in the timeline, just to give some context.

The first use case--the one that we may have outlined more often--is checking a model after an operation has been performed. This would (eventually) include structural checks, GORULEs, etc., that could be run at a decent speed on every edit, with the responce being send back as part of the standard edit packet.

The second use case is for proactively preventing curators from entering inputs that should be illegal for some reason, before they attempt to edit a model. This is the use case that was outlined for @tmushayahama , but could be applied elsewhere. One would essentially have a model (or build a simple model), make changes to it, and ask if the resulting model would be okay. The temporary model (or model bits) would never be saved or made available to others. The checks for this and the first use case would probably have overlap and differences.

(Apologies for retreading that, I just wanted to make sure that future readers don't get lost in the chain of thought.)

If I'm following you (and I may not be), if we had a separate reasoning service, it would be serving very much the same information as the after-the-fact system that would need to be in minerva to supply checks anyways. But there are other ways forward; I guess the general architectures could be:

  • all reasoning/validation in general-purpose service, no reasoning/validation in after-the-fact minerva
  • no general-purpose service, all reasoning/validation in new minerva route and after-the-fact minerva
    • what we have now is this, minus the new route
  • a mix of the first two, where we have a general-purpose service for some things and minerva provides some others

If there is interest in providing a public general-purpose service (good!), that would be a viable way forward. I would guess that it would probably share a lot of the same code (if we had both), but incur some additional overhead (bad!). I would be interested in the scope of what additional things may be offered to the public, but as I am not a consumer of such services, I can't really weigh in on how much marginal utility there might be in making something like that available to the community, versus continuing on the current path.

@goodb
Copy link
Contributor Author

goodb commented Sep 6, 2018

Though I have long bothered @balhoff about making a public general purpose reasoning service for the GO (and its brethren) I don't think it is the right time to do it. I think that right now we have an acute need for iterative progress on the Noctua front end (which IMO should be considered to directly include the form input editor, the table-based review system and anything else that is built for curators working on GO-CAMs). After coming to a better understanding of how Noctua/Minerva works, I think the best way that server-side development can help move Noctua forward right now is to develop incremental expansions to the existing service (as there is really only one Minerva service). Such expansions should be driven very specifically by clearly articulated needs in the client code driving the UI.

I think it would be helpful if @kltm or @tmushayahama could define, with sample inputs and outputs, what is needed by the Noctua client (broadly speaking) that is not currently accessible from an existing call to Minerva with the Reasoner on. This is not clear to me from your comment above.

@kltm
Copy link
Member

kltm commented Sep 7, 2018

@goodb I fully agree. I think that at this point in time it's probably better to evolve what we have and revisit (and possibly split out) when we have worked with it a bit more.
I'll hopefully get back to the client serializer we need tomorrow.

@kltm
Copy link
Member

kltm commented Sep 13, 2018

Also see geneontology/noctua#567

@kltm
Copy link
Member

kltm commented Sep 14, 2018

My understanding is that as we are no longer implementing a new endpoint for this use case, instead evolving the response of the current system, this can be mothballed for now.

@kltm kltm closed this as completed Sep 14, 2018
@kltm kltm added the wontfix label Sep 14, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants