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

fix-gene: remove non-transporter genes - updated #450

Merged
merged 1 commit into from
Jan 13, 2023

Conversation

haowang-bioinfo
Copy link
Member

@haowang-bioinfo haowang-bioinfo commented Jan 10, 2023

Main improvements in this PR:

I hereby confirm that I have:

  • Tested my code on my own computer for running the model
  • Selected develop as a target branch

@mihai-sysbio
Copy link
Member

See my comment in #444 - I'm worried that we might be moving too fast here.

Copy link
Collaborator

@johan-gson johan-gson left a comment

Choose a reason for hiding this comment

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

It was difficult to see the exact changes from last time, but I think you thought it through - I will not go over everything again. Good job!

@haowang-bioinfo
Copy link
Member Author

haowang-bioinfo commented Jan 11, 2023

as suggested in #426, simulation checks with models before and after these deletions were conducted and showed exactly the same growth rates.

@JonathanRob
Copy link
Collaborator

simulation checks with models before and after these deletions were conducted and showed exactly the same growth rates

Why would removing genes change simulation results? Unless you're using an enzyme-constrained model, I don't quite understand how it would influence flux at all (assuming by "simulation" you mean FBA).

@haowang-bioinfo
Copy link
Member Author

haowang-bioinfo commented Jan 11, 2023

Why would removing genes change simulation results? Unless you're using an enzyme-constrained model, I don't quite understand how it would influence flux at all (assuming by "simulation" you mean FBA).

@JonathanRob yes, now it's just FBA that gave the same results since stoichiometric was not changed. As just commented in #426, a systematic checking code would be needed as reaction addition/removal may happen in the future.

@haowang-bioinfo haowang-bioinfo merged commit a841814 into develop Jan 13, 2023
@haowang-bioinfo haowang-bioinfo deleted the fix/delNonTransporterGenes2 branch January 13, 2023 08:35
@feiranl
Copy link
Collaborator

feiranl commented Jan 15, 2023

Gene essentiality analysis should performed in terms of removing genes? The check function is coming.

@haowang-bioinfo
Copy link
Member Author

Gene essentiality analysis should performed in terms of removing genes?

agree

@johan-gson
Copy link
Collaborator

If you want I can do the gene essentiality part - I recently did this for the single-cell modeling paper, so I know how to do it and have the code on the cluster etc. set up for this. I can do it in 1-2 weeks (or later), I don't know the status of the model, when it will be ready.

@johan-gson
Copy link
Collaborator

But don't expect any large changes to the prediction power :)

@haowang-bioinfo haowang-bioinfo mentioned this pull request Feb 24, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants