Skip to content

Smith_BMCSystBiol2013#180

Merged
FFroehlich merged 33 commits intomasterfrom
Smith_BMCSystBiol2013
Apr 28, 2023
Merged

Smith_BMCSystBiol2013#180
FFroehlich merged 33 commits intomasterfrom
Smith_BMCSystBiol2013

Conversation

@FFroehlich
Copy link
Collaborator

fixes #177

draft implementation. Includes scripts to generate petab file. Still requires validation in simulation experiments (simulation results are specified in the supplement.

Requires clarification if petab condition table should provide concentrations/amounts first.

@FFroehlich
Copy link
Collaborator Author

FFroehlich commented Mar 8, 2023

validated simulation against references provided in supplementary material (available in sim_test directory) using AMICI (requires fix for PEtab import). Satisfying general agreement (rtol=1e-1/1e-2, atol=1e-3). Only notable differences are in simulations for figure 2H, in species related to SOD2/InR transcription. Looks like transcription rates are off by a factor 0.8 and 0.4 resepectively and translation/degradation rate is off by a factor 0.9 for InR. Unclear why though, likely there were some changes to the model itself, impossible to track down now though, since these rates are implemented as local parameters and values are thus not reported in simulation files. Decided to therefore keep this as is, as impact on remainder of model seems negligible.

@FFroehlich FFroehlich marked this pull request as ready for review March 8, 2023 19:44
Copy link
Collaborator

@dilpath dilpath left a comment

Choose a reason for hiding this comment

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

Overall, thanks a lot, clearly a lot of work to curate this!

  • Currently, this is larger in filesize than the Froehlich_CellSystems2018 model. The simulation file is 39MB. Can this be reduced by simulating fewer timepoints, or do you think the current state is best?
  • Regarding the sim_test
    • Why are there two PEtab problems, and what are the differences between them?
    • Rename measurement to simulation in the simulation file, and rename the file to e.g. simulatedData_Smith_BMCSystBiol2013.tsv
    • Could you provide a visualization PEtab file?

Thanks for providing some tips on working with this model. Currently, there is no expected way to share this information, but putting such things in some README.md would be great, if you think it would be useful (completely optional).

Comment on lines +72 to +79
sc_PI3K log10 0.0001 10000.0 1.6607492397638188 1
sc_GLUT_2B log10 0.0001 10000.0 0.005003654843537889 1
sc_BINS log10 0.0001 10000.0 0.001627712376345442 1
sc_PIRS log10 0.0001 10000.0 0.005656559423442325 1
sc_PTP log10 0.0001 10000.0 1.6e-06 1
sc_GLUT_3B log10 0.0001 10000.0 0.013426907522675441 1
sc_SOD2 log10 0.0001 10000.0 0.0002336225060797476 1
sc_FOXO1 log10 0.0001 10000.0 0.0009623342379275169 1
Copy link
Collaborator

Choose a reason for hiding this comment

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

Could annotate these to be hierarchically optimized in pyPESTO, but no need for this collection.

@dilpath
Copy link
Collaborator

dilpath commented Mar 9, 2023

Please also update the README, e.g. by running python overview.py --markdown and copying the output there.

https://github.com/Benchmarking-Initiative/Benchmark-Models-PEtab/blob/master/scripts/overview.py

@FFroehlich
Copy link
Collaborator Author

Please also update the README, e.g. by running python overview.py --markdown and copying the output there.

https://github.com/Benchmarking-Initiative/Benchmark-Models-PEtab/blob/master/scripts/overview.py

done

@FFroehlich FFroehlich requested a review from dilpath March 9, 2023 12:59
@dilpath
Copy link
Collaborator

dilpath commented Mar 10, 2023

Thanks for the changes! Did you see these points? #180 (review)

e.g. if the second PEtab is for validation, is it possible to simplify the second PEtab problem to just be extra measurement file(s) and PEtab YAML files, like done for Fujita? https://github.com/Benchmarking-Initiative/Benchmark-Models-PEtab/tree/master/Benchmark-Models/Fujita_SciSignal2010/test_data

I guess this makes validation easiest, since then e.g. user's workflows would just involve replacing the measurements, rather than compiling a second model.

@FFroehlich
Copy link
Collaborator Author

Sorry missed this comment.

Overall, thanks a lot, clearly a lot of work to curate this!

  • Currently, this is larger in filesize than the Froehlich_CellSystems2018 model. The simulation file is 39MB. Can this be reduced by simulating fewer timepoints, or do you think the current state is best?

The simulation file was actually not simulated by me, but are files from the original publication that were used for visualization.

  • Regarding the sim_test

    • Why are there two PEtab problems, and what are the differences between them?

It's also different conditions (additional conditions and additional columns).

  • Rename measurement to simulation in the simulation file, and rename the file to e.g. simulatedData_Smith_BMCSystBiol2013.tsv

Hmm, but those are not actual simulations from the PEtab model, so this might be confusing. I could also provide additional simulation results files.

  • Could you provide a visualization PEtab file?

Will try.

Thanks for providing some tips on working with this model. Currently, there is no expected way to share this information, but putting such things in some README.md would be great, if you think it would be useful (completely optional).

Okay will add a readme file with some additional information.

@FFroehlich
Copy link
Collaborator Author

Thanks for the changes! Did you see these points? #180 (review)

e.g. if the second PEtab is for validation, is it possible to simplify the second PEtab problem to just be extra measurement file(s) and PEtab YAML files, like done for Fujita? https://github.com/Benchmarking-Initiative/Benchmark-Models-PEtab/tree/master/Benchmark-Models/Fujita_SciSignal2010/test_data

I guess this makes validation easiest, since then e.g. user's workflows would just involve replacing the measurements, rather than compiling a second model.

Can try to simplify, main limitation was that there are not simple routines to write petab problems to files where some of the files already exist (but maybe I should just overwrite them).

@dilpath
Copy link
Collaborator

dilpath commented Mar 10, 2023

Thanks, as the second PEtab problem is to validate this implementation against the original implementation, I think what you've done already is sufficient. Visualization file(s) and a short readme would be great!

It's clear from the script that you use the same model file in both PEtab problems, but please include the similarity/difference as a note in the readme.

@FFroehlich
Copy link
Collaborator Author

Thanks, as the second PEtab problem is to validate this implementation against the original implementation, I think what you've done already is sufficient. Visualization file(s) and a short readme would be great!

It's clear from the script that you use the same model file in both PEtab problems, but please include the similarity/difference as a note in the readme.

both done

@FFroehlich FFroehlich requested review from dilpath and plakrisenko and removed request for dilpath and plakrisenko April 25, 2023 09:48
@FFroehlich
Copy link
Collaborator Author

looks like poetry/pre-commit hooks are broken, but I don't think this is an issue on my end

@FFroehlich FFroehlich merged commit 15df97d into master Apr 28, 2023
@FFroehlich FFroehlich deleted the Smith_BMCSystBiol2013 branch April 28, 2023 09:43
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.

Add Smith_BMCSystBiol2013

3 participants