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

Feat/SBOterms #132

Merged
merged 3 commits into from
Jul 12, 2018
Merged

Feat/SBOterms #132

merged 3 commits into from
Jul 12, 2018

Conversation

BenjaSanchez
Copy link
Contributor

Main improvements in this PR:

Following up on #106, an important model feature for memote is the existence of SBO terms for all components in the model. Here we have included SBO terms for all rxns and mets based on an automatic script that runs each time the model is updated with saveYeastModel.m, based on the criteria from addSBOterms.m:

  • For metabolites:
SBO term name criterion # in model
SBO:0000649 biomass any metabolite name matching biomass, DNA, RNA, protein, carbohydrate or lipid, or ending with backbone or chain (pseudo-metabolites in SLIME reactions) 50 mets
SBO:0000247 simple chemical the rest 2191 mets
  • For reactions:
SBO term name criterion # in model
SBO:0000627 exchange reaction any reaction with only one metabolite involved, in the extracellular compartment 165 rxns
SBO:0000628 demand reaction any reaction with only one metabolite involved, not in the extracellular compartment, and being produced 0 rxns
SBO:0000632 sink reaction any reaction with only one metabolite involved, not in the extracellular compartment, and being consumed 3 rxns
SBO:0000629 biomass production the reaction matching biomass pseudoreaction 1 rxn
SBO:0000630 ATP maintenance the reaction matching non-growth associated maintenance reaction 1 rxn
SBO:0000395 encapsulating process any reaction containing the words pseudoreaction or SLIME rxn 193 rxns
SBO:0000655 transport reaction any reaction with at least one metabolite being moved from one compartment to another (same name but different compartment) 917 rxns
SBO:0000176 biochemical reaction the rest 2240 rxns

@Midnighter @ChristianLieven opinions on this scheme?

I hereby confirm that I have:

  • Tested my code with all requirements for running the model
  • Selected devel as a target branch (top left drop-down menu)
  • If needed, asked first in the Gitter chat room about this PR

@BenjaSanchez BenjaSanchez added enhancement new field/feature question ideas/feedback from other people would be appreciated labels Jul 9, 2018
@BenjaSanchez BenjaSanchez self-assigned this Jul 9, 2018
@BenjaSanchez BenjaSanchez mentioned this pull request Jul 10, 2018
12 tasks
@ChristianLieven
Copy link

Hi Ben, just a few comments on the current state of SBO testing in memote. In general, it is not fully developed yet. We're really specifically checking for a few node SBO terms and not yet for the presence of any children of those nodes.

  • We don't test for biomass metabolites using SBO:0000649 (yet). There is only one function needs to identify biomass metabolites and here we currently look for biomass metabolites matching this regex ^biomass(_[a-zA-Z]+?)*?$ i.e. we look for metabolite IDs with the prefix biomass_. However, I believe this has to change in the future.

  • As for metabolites we first test if there is generally any SBO term defined, and them more specifically we check how many metabolites are defined as for the presence of SBO:0000247 i.e. 'simple chemical'. Again, this will have to change in the future, when we start testing for this term and all of its children.

  • For transport reactions, again, we're only looking at the parent node SBO:0000185 not at any of its children 'SBO:0000588', 'SBO:0000587', 'SBO:0000655', 'SBO:0000654',
    'SBO:0000660', 'SBO:0000659', 'SBO:0000657', and 'SBO:0000658'

  • ATP maintenance and encapsulating process are not tested for either, but I think the former is definitely a thing we should do too! I'll add that to the issue about our SBO overhaul (Improve SBO term handling opencobra/memote#469).

Thanks again for giving us this feedback Ben! It's much appreciated!

@BenjaSanchez
Copy link
Contributor Author

@ChristianLieven thanks a lot for the comments!

We don't test for biomass metabolites using SBO:0000649 (yet).

The biomass reaction is at least recognized, so for now this is not a big concern for us (just 98% instead of a 100% in the "Metabolites without SBO:0000247" test)

For transport reactions, again, we're only looking at the parent node SBO:0000185

The memote report states:

'SBO:0000185', 'SBO:0000588', 'SBO:0000587', 'SBO:0000655', 'SBO:0000654', 'SBO:0000660', 'SBO:0000659', 'SBO:0000657', and 'SBO:0000658' represent the terms 'transport reaction' and 'translocation reaction', in addition to their children (more specific transport reaction labels). Every transport reaction that is not a pure metabolic or boundary reaction should be annotated with one of these terms.

So I guess this will also change in the future to account for all children?

For the time being this PR will be merged, and after memote finishes developing SBO testing we will asses if any of our criteria should change. Issue #106 will be adapted accordingly.

@BenjaSanchez BenjaSanchez merged commit e2d594c into devel Jul 12, 2018
@BenjaSanchez BenjaSanchez mentioned this pull request Jul 13, 2018
@BenjaSanchez BenjaSanchez deleted the feat/SBOterms branch July 20, 2018 13:18
@hongzhonglu
Copy link
Collaborator

hongzhonglu commented Sep 12, 2018

@BenjaSanchez
As for the SBO term, I find that r_0226, r_0438, r_0439... belong to SBO:0000655 (transport type), however, these reactions are not belong to transport reaction. Can you check it?

@BenjaSanchez
Copy link
Contributor Author

@hongzhonglu those 3 rxns are 3 of the oxidative phosphorylation complexes, and they do pump protons through the mitochondrial membrane (in one direction or the other), so from the SBO perspective they can be categorized as transports. If you think they are better suited to another ontology feel free to open an issue for further discussion

@hongzhonglu
Copy link
Collaborator

@BenjaSanchez OK! Thanks.

@BenjaSanchez BenjaSanchez removed the question ideas/feedback from other people would be appreciated label Nov 14, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement new field/feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants