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

Massive amount of unsats in cl-edit #1782

Closed
shawntanzk opened this issue Dec 23, 2022 · 22 comments
Closed

Massive amount of unsats in cl-edit #1782

shawntanzk opened this issue Dec 23, 2022 · 22 comments

Comments

@shawntanzk
Copy link
Contributor

shawntanzk commented Dec 23, 2022

@bvarner-ebi spotted that there are a lot of unsats in cl-edit.owl in master branch
interestingly explain_unsat isn't picking this up which is probably why QC didnt pick up on it
Here's an example from @bvarner-ebi
209308227-51e3ec28-f9e2-44d1-87c9-edcc751fbc0a

@bvarner-ebi suspects this PR in Ubeorn obophenotype/uberon#2695 (tagging @gouttegd)
Will try to look into this when I can, but any help is also welcomed

@gouttegd
Copy link
Collaborator

The (main? only? not sure yet) culprit seems to be results in developmental progression of (RO:0002295), which has a range restricted to connected anatomical structure (CARO:0000003) (which is itself DisjointWith disconnected anatomical group, CARO:0020000).

This restriction seems wrong. A developmental process can very well result in the development of an entity made of disconnected parts, not only of a connected structure. E.g., endocrine system development results in the development of the endocrine system, which is a non-connected functional system (it is made of endocrine glands dispersed throughout the body).

This range restriction should either be removed or at least replaced by a restriction to a higher class in the hierarchy, one that does not imply anything about “connectedness” (anatomical entity would be correct, I think).

@ghost
Copy link

ghost commented Dec 23, 2022

Thank you for taking a look, @gouttegd.

Reviewing related tickets I found this comment, where it was proposed that 'anatomical cluster' should be a 'disconnected anatomical group'.
I'm not sure this is accurate, but unsure how "connected" is defined.

@gouttegd
Copy link
Collaborator

I note that the definition of RO:0002295 does state that the relation applies (or should apply) to anatomical structures:

p results in the developmental progression of s iff p is a developmental process and s is an anatomical structure and p causes s to undergo a change in state at some point along its natural developmental cycle (this cycle starts with its formation, through the mature structure, and ends with its loss).

The range restriction is consistent with that definition, but then, if we follow that, the relation should not be used to describe the development of anatomical entities that are not anatomical structures. And again that seems wrong to me: I see no reason for limiting the use of that relation to the development of anatomical structures, especially since there are no equivalent relations to describe the development of disconnected anatomical entities.

@gouttegd
Copy link
Collaborator

Reviewing related tickets I found obophenotype/uberon#2682 (comment), where it was proposed that 'anatomical cluster' should be a 'disconnected anatomical group'.
I'm not sure this is accurate, but unsure how "connected" is defined.

I am not 100% sure either. As I wrote in that comment, it depends on how you interpret “adjacents” in the definition of anatomical cluster (“anatomical group that has its parts adjacent to one another”). I took it as not implying that the parts are connected, that is, they don’t have physical connection to each other.

(For reference, PATO defines disconnected as “a structural quality inhering in the bearer by virtue of the bearer consisting of multiple structures lacking any physical connection to each other”.)

@ghost
Copy link

ghost commented Dec 23, 2022

I took it as not implying that the parts are connected, that is, they don’t have physical connection to each other.

(For reference, PATO defines disconnected as “a structural quality inhering in the bearer by virtue of the bearer consisting of multiple structures lacking any physical connection to each other”.)

If parts are adjacent (which I interpret as sharing a physical boundary), I would argue that they are physically connected.

@ghost ghost added the help wanted label Dec 23, 2022
@gouttegd
Copy link
Collaborator

I won’t argue with that, feel free to revert that change if you want. (Please do let me know if you do, because that term is mapped to FBbt’s anatomical cluster, which is clearly defined as disconnected; if you re-classify it as connected, I will need to remove the mapping.)

I actually question the usefulness of anatomical cluster in Uberon, regardless of its definition, given the diversity of its subclasses: muscle spindle, pineal complex, upper urinary tract, vasculature … Seems like a mess to me: under which definition would the vasculature or the urinary tract be anatomical clusters ?

@dosumis
Copy link
Contributor

dosumis commented Dec 23, 2022 via email

@ghost
Copy link

ghost commented Dec 23, 2022

I actually question the usefulness of anatomical cluster in Uberon, regardless of its definition, given the diversity of its subclasses: muscle spindle, pineal complex, upper urinary tract, vasculature … Seems like a mess to me: under which definition would the vasculature or the urinary tract be anatomical clusters ?

