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

Review the Base extraction process in ROBOT for bugs/incompleteness #819

Closed
matentzn opened this issue Mar 6, 2023 · 5 comments
Closed
Assignees
Labels
Milestone

Comments

@matentzn
Copy link
Contributor

matentzn commented Mar 6, 2023

We did a cursory review of the process, and it looks correct, but due to the importance of the upcoming base-revolution @cmungall suggests to review the code for the extraction of base axioms at least once.

In the end it all comes down to this method in ROBOT: RelatedObjectsHelper::getAxiomSubjects which is called by RelatedObjectsHelper::filterExternalAxioms.

@matentzn matentzn self-assigned this Mar 6, 2023
@matentzn
Copy link
Contributor Author

matentzn commented Mar 12, 2023

Note from @dosumis (rephrased by me):

If an external ontology provides no base, or a faulty one, there is a risk that our current process to define a "base release" will result in unwanted axiom injection (because for example because the base file reclassifies one of our base terms, illegally). The risk is more pertinent for the "providing a faulty one" as in the case of "no base", our approach to "make a base" would strip out injected axioms (assuming robots remove --axioms external command works, this issue here).

The question is whether to be 100% safe, we need to unmerge all external axioms from a base release. This can be safely done using the unmerge command if (and only if) we use the "inject provenance" feature into the imports module. Basically this would happen:

  1. All axioms would be tagged with prov:wasDerivedFrom <http:..external.owl>
  2. These exact axioms are first merged to prepare the base plus release
  3. Because they have this annotation, they are not automatically merged with the exact same axiom should it exist in the ontology. So, lets say an import injects A sub B, and that exact axiom is in our base (because its ours!), then unmerge would not delete it because it has the prov:wasDerivedFrom <http:..external.owl> annotation on it. Practical.

Vote: Should we play it safe and add an additional processing step?

  • 👍: Lets do it right once and for all and unmerge all external axioms as a last step of the base-plus pipeline.
  • 👎: Lets see if this really is a problem in practice. It seems redundant if (1) our base-method works as intended and (2) people are following our strict rules on what a base is (i.e. such issues should be fixed upstream, and not by a hack).

@matentzn matentzn added this to the 1.5 milestone Mar 12, 2023
@matentzn
Copy link
Contributor Author

From @dosumis on slack:

Interested to see what happens with GCIs in this case, but that’s an edge case we can deal with in a separate ticket if necessary.

I think we should figure out as part of this review what happens exactly with GCIs (complex subject). I think they are removed.

@matentzn
Copy link
Contributor Author

One bug: ontodev/robot#1108

@matentzn
Copy link
Contributor Author

I motion to close this issue, as I think we are good on the ODK side of things, and we do not have the capacity for a complete rundown of all potential issues around the remaining problem (which is the unambiguous definition and implementation for "what is a base axiom" in ROBOT). If we see this issue without a comment on the 20th of October, we close it as "not planned".

@matentzn
Copy link
Contributor Author

from @cmungall on slack for future reference:

What belongs in the base?

  • “SomeValuesFrom(?object_property, ?x) rdfs:subClassOf ?y -> in or out if ?x is a base entity?” — I am tending towards in (do you know offhand if owlapi considers the axiom to be “about” x?)
  • “Somehow avoid relaxing A=B if A and B are names” - I think relaxing equiv between NCs is universally confusing regardless of base files. I get that it logically makes sense but I think it was always a mistake to do this
  • “The pure uglyness of relaxing rococo axioms”. Less concerned about this. These disappear on standard graph projections anyway. Simple edges can be obtained with reasoning or materialization but this is not really the concern of base files per se

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