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

components/reflexivity_axioms.owl is unused #2980

Closed
gouttegd opened this issue Jul 19, 2023 · 5 comments
Closed

components/reflexivity_axioms.owl is unused #2980

gouttegd opened this issue Jul 19, 2023 · 5 comments
Assignees
Labels

Comments

@gouttegd
Copy link
Collaborator

gouttegd commented Jul 19, 2023

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 a3be1f6. 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)/[email protected]
+$(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 > [email protected]
+.PRECIOUS: $(TMPDIR)/materialized.owl
+
+ext.owl: $(TMPDIR)/materialized.owl
+       $(ROBOT) annotate -i $< --ontology-iri $(ONTBASE)/$@ $(ANNOTATE_ONTOLOGY_VERSION) -o $@ 2>&1 > $(TMPDIR)/[email protected]

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
Copy link
Collaborator Author

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.

@gouttegd gouttegd self-assigned this Jul 19, 2023
@gouttegd gouttegd added the tech label Jul 19, 2023
@cmungall
Copy link
Member

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
Copy link
Member

Here is a RO ticket about reflexivity and part-of

@gouttegd
Copy link
Collaborator Author

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

@gouttegd
Copy link
Collaborator Author

Fixed in #3028

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants