geneontology / minerva

BSD 3-Clause "New" or "Revised" License
6 stars 8 forks source link

Minerva should include BioLink types withing all return payloads #217

Closed kltm closed 3 years ago

kltm commented 5 years ago

To support use cases like geneontology/noctua#165 (e.g. type lookup table), Minerva should include this information, hinted by file taken at startup (or similar), in all relevant payloads. Should only be additional information.

This will possibly require the reasoner to be permanently on.

Tagging @cmungall @balhoff for comment

kltm commented 5 years ago

This will also be useful for @tmushayahama and entity identification.

kltm commented 5 years ago

Should this be in a project?

goodb commented 4 years ago

After working on https://github.com/geneontology/minerva/issues/212 I can see where all the super types for each instance in a model could be added to the response easily. Do we want biolink types here as the ticket says or are the upper level ontology terms preferred - as seems to have been the decision for the shex work?

This does depend on the reasoner being on. Suggest that we should go ahead and make reasoner = on the default and perhaps only option. It seems to work fine. If it does cause problems in edge cases, then turning it on is the best way to find them.

tmushayahama commented 4 years ago

@goodb will that affect performance?

goodb commented 4 years ago

server response times could be slower - mainly for larger models.

tmushayahama commented 4 years ago

I explained it on here https://github.com/geneontology/minerva/issues/231#issuecomment-522847828 . I think having all these extra things via api or additional calls might be reasonable (no pun intended) . I feel like now the initial minimal m3Batch response is okay

kltm commented 4 years ago

@goodb From our conversation earlier today, it seems like one more reason for the reasoner to be default on.

goodb commented 4 years ago

@kltm @tmushayahama @lpalbou is this still a problem and is it still the plan?
I haven't heard anything about the use of BioLink within the Noctua/Minerva context since last summer. If this is no longer the plan, please close the ticket and make a new one with the current plan for getting upper-level entity types to the client (if there is one.). If it is the plan, then let me know so I that can make it happen.

tmushayahama commented 4 years ago

@goodb I think this will be a life saver and solve so many problems arising, but not a blocker if you have other things. The work around being used in NF is not ideal. I think we started discussing about making Golr queries https://github.com/geneontology/noctua-form/issues/73 efficient, but it is just building the solution on a poor foundation of NF. It might be helpful if I make some diagrams/presentation to see how NF is getting the types.

goodb commented 4 years ago

@tmushayahama I'm pretty sure we want to give you access to entity types via one of the minerva APIs. I'm just not clear how we want that to happen. e.g. are they biolink types or are they the corresponding ontology classes. and how do we want the api to deliver this information - e.g. separate entity typing service or adding them into m3batchresponse payload (the way they are now when the reasoner is on) or something else. This is pretty fundamental.

I need input here from @kltm and @cmungall .

kltm commented 4 years ago

@goodb Let's try and get a few minutes with @cmungall in the next few days to sort out if there are strategic needs here or some other reason not to align with shex.

tmushayahama commented 4 years ago

@goodb @kltm @vanaukenk I have been thinking about this. Now everything seems to align with shex, Although nice to have, but there might be no reason to implement it just for the form. It will be much useful to share resources (algorithms, implementations, APIs) because pretty much types are used to achieve the same thing in importer, shex, noctua form, validator, reasoner. I can make little diagrams and pinpoint use cases in NF for types.

However on an off topic @kltm it will be nice to start discussions on how to "fold" (more than by predicates) response graphs into shex shapes specifically activity units, but thats another topic. Then types will not be needed, only for autocomplete.

goodb commented 4 years ago

@kltm I don't think there is any argument that says we should not be aligning with shex. Its more a matter of how that information - specifically the most upper level semantic type of each node in a model - is accessed by the UI. From the server perspective, we are good, we access it using the ontologies loaded into the reasoner and, now, the neo entities loaded into the local blazegraph.

tmushayahama commented 4 years ago

The following comment might be in a long run and don't wanna pollute this issue. But wanted to mention a workaround potential.

@goodb that brings to my off topic point about folding Example. here the bbop-graph representation is folded into subgraph but not any useful shape.

image

But, a few tweaks can fold it into useful shapes like activity units, bp only etc. eg image

There is not much use cases of needing dynamic node types or closure info in the UI except for this loading part to chunk/fold graph into activity unit shapes (graph model into sub-graphs model). In other words, the Noctua Form Code which makes those subgraphs in the table can live in the server side and I can use all the benefits of locally loaded blazegraph, shex, validators then minerva will just bring already folded graphs. Therefore types will not be needed much