-
Notifications
You must be signed in to change notification settings - Fork 30
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 taxslim-disjoint-over-in-taxon.owl
to the pipeline
#2928
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it looks good.
So glad to see the enhanced taxon constraint checks are working! These problems seem to be due to: That statement is wrong; Sauropsida includes birds. |
reports/taxon-constraint-check.txt content
UNSAT: ZFA:0005536 ZFA:0005536 |
can you make explanations for the remaining violations? My hunch is that first ZFA fontanel one is to do with sutures |
The explanations are here |
reports/taxon-constraint-check.txt content
UNSAT: FMA:54687 FMA:54687 |
We need to figure a way to work through these more efficiently The first example is interesting - we could go down the rabbit of hole of what constitutes a tail vs a vestigial tail vs an embryonic bud that fails to develop in human... but we should make fast decisions. I would just weaken the equivalence to FMA to a closeMatch. Mapping to non-human parts of FMA has very little value. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See comment below. I don’t understand why we don’t import taxslim-disjoint-over-in-taxon.owl
directly instead of importing taxslim.owl
and then injecting the missing disjointness axioms at the beginning of the pipeline in a somewhat sneaky way.
Fix the conflicts in uberon.Makefile caused by the overhaul of that Makefile (which occured while this PR was under way).
Fixed the merge conflict, the branch now applies cleanly to master. There are still some unsatisfiable classes in the taxon constraint check, full explanations attached |
Good points, I believe this step in the pipeline predated the availability
of the disjoints upstream
…On Tue, Sep 5, 2023 at 3:53 AM Damien Goutte-Gattat < ***@***.***> wrote:
***@***.**** requested changes on this pull request.
See comment below. I don’t understand why we don’t import
taxslim-disjoint-over-in-taxon.owl directly instead of importing
taxslim.owl and then injecting the missing disjointness axioms at the
beginning of the pipeline in a somewhat sneaky way.
------------------------------
In src/ontology/uberon.Makefile
<#2928 (comment)>:
> @echo "STRONG WARNING: issues/contributor.owl needs to be manually updated."
$(ROBOT) merge -i $< \
-i $(COMPONENTSDIR)/disjoint_union_over.ofn \
-i issues/contributor.owl \
+ -i $(TMPDIR)/taxslim-disjoint-over-in-taxon.owl \
I have two fundamental problems with the approach here.
First, we inject a ~100 MB component freshly downloaded from the Internet
right at the beginning at the pipeline. If that component is supposed to be
part of all products, then it should be made into a *proper* component
(declared as such in the ODK config, and mostly managed by the ODK) instead
of being sneakily downloaded and injected in the custom Makefile.
Second and most importantly, we are already (properly) importing
http://purl.obolibrary.org/obo/ncbitaxon/subsets/taxslim.owl, which in
this very step is merged (along with the other imports) into the pipeline.
The only difference between taxslim.owl and
taxslim-disjoint-over-in-taxon.owl is that the second contains
disjointness axioms.
So, we are explicitly declaring an import of a version of NCBITaxon
without taxon disjointness axioms, and then at the same moment as we merge
the imports, we inject the disjointness axioms that are missing from the
declared import… What is the rationale here? Why not importing
taxslim-disjoint-over-in-taxon.owl directly?
—
Reply to this email directly, view it on GitHub
<#2928 (review)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAAMMONJXUOJBQ7QLS5NHUTXY4AD5ANCNFSM6AAAAAAZL4EVPA>
.
You are receiving this because you commented.Message ID:
***@***.***>
|
Either we should have another term to represent more generic cortex-like structures beyond mammals, or we should remove (or relax) the mammalian restriction on neocortex. |
HBA:4413 is “lateral nucleus of the pulvinar, left”, a regional part of brain. The mapping with Uberon’s “posterior lateral line” is clearly bogus and should be removed. |
The notochord is present in all chordates by definition, so any chain of axioms that restricts its existence to vertebrates is wrong. The problematic link is here, I believe: axial skeletal system has part some axial skeleton plus cranial skeleton The latter term is textually defined as the “subdivision of skeleton which consists of cranial skeleton, set of all vertebrae, set of all ribs and sternum” (emphasis mine). That definition makes it vertebrate-specific, so the (non-vertebrate-specific) axial skeletal system should not have as part something that is vertebrate-specific. |
Not sure here. Either
|
AEO’s organism surfacef (sic) is mapped (and therefore, EquivalentTo) to both anatomical surface region and integumental system, making the two Uberon terms equivalent by transitivity. The mapping with integumental system is clearly bogus and should be removed. |
The website given as reference for the existence of the basement membrane of epithelium in homoscleromorphs clearly states that “the homoscleromorphs are unique among sponges in having a basement membrane and, therefore, a true epithelium.“ On that basis, the taxon constraint that restricts epithelium to Eumetazoa should be relaxed. |
A “terminology note“ on skull explains that
Assuming this note is correct and that Uberon follows that distinction between “skull” and “cranium” (where “skull“ = “cranium” + “mandible”), it is then an error to say that hagfishes have a “skull” – they should be said to have a “cranium” instead. So the “present in taxon some Myxinidae” axiom above should be on cranium, not on skull. |
Judging from this abstract, the taxon constraint can be relaxed to all cartilaginous fishes (Chondrichthyes), not just Elasmobranchii. |
Good catch
This is an example we see a bit where we have a choice between 1. Losing a
useful inference in many species for the sake of one; 2. Making an awkward
class for the one species and leaving the others alone
We often do 1 when we should do 2. In this case the element would be
curiously floating with no part parent
In this case 2 would involve creation of a class like rudimentary vertebral
element OR removing the TC and implying the series of vertebral elements
forms a rudimentary vertebral column.
…On Fri, Sep 22, 2023 at 11:59 AM Damien Goutte-Gattat < ***@***.***> wrote:
- Vertebra cartilage element
<http://purl.obolibrary.org/obo/UBERON_0011094> is present in taxon
<http://purl.obolibrary.org/obo/RO_0002175> some Eptatretus burgeri
<http://purl.obolibrary.org/obo/NCBITaxon_7764> (inshore hagfish)
- but is a vertebral element
<http://purl.obolibrary.org/obo/UBERON_0010913>
- which is part of <http://purl.obolibrary.org/obo/BFO_0000050> some vertebral
column <http://purl.obolibrary.org/obo/UBERON_0001130>
- which is never in taxon <http://purl.obolibrary.org/obo/RO_0002161>
some Myxinidae <http://purl.obolibrary.org/obo/NCBITaxon_7762>
(hagfishes).
It is apparently well established that hagfish have at least “vertebral
rudiments” even though they lack a *bona fide* vertebral column.
Given that we have both:
- vertebral element <http://purl.obolibrary.org/obo/UBERON_0010913> is part
of <http://purl.obolibrary.org/obo/BFO_0000050> some vertebral column
<http://purl.obolibrary.org/obo/UBERON_0001130>
- vertebral column <http://purl.obolibrary.org/obo/UBERON_0001130> is composed
primarily of <http://purl.obolibrary.org/obo/RO_0002473> some vertebral
element <http://purl.obolibrary.org/obo/UBERON_0010913>
A possible fix would be to keep only the second of those axioms, so that
the existence of a vertebral column implies the existence of vertebral
elements but not the other way round.
—
Reply to this email directly, view it on GitHub
<#2928 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAAMMOPU2PGIYDYC3TRQQLLX3XN2RANCNFSM6AAAAAAZL4EVPA>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
This is a simple mapping error. In Uberon lateral tuberal nucleus is explicitly intended to refer to a structure in human and higher primates, there is a distinct term intended to refer to a similar structure in rodents: tuberal nucleus (sensu Rodentia), where a comment explains:
Of note, this erroneous mapping comes from one of the custom bridges we have with the Allen Mouse Brain Atlas, which are maintained elsewhere and simply used as they come in Uberon. So the fix needs to happen in the remote source. |
I am not sure what this “oral subdivision or organism” is. The definition says
but I don’t understand what “oral to the plane” means. But I think a possible fix is to remove this: mouth overlaps some respiratory system mouth is seemingly intended to apply broadly (it is taxon-constrained only to Eumatazoa), so it seems wrong to assert that it necessarily overlap with (and thus, implies the existence of) a respiratory system. |
A self-contained one:
If that structure does exist in salamanders (the axiom asserting that references this book: https://isbnsearch.org/isbn/0471888893, that I cannot check), then the taxon constraint should be relaxed to one level higher (Tetrapoda), so as to cover both Amniota and Amphibia. |
Those were all the unsats detected by the new check enforcing taxon constraints. I’ll start fixing them in the second half of October. Of course if anyone wants to start before that, you’re more than welcome to do so. Note that one fix must happen in CL, and another one must happen in ABA_Uberon (where the custom bridge between Uberon and the Mouse Brain Atlas is maintained). All other unsats are fixable directly within Uberon. |
Thank you so much for all the explanations, @gouttegd. I'll create some tickets from your comments that might be related to HuBMAP and assign them to @aleixpuigb. |
@gouttegd oral–aboral is a body axis used in various invertebrate groups that don't have bilateral symmetry, such as echinoderms. See https://www.ebi.ac.uk/ols4/ontologies/bspo/classes/http%253A%252F%252Fpurl.obolibrary.org%252Fobo%252FBSPO_0000198 I think “oral subdivision or organism” should be “oral subdivision of organism”. |
At the Uberon call on 16 October 2023, it was decided that it would be better if the disjointness axioms between taxa were actually included in the released artefacts, instead of being only present during the QC pipeline to enforce taxon constraints. The recommendation is therefore to make a SMLE module out of the The straightforward way to do that would be to edit the ODK configuration so that we import Except that I tested doing just that, and it does not work. :( What’s extracted from I am not sure why, but I suspect this is because the various macros that expands to Not sure yet how to best fix this. Clearly we can’t use the standard, ODK-generated import pipeline; we need an ad-hoc rule specifically for the I am not keen on delaying this PR until we come up with a satisfactory way of ensuring both that the taxon constraint check works and that the disjointness axioms are present in released artefacts. I suggest that for now we proceed with what @anitacaron has done (merging the disjointness axioms only during the QC pipeline) so that we can at least have a working taxon constraint check – which will already be a strict improvement over the current situation. Later, in a distinct PR, we can try to improve further by making sure the disjointness axioms are also included in release products. |
Regarding this unsat: Fixing it by moving the “present in taxon some Myxinidae” axiom to cranium, as proposed in the linked comment, is not enough, because cranium
Since cranium exists throughout all craniates, it should not be said to be part of some skull which is (through skeleton of lower jaw) specific to jawed vertebrates. |
Regarding this one: Even if we relax the taxon constraint on dermatocranium as suggested, there is still another path to unsatisfiability, because external naris
For this one I don’t have a fix that I would be really happy with. I am considering removing nasal skeleton SubClassOf: part of some facial skeleton to break the link that ultimately makes the nose specific to jawed vertebrates, which seems to me the “least bad” fix. |
This one also has another path to unsatisfiability, even after removing the link between mouth and respiratory system. That’s because oral subdivision of organism
An homology note on mouth states that
which suggests to me mouth should not be part of some head, since the head only exists in Bilateria. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is great, I approve of both ontology changes and all pipeline changes.
I agree with @gouttegd's course of action on 1 vs 2. And keeping the original inference as a taxon GCI makes it easy to review these all en masse at some future date
Superseded by #3102, which has now been merged. |
Fixes #2707
Fixes #2613