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

Taxon info displays "none" when there are constraints #712

Open
suzialeksander opened this issue Jan 24, 2024 · 24 comments
Open

Taxon info displays "none" when there are constraints #712

suzialeksander opened this issue Jan 24, 2024 · 24 comments

Comments

@suzialeksander
Copy link

AmiGO displays no taxon info: https://amigo.geneontology.org/amigo/term/GO:0007117
Screenshot 2024-01-24 at 12 04 30

But QuickGO shows the expected constraints: https://www.ebi.ac.uk/QuickGO/GTerm?id=GO:0007117#term=history
Screenshot 2024-01-24 at 12 27 57

Shouldn't AmiGO be loading these too?

@kltm
Copy link
Member

kltm commented Jan 25, 2024

Related to #56

@suzialeksander Good catch. I'm not sure why this is not showing up in this case, but still "works" in others (e.g. https://amigo.geneontology.org/amigo/term/GO:0000165).

@balhoff Would you know off the top of your head about how our filters or propagation might be different? If not, I can start working backwards. I would note that the constraints may be relatively new (Nov).

@balhoff
Copy link
Member

balhoff commented Jan 25, 2024

@kltm I see these constraints in snapshot go-plus (2024-01-12).

@kltm
Copy link
Member

kltm commented Jan 25, 2024

Thank you @balhoff !
Hm. This may be an issue with something in owltools then (hoping not).

@kltm
Copy link
Member

kltm commented Jan 25, 2024

Okay, I don't think this is getting through to the AmiGO side. @balhoff would you happen to see any difference between GO:0000165 and GO:0007117? I'm really hoping this isn't owltools, but am wondering if there might be a hard filter for relations in there, only considering only_in

@balhoff
Copy link
Member

balhoff commented Jan 25, 2024

@kltm which relations are you displaying there? I recommend RO:0002162 and RO:0002161.

@kltm
Copy link
Member

kltm commented Jan 25, 2024

The thing is: I'm basically scanning for an NCBITaxon and just taking whatever comes up and passing it through. I've now traced things most of the way back through AmiGO and haven't hit anything yet. If you can't see anything on your side, next is getting into the index itself and seeing if I can see anything in the raw data underneath.

@balhoff
Copy link
Member

balhoff commented Jan 25, 2024

@kltm for this term the only constraints that are directly on it are never in taxon. That relation is different from in taxon in that it's an annotation property rather than an object property. So maybe it's not getting indexed in Solr? I modified this URL from one of the earlier tickets, and I don't see it: https://golr-staging.geneontology.io/solr/select?defType=edismax&qt=standard&indent=on&wt=json&rows=10&start=0&fl=*,score&facet=true&facet.mincount=1&facet.sort=count&json.nl=arrarr&facet.limit=25&hl=true&hl.simple.pre=%3Cem%20class=%22hilite%22%3E&hl.snippets=1000&fq=document_category:%22ontology_class%22&q=id:%22GO:0007117%22

@suzialeksander
Copy link
Author

Another example is GO:0009764 PEP carboxykinase C4 photosynthesis which should inherit 4 never_in and 1 only_in [we use in_taxon I think(?)].

https://amigo.geneontology.org/amigo/term/GO:0009764
https://www.ebi.ac.uk/QuickGO/term/GO:0009764

@kltm
Copy link
Member

kltm commented Jan 31, 2024

@balhoff @suzialeksander For https://amigo.geneontology.org/amigo/term/GO:0009764, I would not necessarily expect a taxon constraint to be listed here, as it's not direct, but ancestor GO:0009760. @balhoff I wanted to confirm that I should not expect propagation here? https://amigo.geneontology.org/amigo/term/GO:0009760 shows for us (expected).

@suzialeksander
Copy link
Author

As a user I'd expect to see any TCs that apply to that term, direct or inherited. "Taxon info: none" implies there are no TCs

@kltm
Copy link
Member

kltm commented Jan 31, 2024

I can confirm using luke that this taxon constraint info for GO:0007117 does not appear in the underlying data. If the information is properly in the ontology, this pushes the issue to either sync (the changes were somehow too "new" to get out in the release) or owltools (bleh).

@kltm
Copy link
Member

kltm commented Jan 31, 2024

Looking at GO:0000804; from QuickGO:
2017-01-21 Added CONSTRAINT never_in_taxon NCBITaxon:40674 (Mammalia)
This should be old enough to have propagated. Using luke, I'm not seeing it in the latest release. This should eliminate one possibility: we're now down to issue in ontology or issue in owltools

@kltm
Copy link
Member

kltm commented Jan 31, 2024

A quick scan for variations of "never in taxon" pops these three files in owltools (modification from 6 to 10 years ago):
https://github.com/owlcollab/owltools/blob/master/OWLTools-Annotation/src/main/java/owltools/gaf/inference/TaxonConstraintsEngine.java
https://github.com/owlcollab/owltools/blob/master/OWLTools-Runner/src/main/java/owltools/cli/TaxonCommandRunner.java
https://github.com/owlcollab/owltools/blob/master/OWLTools-Core/src/main/java/owltools/mooncat/SpeciesSubsetterUtil.java

@balhoff I don't suppose you see anything obviously wrong here? Coming into the code the other way:

  1. It's in GOlr field neighborhood_graph_json
  2. Produced in owltools from getNeighborsJSON
  3. Calls getNeighbors
  4. Calls createNeighbors
  5. Gets into the weeds: https://github.com/owlcollab/owltools/blob/9faa4f42b761839a26e8c8096cd24044e2bdcfc7/OWLTools-Core/src/main/java/owltools/graph/OWLGraphWrapperEdgesAdvanced.java#L1219

