obophenotype / human-phenotype-ontology

Ontology for the description of human clinical features
http://obophenotype.github.io/human-phenotype-ontology/
Other
288 stars 51 forks source link

Unsatisfiable classes referencing CHEBI roles #4952

Closed balhoff closed 4 years ago

balhoff commented 4 years ago

There are a number of HPO classes that are unsatisfiable when HPO is combined with RO, CHEBI, and GO:

these classes do not have a logical def

the terms below are using Chebi classes that are molecular entities:

NV: I updated the logical def for these terms:

All the ones I checked have to do with using the term metabolite as a material entity; however it is a biological role. Needs to be replaced with 'chemical entity' and 'has role' some metabolite.

balhoff commented 4 years ago

cc @matentzn

matentzn commented 4 years ago

Oh that's a nice ticket, thanks; @nicolevasilevsky and I will take care of it on Wednesday, and make a pattern. So are you saying that given an EQ like:

Abnormal CSF metabolite level:

has part some (
    amount and 
    inheres in some (
        metabolite and 
        part of some cerebrospinal fluid) and 
    has modifier some abnormal)

it should in fact be:

ALT1:

has part some (
    amount and 
    inheres in some (
        cerebrospinal fluid and 
        has role some 'metabolite') and 
    has modifier some abnormal)

This seems somehow not the intention; I have no idea of biology, but it seems that in everyday parlance, a metabolite could be referred to as a a material entity (Google says: "a substance formed in or necessary for metabolism."). Or do you mean something like:

ALT2:

has part some (
    amount and 
    inheres in some (
        part of some cerebrospinal fluid and 
        has role some 'metabolite') and 
    has modifier some abnormal)
pnrobinson commented 4 years ago

Well, in the real world a metabolite IS a material entity. Why does this make the class unsatisfiable in the OWL world?

matentzn commented 4 years ago

Someone at CHEBI decided it is a biological role instead of a material entity.. MAybe we can find out why and use another class..

pnrobinson commented 4 years ago

A metabolite is a chemical (molecule). The role metabolite indicates that the molecule was created by processes of metabolism (and not say, by synthesis in a factory). In the HPO definitions, we are not modeling the provenance of the molecules and so the role is not important. The HPO definition does not appear to be wrong -- but there might be some situations in the future where we will want to reason that metabolites are molecules? Probably ok as is.

balhoff commented 4 years ago

It's just that CHEBI has this term (and some others like 'hormone') in a separate hierarchy. The way you use them as chemicals/material entities is to make an expression 'chemical entity' and 'has role' some metabolite. The resulting concept is the chemical you want. It has to be changed for reasoning to work.

@matentzn it should be like this:

'has part' some (
    amount and 
    'inheres in' some (
        ('chemical entity' and 'has role' some metabolite) and 
        'part of' some 'cerebrospinal fluid') and 
    has modifier some abnormal)

See for example the logical definition of Decreased circulating androgen level.

I have set up a daily Jenkins job for a bunch of ontologies that I'm trying to keep consistent. Hopefully it will catch this kind of stuff early on. At the moment it uses release versions, but it would be better to make it build base files from master of each ontology and merge those.

pnrobinson commented 4 years ago

@balhoff -- this definition "feels right" to me.

matentzn commented 4 years ago

Alright, we will create a pattern for this; @nicolevasilevsky would you mind giving it a shot? I think an appropriate level of generality would be:

After this is done we will assign all of the definitions above to that pattern.

matentzn commented 4 years ago

(I think I prefer chemical entity to be a slot, variable rather than hard backed into the pattern)

matentzn commented 4 years ago

@balhoff can you please take a look at:

http://purl.obolibrary.org/obo/upheno/patterns-dev/abnormallyIncreasedLevelOfChemicalEntityWithRoleInLocation.yaml

Nicole has created the analogous patterns for:

Thanks.

balhoff commented 4 years ago

That looks good to me.

nicolevasilevsky commented 4 years ago

@matentzn asked me to work on this. I think my action item is to update the logical defs for the terms above. I am going to work on this now

matentzn commented 4 years ago

Thanks Nicole. yeah I am also starting to hit downstream problems because of the current pattern. With our new one, it will all be well!

nicolevasilevsky commented 4 years ago

@matentzn I revised the logical def for three terms that were using Chebi role terms:

nicolevasilevsky commented 4 years ago

The rest of the classes listed above were either:

  1. obsolete
  2. did not have a logical def (maybe they did at one point?)
  3. The chebi class is classified as a molecular entity. In this case, is the logical def okay? Or should it be revised to conform to the role pattern above?
matentzn commented 4 years ago

All ok!! Perfect thanks Nicole!

matentzn commented 4 years ago

As long as no metabolites are used with inheres in, I am happy.

matentzn commented 4 years ago

(And no chebi roles are connected to anything else with part of)

nicolevasilevsky commented 4 years ago

I think it should be good now, but let me know if I missed anything

matentzn commented 4 years ago

Re-open if necessary.