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

Gapfilling #185

Merged
merged 9 commits into from
May 23, 2019
Merged

Gapfilling #185

merged 9 commits into from
May 23, 2019

Conversation

feiranl
Copy link
Collaborator

@feiranl feiranl commented Feb 6, 2019

Main improvements in this PR:

  • This PR is to fill the gap for dead end metabolites in the model. Here we only focus on the deadend metabolites that can be fixed by adding a transport reaction.
  • MissingTransDeadEnd.m is a function that detect this kind of dead end metabolites
  • After detect those metabolites, all transport reactions in MetaNet database of this metabolite would be extracted. We choose the most possible reactions to add according to existing transport reactions in the model.
  • addGapfillingRxnToGEM.m is to update the model with chosen reactions

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

Copy link
Collaborator

@hongzhonglu hongzhonglu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here, the gap filling mainly find transport reactions connect the dead metabolites which existed in two different compartments. It will be a little dangerous that if no evidences support the related transport reactions. Can we find the related evidences?

@edkerk
Copy link
Member

edkerk commented Feb 8, 2019

Here, the gap filling mainly find transport reactions connect the dead metabolites which existed in two different compartments. It will be a little dangerous that if no evidences support the related transport reactions. Can we find the related evidences?

I agree with Hongzhong on this. Taking the list of suggested new transport reactions, can we evaluate what type of transport they are? It seems some of them are probably simple diffusion (cytoplasm <-> nucleus), while others would be facilitated by some protein. Is there evidence for these reactions in yeast or other organisms? There probably is as they are all MetaNetX reactions?

@feiranl
Copy link
Collaborator Author

feiranl commented Feb 11, 2019

First, we got the list of deadend metabolites due to transport rxns missing. Next step, we go to MetaNetx to search for all transport rxns related to that met, then we find whether our model has the same transport rxn for this met for another compartment or same transport mechanism for similar met to decide which kind of transport rxns that we should add.

Example:

glutathione[er] is a deadend, according to this rxn: r_1168 glutathione transport glutathione[c] <=> glutathione[v] YDR135C or YLL015W -1000 1000

we added the simple rxn for glutathione[er]:glutathione[c] <=> glutathione[er]

@feiranl feiranl requested a review from edkerk February 11, 2019 16:01
@BenjaSanchez BenjaSanchez self-assigned this May 13, 2019
@BenjaSanchez BenjaSanchez added enhancement new field/feature wip work in progress labels May 13, 2019
Copy link
Contributor

@BenjaSanchez BenjaSanchez left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@feiranl The PR looks good regarding compatibility & reproducibility. A couple of things that I think you should address though:

  • As in previous PRs, the new rxns should contain in the rxnNotes a reference to this PR number. E.g.: "Gap-filling (PR Gapfilling #185); MNXR95431"
  • Also in the rxnNotes field, "unknown transporter" should be stated for any reaction that does not occur by difusion.
  • MissingTransDeadEnd is lacking documentation: you should specify what is each column in the output table, otherwise it's hard to understand.
  • metsinComps could be simplified to a large extent to not require both the COBRA and RAVEN models, you could just provide the metabolite index instead of the name, and return the indexes in the RAVEN model that match that name, as indexes of RAVENized models are the same as the COBRA counterpart. In fact you could avoid metsinComps entirely and simplify a lot the code by just using the indexes of the RAVEN model in your script.

Note that I added a couple commits fixing some conflicts and styling preferences. Make sure to pull those changes ;)

@BenjaSanchez
Copy link
Contributor

@edkerk @hongzhonglu I don't see the problem of adding these rxns as they all have corresponding MetaNetX ids, or perhaps I'm missing something? @feiranl are the predictions when simulating the model before and after these additions changing any fluxes significantly? Could you display those results here somehow?

@hongzhonglu
Copy link
Collaborator

@BenjaSanchez I think Feiran has already give the explanation. what I mean is that though the metnetx ids exist, the detailed reaction can be different in the compartment. It is nice to give evidence to show why we can give the related transport reactions.

@BenjaSanchez
Copy link
Contributor

@hongzhonglu what is most likely in all these cases is that transport does occur, but there is missing annotation for the specific compartment. So as long as we indicate "unknown transporter" and that the reactions were added due to gap-filling, I think it would be ok. Adding a transport reaction for which there is only evidence in another compartment is common practice in other models such as human-GEM I believe, maybe @Hao-Chalmers can confirm this?

@BenjaSanchez BenjaSanchez removed the wip work in progress label May 21, 2019
@edkerk
Copy link
Member

edkerk commented May 22, 2019

@BenjaSanchez. This is what I was missing, either proof or at least clarification why transport reactions were added:

So as long as we indicate "unknown transporter" and that the reactions were added due to gap-filling, I think it would be ok.

This is done in 789533d, so it looks good to me!

@BenjaSanchez BenjaSanchez mentioned this pull request May 22, 2019
@feiranl
Copy link
Collaborator Author

feiranl commented May 23, 2019

image

image

This is the result of growth prediction before and after gapfilling. Adding these rxns doesn't change the model prediction.

@BenjaSanchez BenjaSanchez merged commit 3f9dcf0 into devel May 23, 2019
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