While some of the variable names smell like mine, the code is more OWLish than I can. I don't suppose you could quickly eyeball that as well for obvious issues?

If there is nothing that we can easily find here, I'm tempted to do as discussed with @pgaudet and withdraw this feature.

@kltm
Copy link
Member

kltm commented Feb 13, 2024

@balhoff I was wondering if you could just briefly look at what I have above and see if you notice anything obvious? If not, we'll be withdrawing this feature.

@kltm kltm moved this from To do to In progress in Software essential and proactive maintenance Feb 13, 2024
@balhoff
Copy link
Member

balhoff commented Feb 14, 2024

I think the loader code for ontology terms is here: https://github.com/owlcollab/owltools/blob/master/OWLTools-Solr/src/main/java/owltools/solrj/OntologySolrLoader.java

It doesn't appear to add any annotation property values besides label and definition and a few other things here: https://github.com/owlcollab/owltools/blob/9faa4f42b761839a26e8c8096cd24044e2bdcfc7/OWLTools-Solr/src/main/java/owltools/solrj/OntologySolrLoader.java#L62-L88

Since never_in_taxon is an annotation property, it wouldn't get loaded along with the "real" semantic relations (part_of, regulates, etc.).

If we want this we might need to tweak that code (and I guess the Solr schema?).

@kltm
Copy link
Member

kltm commented Feb 14, 2024

@balhoff Yes, code tweaks are going to slide this from "issue" to "project", as we have to dig up docker and image changes, as well as the basic code and schema changes. (It also means that new versions of AmiGO wouldn't work on older indexes, etc.).

You mentioned on the slack that if "never_in_taxon is added as a relation, it may have unintended traversal consequences (it isn’t an existential restriction like the other relations )" and that "what you have is right for 'only in taxon'". Perhaps we could change the wording or display of the feature, so keep at least some of the functionality, and save the rest for later? Tagging @pgaudet .

@kltm
Copy link
Member

kltm commented Mar 18, 2024

From @pgaudet , it needs to be all in or all out--it cannot be partial.

@kltm
Copy link
Member

kltm commented Mar 18, 2024

Also noting for @suzialeksander that we are temporarily removing this feature.

kltm added a commit that referenced this issue Mar 18, 2024
@kltm
Copy link
Member

kltm commented Mar 18, 2024

@pgaudet @suzialeksander taxon info now suspended until further work done.

@cmungall
Copy link
Member

There is no need to touch owltools.

Here is the fix

STEP 1:

Add a never_in_taxon_field in the golr config https://github.com/geneontology/amigo/blob/master/metadata/ont-config.yaml

- id: never_in_taxon
    description: Term that replaces this term.
    display_name: Replaced By
    type: string
    property: [getAnnotationPropertyValues, RO:0002161]
    cardinality: multi

(am 99% sure this will work)

This will populate a new field in solr

STEP 2:

In AmiGO this can be queried just like any other solr field, say, for example alt_ids:

<!-- Alternate IDs -->
<dt>Alternate IDs</dt>
[% al = TERM_INFO.alternate_ids %]
[% IF al.size == 0 %]
<dd>None</dd>
[% ELSE %]
[% IF al.size != 0 %]
<dd id="syn-collaspe-alt" class="syn-collapsible">[% al.join(", ") %]</dd>
[% END %]
[% END %]

It is mildly awkward how the template logic is a little different for negative vs forward taxon constraints -- but this reflects the OWL modeling that we are using in GO and other ontologies, for better or worse

Both are super-minor changes

@kltm

This comment was marked as duplicate.

@cmungall
Copy link
Member

For information purposes, here is some background. It's not necessary to understand this background, all that needs to be done for this ticket is my 2 steps above.

For modeling taxon constraints in OWL the community has gone back and forth over different ways to capture the logic of these constraints directly in OWL. It turns out that this is actually somewhat hard, as there are nuanced issues, and engineering considerations based on whether reasoners will handle the expressions. I can point at various long running tickets where people go in circles about this sort of thing. TL;DR the way we do it in GO and many other ontologies is to model the positive constraint as a "logical axiom" similar to any other edge like part-of, and to model the negative constraint as an "annotation assertion" which is formally logically silent, but which we expand to more complex OWL axioms when we need to. These annotation assertions are more typically used for things like linking a GO term to an issue in an issue tracker. It may seem odd, but those are the reasons.

The AmiGO/owltools loader has different logic for dealing with logical axioms and annotation assertions. This makes sense. You don't want to treat links between terms and github issues the same way you treat inter-ontology links. However, in this case need to introduce a little bit of asymmetry into the handling of both cases in the loader and the amigo templates in order to account for how it's modeling in OWL.

@kltm
Copy link
Member

kltm commented Mar 20, 2024

Thank you for the clarification here, @cmungall

To fill this out a bit more:

  • update the ontology metadata YAML (as above)
  • update and push the new loader image
  • update the "production" pipelines to point to these image
  • propagate the new index out to production
  • (IIRC we're getting the whole doc in amigo) thread through the additional variable to get this to the templating system
  • add template bits

@pgaudet
Copy link

pgaudet commented Apr 8, 2024

@kltm Should we close this ? Since we removed TCs from AmiGO?

@kltm kltm added this to TODO in AmiGO 2.7 via automation Apr 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
AmiGO 2.7
  
TODO
Development

No branches or pull requests

5 participants