matthiaskoenig / dfba

Encoding Dynamic Flux Balance Analysis in SBML
11 stars 1 forks source link

Action Items for Manuscript #2

Closed matthiaskoenig closed 7 years ago

matthiaskoenig commented 7 years ago

The following things still have to be done before submission @matthiaskoenig

@leandrohw

matthiaskoenig commented 7 years ago

Managing the open things for me at https://github.com/matthiaskoenig/dfba/blob/master/TODO.md

matthiaskoenig commented 7 years ago

Hi @leandrohw,

can you run `/models/toy_wholecell/toy_wholecell_v14.omex to create the latest version of

toy_wholecell_mk_v14-ibiosim.csv
toy_wholecell_mk_v14-ibiosim.png
toy_wholecell_mk_v14-ibiosim.pdf

Could you create this for dt=0.1, dt=1 and dt=5 (dt=0.1 is for the figure 4 and what is defined in the omex, but wanted to compare the results for the other time steps also.

M

matthiaskoenig commented 7 years ago

@leandrohw, also finished the toy_atp now with supplements. can you run /models/toy_atp/toy_apt_v12.omex with dt=0.1, tend=15 and create

toy_atp_mk_v12-ibiosim.csv
toy_atp_mk_v12-ibiosim.png
toy_atp_mk_v12-ibiosim.pdf

The numerical comparison is described now in the manuscript and works for toy_wholecell and toy_atp, i.e. results are numerical identical within epsilon.

matthiaskoenig commented 7 years ago

@leandrohw diauxic results are also updated. I would need from you to run /models/diauxic_growth/mk/v13 and /models/diauxic_growth/mk/v4 with dt=0.01, tend=15.0 to create

diauxic_growth_lw_v4-ibiosim.csv
diauxic_growth_lw_v4-ibiosim.png
diauxic_growth_lw_v4-ibiosim.pdf
diauxic_growth_mk_v13-ibiosim.csv
diauxic_growth_mk_v13-ibiosim.png
diauxic_growth_mk_v13-ibiosim.pdf

The plots should only display until 12.5, but simulate until 15 (because my model is a bit slower). I could reproduce your results. So I think we can both reproduce each others results, despite the two diauxic models having a bit different kinetics.

You basically have to create the results for the latest model versions (see this comment and above) than I can run the numerical comparison and we are finished.

edit: v13 of diauxic

leandrohw commented 7 years ago

How do we manage the file names for different dt?

matthiaskoenig commented 7 years ago

Just append a _dt0.1, _dt0.05, ... at the end of the filename before the extension, I did this for some simulations in the following folder to test the diffferent step sizes https://github.com/matthiaskoenig/dfba/tree/master/models/toy_wholecell/mk/v14 i.e.,

diauxic_growth_mk_v13_dt0.1-ibiosim.csv
leandrohw commented 7 years ago

@matthiaskoenig the sed-ml for toy atp has timelimit=50. Based on your plots, I think it should be 15. I am going to manually change to 15 to match your plots.

matthiaskoenig commented 7 years ago

@leandrohw had this already fixed in my creator script, but forgot to recreate the SED-ML and omex :/ Just pushed the fixed version with tend=15

matthiaskoenig commented 7 years ago

@leandrohw I just updated my scripts for simulation so the dt and tend is added to the files and created all my files accordingly. See latest push Please could you do

diauxic_growth_mk_v13-ibiosim_dt0.1_tend15.0.csv

i.e.

{modelname}_{creatorinitial}_v{version}-{simulator}_dt{dtvalue}_tend{tendvalue}.csv

This will make it the easist for me to compare the csv.

leandrohw commented 7 years ago

Will do.

matthiaskoenig commented 7 years ago

Updated all my files. Let me know when you finished your files than I do the comparison. Late here, will have a look tomorrow morning.

leandrohw commented 7 years ago

They have been updated.

matthiaskoenig commented 7 years ago

@leandrohw Here some feedback:

toy_wholecell

wrong timepoints: /dfba/models/toy_wholecell/toy_wholecell_mk_v14-ibiosim_dt0.1_tend50.0.csv, should have 501 not 51 wrong timepoints: /dfba/models/toy_wholecell/toy_wholecell_mk_v14-ibiosim_dt5.0_tend50.0.csv, should have 11 not 51 I think there is some issue with writing the csv on your site (figures look correct, but content of csv is wrong Here we really need the 0.1 simulation, also in the manuscript figures, because dt=1.0 is not yet converged to correct solution (you can see this in the different steady state reached) -> numerical identical with dt1.0, but we need dt=0.1 By the way, this is also a very interesting result which I will add in 1-2 sentence in the manuscript as soon as the csv are comparable, i.e. something like:

"In a SOA-DFBA approach it is important that the dt timesteps are small enough so that the solution converges against the correct solution and it is necessary to test different step sizes (for instance changing the step size in the toy_wholecell model from 1.0 to 0.1 resulted in differences in steady state concentrations of up to 10%). Very interestingly, our encoding schema allowed to reproduce the numerical results even if the step sizes were not yet small enough to have converged against the correct solution. By having an exchange schema we were able to test the effects of varying dt in a reproducible manner."

toy_atp

wrong timepoints: toy_atp_mk_v12-ibiosim_dt0.1_tend15.csv, should have 151 not 16

diauxic

I can compare the files, but the difference is to large. We are really close, but I think the numerical errors of the ODE integrators just accumulate (you can see this in the plots at https://github.com/matthiaskoenig/dfba/blob/master/models/compare.ipynb). Basically if we make a small error of 1E-6 in opposite directions in every timestep this accumulates over all the timepoints :/. But we are very close, i.e. diff of 0.0075 > 0.001. Three solutions to this:

  1. We try to set our ode absTol and relTol more strict and see if this solves it (I would like to try this, because we probably can also press down the diff of the other models, and think this is the right way to solve it). I.e. more stringent absTol, relTol and simulate again (add these algorithm parameters to SED-ML). I will try to set my tolerances more stringent, perhaps you already are more accurate (if this is not the case you also have to make your tolerances more strict)

  2. We discuss for issue in the manuscript, i.e.,

    "Due to small solver differences which accumulate over time, ... the maximum absolute difference between the implementations is 0.0075 (which is very close but does not fullfill our defined criterium of numerical identity")

  3. We try to get the mk diauxic to run and see if the diff is correct (yes, we should get the model working with latest version, but not solving the underlying issue)

Action items for you:

matthiaskoenig commented 7 years ago

@leandrohw Good news. I set my tolerances on my ode solver much stricter and now the results are much more identical. I.e. the diauxic issue is solved:

We can set epsilon for comparison from 1E-3 to 1E-6 which is great. So your tolerances were stricter than mine. So only need the fixed csv for toy_atp and diauxic. We will use the diauxic_lw_v4 in the publication, lets fix the diauxic_mk issue after publication, because its not relevant for now.

matthiaskoenig commented 7 years ago

Just released latest version cy3sbml which allows to view the created *.cys supplements, so that reviewers can interact with the supplements. http://apps.cytoscape.org/apps/cy3sbml

leandrohw commented 7 years ago

@matthiaskoenig Probably. I was using 1e-9 for absTolerance and 1e-6 for relative. The problem with the csv is that I set the parameter dt to be 0.1, 1, and 5 in the model. I didn't change the print interval for the csv file. Do I change both?

matthiaskoenig commented 7 years ago

Yes, would be great if you could create the csv files with the same printed points like specified by dt and tend, i.e. all steps are printed in the csv.

leandrohw commented 7 years ago

Ok, I just pushed the files.

matthiaskoenig commented 7 years ago

Perfect. I did the comparison, updated supplements, updated figures and fixed references. I think we are finished.