Agreed. I cannot justify why these terms are 'anatomical clusters'.

Of note, removing the range 'connected anatomical structure (CARO:0000003)' from 'developmental progression of (RO:0002295)' fixes all unsats but one (a GO term: 'calcium activated cation channel activity').

@ghost ghost self-assigned this Dec 23, 2022
@ghost
Copy link

ghost commented Dec 23, 2022

Also makes sense to relax domain/range on the dev relation causing unsats.

@dosumis, should this change and new CL release wait until the new year?

@gouttegd
Copy link
Collaborator

all unsats but one (a GO term: 'calcium activated cation channel activity').

This one seems to be caused by regulated by (RO:0002334), which has both a domain and a range restriction to process (BFO:0000015) (so, a process is regulated by another process), where process is an occurrent.

calcium activated cation channel activity is defined as gated channel activity (a process, OK) that is regulated by some calcium(2+) (not a process).

shawntanzk added a commit to oborel/obo-relations that referenced this issue Dec 23, 2022
Change the range of RO to be anatomical entity instead
Related to obophenotype/cell-ontology#1782 (see CL ticket for reason)
@shawntanzk
Copy link
Contributor Author

calcium activated cation channel activity is defined as gated channel activity (a process, OK) that is regulated by some calcium(2+) (not a process).

I think this might be a RO issue, again I think the range needs to be relaxed too, given a process can be regulated by things other than a process (probably also means we should change the domain of regulates given that regulates by is defined by regulates)

@gouttegd
Copy link
Collaborator

@shawntanzk Not sure about that. regulated by is a subclass of causal relation between processes, so the range and domain restrictions are clearly not accidental – the relation is explicitly intended to be about processes on both ends. There is another set of relations intended for causal relations between a process on one side and a material entity on another side (RO:0002595 and its subclasses), the definition of calcium activated cation channel activity should probably use one of those instead.

@dosumis
Copy link
Contributor

dosumis commented Dec 29, 2022

calcium activated cation channel activity is defined as gated channel activity (a process, OK) that is regulated by some calcium(2+)

@gouttegd is correct - this is a GO error. GO should fix by changing to regulated by 'Ca2+ binding'.

@ghost
Copy link

ghost commented Jan 3, 2023

To revisit this issue, is the ideal approach as follows:

1: In CL, remove the range 'connected anatomical structure (CARO:0000003)' from 'developmental progression of (RO:0002295)'
2: Resolve geneontology/go-ontology#24605
3: Refresh CL imports and make a new CL release

For step 1, does this change need to happen elsewhere / outside of CL?
For step 2, can someone recommend a GO editor to ping?

Thanks again for the help with this.

@gouttegd
Copy link
Collaborator

gouttegd commented Jan 3, 2023

Step 1 should happen in obo-relations. There’s already a PR for that (oborel/obo-relations#659), once it’s merged (only after a discussion about it at the next RO meeting apparently, whenever that meeting may be) the fix will be brought to CL by refreshing the RO import.

@shawntanzk
Copy link
Contributor Author

whenever that meeting may be

luckily it is today :) I'll attend and try to make sure this gets merged in (don't think its super controversial). Once that happens, I'll try to do a release (if merged in this evening, will probably get it done by tmr).

@gouttegd
Copy link
Collaborator

gouttegd commented Jan 3, 2023

don't think its super controversial

Famous last words… :D

@shawntanzk
Copy link
Contributor Author

For step 2, can someone recommend a GO editor to ping?

@raymond91125 was wondering if you could help with this (see geneontology/go-ontology#24605) if not could you suggest someone that can help us with this please? thanks heaps :)

@shawntanzk
Copy link
Contributor Author

GO issue should be solved in this PR geneontology/go-ontology#24610 (see this ticket geneontology/go-ontology#24605), will probably just have to refresh import after next GO release :)

@shawntanzk
Copy link
Contributor Author

RO issue solved too :D will make a RO release tmr -> want to wait till after RO meeting today to let anyone get their stuff in and all

@ghost
Copy link

ghost commented Jan 10, 2023

With the new CL release, the only unsatisfiable class is now GO:0005227 'calcium activated cation channel activity'. The GO term update appears to have been merged. This last unsatisfiable class should be resolved once there is a new GO release and it is imported to CL.

@ghost
Copy link

ghost commented Mar 15, 2023

Now that GO has a new release release and imports have been refreshed in CL, there appear to be no more unsats in cl-edit.

Thanks to all for the help.

@ghost ghost closed this as completed Mar 15, 2023
This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants