obophenotype / uberon

An ontology of gross anatomy covering metazoa. Works in concert with https://github.com/obophenotype/cell-ontology
http://obophenotype.github.io/uberon/
Other
132 stars 29 forks source link

`components/reflexivity_axioms.owl` is unused #2980

Closed gouttegd closed 1 year ago

gouttegd commented 1 year ago

We have an awkward step in the Makefile (and I am not even the one who says it is awkward):

# somewhat awkward: we temporarily inject reflexivity axioms
# **Hacking_Feb_2022** Notes start here
# Background: 
## This ends up in the release file, reflexivity_axioms.owl temp merge as consistency QC
## This is only a QC.  Rationale - this would catch some errors because expands part disjointness ()
## **Hacking_Feb_2022** TODO - rewrite as as expansions from annotation axioms using SPARQL. Expanded axioms --> component (as for taxon restrictions).

TMP_REFL=$(COMPONENTSDIR)/reflexivity_axioms.owl
DEVELOPS_FROM_CHAIN=$(COMPONENTSDIR)/develops-from-chains.owl
# see https://github.com/obophenotype/uberon/issues/2381

$(TMPDIR)/materialized.owl: $(TMPDIR)/unreasoned.owl $(TMP_REFL)
    $(ROBOT) merge -i $< -i $(DEVELOPS_FROM_CHAIN) --collapse-import-closure false \
        relax \
        materialize -T $(CONFIGDIR)/basic_properties.txt -r elk \
        reason -r elk --exclude-duplicate-axioms true --equivalent-classes-allowed asserted-only \
        unmerge -i $(TMP_REFL) \
        unmerge -i $(DEVELOPS_FROM_CHAIN) \
        annotate -O $(URIBASE)/uberon/materialized.owl -V  $(RELEASE)/materialized.owl -o $@ 2>&1 > $@.LOG
.PRECIOUS: $(TMPDIR)/materialized.owl

What should happen in this step, among other things, is that the components/reflexivity_axioms.owl component is merged with the ontology, materialisation and reasoning are performed, then the axioms from the reflexivity_axioms.owl components are removed from the output.

That is not what actually happens. Notice how the reflexivity_axioms.owl component, even though it is listed as a dependency of $(TMPDIR)/materialized.owl, is never actually merged in – only the develops-from-chains.owl component is merged.

This is seemingly the result of the conversion of that rule from OWLTOOLS to ROBOT, which happened in commit a3be1f63775feb0e8ce1ff3b0b03410ef236b9aa. Here is the relevant diff:

-ext.owl: $(TMPDIR)/materialized.owl $(TMP_REFL)
-       $(OWLTOOLS) $< $(TMP_REFL) --merge-support-ontologies -o $(TMPDIR)/m1.owl && \
-       $(ROBOT) --catalog $(CATALOG) merge -i $(TMPDIR)/m1.owl --collapse-import-closure false \
-       unmerge -i $(TMP_REFL) \
-       annotate --ontology-iri $(ONTBASE)/$@ $(ANNOTATE_ONTOLOGY_VERSION) -o $@ 2>&1 > $(TMPDIR)/$@.LOG
+$(TMPDIR)/materialized.owl: $(TMPDIR)/unreasoned.owl $(TMP_REFL)
+       $(ROBOT) merge -i $< --collapse-import-closure false \
+               relax \
+               materialize -T $(CONFIGDIR)/basic_properties.txt -r elk \
+               reason -r elk --exclude-duplicate-axioms true --equivalent-classes-allowed none \
+               unmerge -i $(TMP_REFL) \
+               annotate -O $(URIBASE)/uberon/materialized.owl -V  $(RELEASE)/materialized.owl -o $@ 2>&1 > $@.LOG
+.PRECIOUS: $(TMPDIR)/materialized.owl
+
+ext.owl: $(TMPDIR)/materialized.owl
+       $(ROBOT) annotate -i $< --ontology-iri $(ONTBASE)/$@ $(ANNOTATE_ONTOLOGY_VERSION) -o $@ 2>&1 > $(TMPDIR)/$@.LOG

In the original rule, $(TMP_REFL) was merged by OWLTOOLS by virtue of the --merge-support-ontologies command:

$(OWLTOOLS) $< $(TMP_REFL) --merge-support-ontologies -o $(TMPDIR)/m1.owl

In the updated rule, $(TMP_REFL) is never passed to the ROBOT merge command:

$(ROBOT) merge -i $< --collapse-import-closure false

The bottom line is that reflexivity_axioms has not been merged in since November 2021. Importantly, trying to merge it now results in the materialized.owl step to fail, because the reflexivity axiom on BFO:0000050 causes the following inferred equivalences:

2023-07-19 17:49:14,982 ERROR org.obolibrary.robot.ReasonOperation - Only equivalent classes that have been asserted are allowed. Inferred equivalencies are forbidden.
2023-07-19 17:49:14,987 ERROR org.obolibrary.robot.ReasonOperation - Equivalence: <http://purl.obolibrary.org/obo/GO_0000313> == <http://purl.obolibrary.org/obo/GO_0005840>
2023-07-19 17:49:14,987 ERROR org.obolibrary.robot.ReasonOperation - Equivalence: <http://purl.obolibrary.org/obo/GO_0005938> == <http://purl.obolibrary.org/obo/GO_0099738>
2023-07-19 17:49:14,988 ERROR org.obolibrary.robot.ReasonOperation - Equivalence: <http://purl.obolibrary.org/obo/GO_0090068> == <http://purl.obolibrary.org/obo/GO_0045787>
2023-07-19 17:49:14,988 ERROR org.obolibrary.robot.ReasonOperation - Equivalence: <http://purl.obolibrary.org/obo/GO_0010564> == <http://purl.obolibrary.org/obo/GO_0051726>
2023-07-19 17:49:14,989 ERROR org.obolibrary.robot.ReasonOperation - Equivalence: <http://purl.obolibrary.org/obo/GO_0045786> == <http://purl.obolibrary.org/obo/GO_0010948>
2023-07-19 17:49:14,989 ERROR org.obolibrary.robot.ReasonOperation - Equivalence: <http://purl.obolibrary.org/obo/GO_0099568> == <http://purl.obolibrary.org/obo/GO_0005737>
gouttegd commented 1 year ago

Given that the reason why the reflexivity axiom was injected (prior to the commit aforementioned) was to serve as a QC check, should that injection be restored?

I personally don’t see the point of a QC check in Uberon that catches inferred equivalences in GO.

cmungall commented 1 year ago

Just get rid of this step. There is something fundamentally incoherent or at least uncommitted about treatment of reflexivity in RO but this is not an uberon problem.

If we want to get complete QC checking then we need to expand the pattern for e.g. spatially disjoint from, to either use union or to make 2x2 combos. But this should be another ticket.

cmungall commented 1 year ago

Here is a RO ticket about reflexivity and part-of

gouttegd commented 1 year ago

Thank you for the input @cmungall . I’ll get rid of that step as part of my ongoing cleanup of the custom Makefile.

gouttegd commented 1 year ago

Fixed in #3028