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

Human 1.19 #823

Merged
merged 150 commits into from
Jun 24, 2024
Merged

Human 1.19 #823

merged 150 commits into from
Jun 24, 2024

Conversation

JHL-452b
Copy link
Collaborator

Main improvements in this PR:

johan-gson and others added 30 commits May 21, 2023 16:49
…ion. I don't think that code had ever been run, it was just not right.
…urrently only processes the model available in the repo, and does not compare with any older model version. The code is however prepared for comparing versions. Currently, it only processes 2 out of 40 chunks of data, this is to be changed when we want to run a full evaluation with 891 cell lines.
Added metabolite SMILES and Inchi Information
…d 2-OADH reactions and thiamine-to-lipoyl transfer for 2-OADH
@edkerk edkerk self-assigned this Jun 24, 2024
@edkerk edkerk merged commit 9a58fef into main Jun 24, 2024
4 checks passed
@feiranl
Copy link
Collaborator

feiranl commented Jun 25, 2024

Thanks Ed for fixing the issue!
Could you paste the paste the Essentiality here? @JHL-452b

@edkerk
Copy link
Member

edkerk commented Jun 25, 2024

There were two problems:

  • One of the earlier PRs was made from a fork with a branch that was derived from an earlier develop branch.
  • The YAML file was not fully sorted alphabetically.

The latter could be resolved by a round of import-export. The resulting model could then be diff'ed, and the differences to main were manually checked to correspond the original PRs.

The first problem could only be resolved by a -Xtheirs-merge to main, which then closes the PR.

To reduce the risk of similar problems in the future:

  • Do not allow PRs made from forks, only from feature branches in the SysBioChalmers/Human-GEM repo. In forks it is less obvious that the feature branch was based on the most recent develop branch. Regular contributors should be given write access to make branches (as is already customary).
    • If a PR is made from a fork (by an external collaborator), then a new feature branch in SysBioChalmers/Human-GEM should be made, and all PR commits cherry-picked to that branch.
  • Before merging a PR, go through a round of import-export (with importYaml -> exportYaml) to ensure that the YAML file is (alphabetically) ordered.

@JHL-452b
Copy link
Collaborator Author

JHL-452b commented Jun 25, 2024

Updated essentiality evaluation using combined (all) Hart2015 datasets:

version TP TN FP FN accuracy sensitivity specificity F1 MCC
v1.12 40 2333 175 77 0.904000000000000 0.341880341880342 0.930223285486443 0.240963855421687 0.204768560393159
v1.13 40 2334 174 77 0.904380952380952 0.341880341880342 0.930622009569378 0.241691842900302 0.205504558241293
v1.14 40 2334 175 77 0.904036557501904 0.341880341880342 0.930251096054205 0.240963855421687 0.204787829413435
v1.15 40 2342 168 77 0.906737723639132 0.341880341880342 0.933067729083665 0.246153846153846 0.210053956889428
v1.16 41 2308 202 76 0.89417587 0.35042735 0.91952191 0.22777778 0.19220101
v1.17 41 2263 237 75 0.880733944954129 0.353448275862069 0.905200000000000 0.208121827411168 0.172768250444715
v1.18 42 2260 245 74 0.878290728729493 0.362068965517241 0.902195608782435 0.208436724565757 0.174052567331894
v1.19 38 2371 131 78 0.920168067226891 0.327586206896552 0.947641886490807 0.266666666666667 0.230477038058642

Compared with Human 1.18, Human 1.19 seems to have changed a lot, especially the TN has increased significantly.

The organ-specific models were generated by the ftINIT (1+1 mode) , and the .code/evaluateHart2015Essentiality function was used to obtain the gene evaluation results.

@johan-gson
Copy link
Collaborator

Hmm, and everything was done exactly the same for all of these versions? Would be good to figure out what makes the difference? Is it possible to do interval halving, i.e., include half of the changes and see if it changed then, then close in on the interval by taking/removing half of the changes where the change occured? For the Hart dataset this may not be that heavy?

@JonathanRob
Copy link
Collaborator

Actually the drop in FP is the biggest relative change; it seems overall to have improved a decent amount, though at the expense of a bit lower sensitivity - a good trade off in my opinion.

@JHL-452b
Copy link
Collaborator Author

Hmm, and everything was done exactly the same for all of these versions? Would be good to figure out what makes the difference?

I'm not sure what was done for previous versions. But we do need to figure out what's causing the difference.

Is it possible to do interval halving, i.e., include half of the changes and see if it changed then, then close in on the interval by taking/removing half of the changes where the change occured?

Do you mean to try to impose half of changes to the model and evaluate it, if I understand correctly. This sounds like a good idea. @johan-gson

@johan-gson
Copy link
Collaborator

Yep, and then depending on in which half the changes, split that in half and include all changes up to that point, and os on. Will give log(n) complexity :)

@mihai-sysbio
Copy link
Member

Fantastic to have this merged! And indeed a positive (great!) suprise with the increase in the MCC. One could use also git bisect, to avoid playing with commits manually.

@mihai-sysbio
Copy link
Member

.code/evaluateHart2015Essentiality function was used to obtain the gene evaluation results.

@JHL-452b there is a (long-standing) PR to run this function automatically on all pull requests to main and develop. Would you want to have a look in #675?

@JHL-452b
Copy link
Collaborator Author

@JHL-452b there is a (long-standing) PR to run this function automatically on all pull requests to main and develop. Would you want to have a look in #675?

Oh yeah, it truly helps a lot. I will have a try in the next few days. Thanks!

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.

8 participants