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: add 19 new GPR and consolidate curations between model releases #313

Merged
merged 14 commits into from
Jun 5, 2022

Conversation

cheng-yu-zhang
Copy link
Collaborator

@cheng-yu-zhang cheng-yu-zhang commented May 26, 2022

Main improvements in this PR:

Add 19 new GPR accoording to SGD and uniprot.

  1. new GPRs are in "databasenewGPR.tsv"
  2. the proof of the new GPR is in "databasenewGPR_proof.tsv"
  3. the detailed new genes are in "databaseNewGPRGenes.tsv"
  4. the result of single gene deletion analysis is 0.8989.

This PR is used to demonstrate consolidation of multiple curations in one script, that can convert a previous model release to the next version.

I hereby confirm that I have:

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

@edkerk
Copy link
Member

edkerk commented May 26, 2022

You should make new feature branches of a recent devel branch, this one is derived from a devel commit from last December.

In addition, please do not modify existing curation files, but make new file/folders in code/modelCuration and data/modelCuration. This prevents confusion on what are new curations and old curations.

@edkerk
Copy link
Member

edkerk commented May 26, 2022

I have rebased this onto the most recent devel branch and refactored the datafiles to be contained in their own folder.

Metric yeast-GEM 8.6.0 this PR
Number of genes 1151 1153
Accuracy 0.8801 0.8810
TP 923 923
TN 61 62
FP 98 97
FN 36 36

@edkerk
Copy link
Member

edkerk commented May 26, 2022

FYI, I'm "hijacking" this PR to demonstrate how to consolidate curation from a particular model version towards the next release. For the moment I've also already merged #305 and #306 to hear, to combine all three curations in one consolidated script.

@edkerk edkerk changed the title add_19_new_GPR feat: add 19 new GPR and consolidate curations between model releases May 26, 2022
@edkerk edkerk added the wip work in progress label May 26, 2022
@hongzhonglu hongzhonglu requested a review from edkerk May 28, 2022 08:50
edkerk added 4 commits May 28, 2022 22:21
# Conflicts:
#	README.md
#	code/modelCuration/addDBNewGeneAnnotation.m
#	model/dependencies.txt
#	model/yeast-GEM.xml
#	model/yeast-GEM.yml
@edkerk edkerk removed the wip work in progress label May 28, 2022
Copy link
Member

@edkerk edkerk left a comment

Choose a reason for hiding this comment

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

Please run /code/modelCuration/v8_6_0.m to test the consolidated curation of #305, #306 and this PR. This new curation pipeline is introduced here, let's test it out for a while to see whether it functions well.

Otherwise, there are a few concerns to look at, @cheng-yu-zhang @feiranl.

@cheng-yu-zhang
Copy link
Collaborator Author

cheng-yu-zhang commented May 29, 2022

@edkerk When I run "/code/modelCuration/v8_6_0.m", I find that there is a extra parameter "grRatioMuts" in line 44 of the "essentialGenes.m", which may cause some issues.
[genes, fluxes, originalGenes, details, grRatioMuts]=findGeneDeletions(model,'sgd','fba');
[genes, fluxes, originalGenes, details]=findGeneDeletions(model,testType,analysisType,refModel,oeFactor)
Apart from that, this pipeline works well.

@edkerk
Copy link
Member

edkerk commented May 30, 2022

When I run "/code/modelCuration/v8_6_0.m", I find that there is a extra parameter "grRatioMuts" in line 44 of the "essentialGenes.m", which may cause some issues. [genes, fluxes, originalGenes, details, grRatioMuts]=findGeneDeletions(model,'sgd','fba'); [genes, fluxes, originalGenes, details]=findGeneDeletions(model,testType,analysisType,refModel,oeFactor) Apart from that, this pipeline works well.

Great, this behaviour of essentialGenes is expected as long as SysBioChalmers/RAVEN#421 is not merged.

@edkerk edkerk mentioned this pull request May 30, 2022
3 tasks
@edkerk edkerk self-requested a review May 30, 2022 16:43
Copy link
Member

@edkerk edkerk left a comment

Choose a reason for hiding this comment

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

  • Note that all changes should be documented in a script, see for instance:
    model = setParam(model,'lb','r_4232',-1000);
    model = setParam(model,'rev','r_4232',1);
    model = removeReactions(model,'r_4566',true,true,true);
  • Note that removal of a reaction remains complete removal, not just blocking by setting LB and UB to 0, so not:
    r_4566 6-phospho-D-gluconate[c] + ADP[c] + H+[c] => ATP[c] + D-gluconate[c] YDR248C 0.00 0.00 0.00

@edkerk edkerk merged commit a3bea04 into develop Jun 5, 2022
@edkerk edkerk deleted the add_new_GPR branch June 5, 2022 10:49
@edkerk edkerk mentioned this pull request Jun 16, 2022
3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants