INCATools / ontology-development-kit

Bootstrap an OBO Library ontology
http://incatools.github.io/ontology-development-kit/
BSD 3-Clause "New" or "Revised" License
212 stars 53 forks source link

New check to detect loops in definitions #528

Open gouttegd opened 2 years ago

gouttegd commented 2 years ago

In FBbt we recently added a custom check to detect cyclic logical definitions, because one such loop found its way into a release (FlyBase/drosophila-anatomy-developmental-ontology#1366) without being caught beforehand.

Any objection to adding the check directly into the ODK?

matentzn commented 2 years ago

We should have plenty of protection against this by using

robot reason -i x.owl --allow-equivalent-classes asserted-only

which you recently made mandatory.

Do you want to add this check because of the necessity to catch the problem without using the reasoner?

The much more heavy problem is the thing @dosumis and @anitacaron are (I think?) working on that procludes cycles over object properties like part-of..

balhoff commented 2 years ago

Have that implemented here (I think): https://github.com/obophenotype/uberon/pull/2125

Maybe somebody could take it over?

matentzn commented 2 years ago

Already happening somewhere somehow in relation to single cell atlas with @dosumis

David can you assign whoever was doing work on stitching part of chaings together to the PR that Jim linked above?

dosumis commented 2 years ago

Seems like multiple issues here:

  1. Detect cycles over subClassOf - A subClassOf B subClassOf A (isn't this logical equivalence?)
  2. Detect any indirect cycle over some relationship (OP restriction) types, where these indicate an error (e.g. part_of - could this be generalised to all transitive, non-symmetric object properties?)
  3. Detect the presence of any cycles at all: foo-basic.obo is meant to be acyclic (DAG). This might be perfectly legal, reasonable OWL (e.g. reciprocal part_of/has_part restrictions), but is required by some legacy users. I think that's what this is about https://github.com/obophenotype/uberon/issues/1829

@anitacaron can potentially help out, but we need to be clear about which task is which here. Do we need three tickets?

dosumis commented 2 years ago

Looking at #1829 a bit more, I can noticed Chris covers case 2 in the discussion (but maybe we need 2 versions for the 2 use cases?). Check 1 could be folded into 2 if working in a completely graph-y way with Soufflé. I'll add @anitacaron to the assignees.