ontodev / robot

ROBOT is an OBO Tool
http://robot.obolibrary.org
BSD 3-Clause "New" or "Revised" License
259 stars 73 forks source link

Relax does not preserve ObjectUnionOf #514

Open matentzn opened 5 years ago

matentzn commented 5 years ago

Given this ontology (ont.owl):

Prefix(:=<http://purl.obolibrary.org/obo/wbbt.owl#>)
Prefix(owl:=<http://www.w3.org/2002/07/owl#>)

Ontology(<http://purl.obolibrary.org/obo/wbbt.owl>
<http://purl.obolibrary.org/obo/wbbt/2018-10-19/wbbt.owl>
    EquivalentClasses(<http://purl.obolibrary.org/obo/WBbt_0008453> ObjectUnionOf(<http://purl.obolibrary.org/obo/WBbt_0001360> <http://purl.obolibrary.org/obo/WBbt_0001518>))
)

running

robot relax --input ont.owl remove --axioms equivalent --output ont-relaxed.owl

Results in an empty ontology (Robot 1.4.0). Just running the relax command does not create any subclass of axioms.

The problem is that some users expect the union_of: relation in obo, which is derived from the ObjectUnionOf statement; which is now not possible (since the new OBO format products require relaxation).

matentzn commented 5 years ago

@dosumis FYI

jamesaoverton commented 5 years ago

Thanks @matentzn. @cmungall has been responsible for the relax code, and it would be best if he can address this issue.

Is there a spec or reference implementation for the correct behaviour somewhere?

matentzn commented 5 years ago

According to the docs, the relax spec seems to (or should be) basically this:

Given any axiom of the form A = {C1 and C2 and ... Cn} (axiom1), with A being a class name and Ci being a (arbitrarily complex) class expression, relax(axiom1) will result in:

A = {C1 and C2 and ... Cn} # this is maintained by relax
A subclassOf: C1
A subclassOf: C2
....
A subclassOf: Cn

Examples

relax(A=B):

A = B
A subclass B

relax(A=R some B)

A=R some B
A subclass R some B

relax(A=(R some B) and D )

A=(R some B)  and D 
A subclass R some B
A subclass D

--> And therefore

relax(A= B or D)

A=B or D
A subclass B or D
matentzn commented 5 years ago

I suspect that Chris original use case was all about pushing stuff into isa relations as much as possible, as these are what downstream tools best knew what to do with. I just realised, independent of this issue, that my use case wont be covered by this unfortunately. The union_of: statement is only generated in the case of equivalence class axioms. However, the issue here is unaffected; lets see what @cmungall thinks.

cmungall commented 5 years ago

fairly sure this can be closed but will comment in more detail later

cmungall commented 5 years ago

I suspect that Chris original use case was all about pushing stuff into isa relations as much as possible

Formally, into relational graphs, i.e. subClassOf between a named class and either a named class or a simple existential R some Y. This is under the motivation section of the relax docs.

(I am not sure if you are using obo format speak, in obo format is_a is a subClassOf between two named classes)

Could go either way here:

  1. Improve the docs to reflect current behavior
  2. Change the behavior to also generate subClassOf-unionOf

Note that 2 won't help the worm people, it's irrelevant to them

I now think 2 may be best for overall consistency. It won't negatively affect people producing relational graphs.

matentzn commented 5 years ago

But you still want to restrict relax to a specified set of axiom patterns (A = B, A = R some C)? Not arbitrarily complex expressions like A= R some (C or R some B)?