Benchmarking-Initiative / Benchmark-Models-PEtab

A collection of mathematical models with experimental data in the PEtab format as benchmark problems in order to evaluate new and existing methodologies for data-based modelling
BSD 3-Clause "New" or "Revised" License
26 stars 14 forks source link

Fix lucarelli #140

Closed FFroehlich closed 2 years ago

FFroehlich commented 2 years ago

removes experimental information from model and fixes observables. The Lucarelli model contains equally named observables with distinct formulas for individual functions, the previous implementation included the incorrect variants of those observables for data6 and data7 conditions.

FFroehlich commented 2 years ago

Thanks!

  • Can the fix_init_* now be removed from the model?

Yes! Removed the assignment involving them but forgot to remove the parameter itself!

  • Do the bounds on S*tot need to be adjusted?

I don't think so. What makes you think this may be necessary?

  • Should simulated data be updated?

Yes, can do that!

  • Could remove unused columns (e.g. measurement table)

👍

  • To print tables on GitHub
dilpath commented 2 years ago

Great, thanks for the fixes overall!

  • Do the bounds on S*tot need to be adjusted?

I don't think so. What makes you think this may be necessary?

Before, the initial assignments were some transformation of S*tot. Now, they are simply S*tot. If bounds were taken from the publication, then fine as is. If bounds were chosen to be "reasonable", then perhaps fine as is, or maybe they need to be adjusted to account for the (now lack of) transformation.

e.g. the nominal value for S2tot is "close" (on log10 scale) to a bound. Looks like this value comes from here so I guess it should still be OK. Fine to merge if you think it's not an issue/can reproduce the log-likelihood from here. https://github.com/LeonardSchmiester/Benchmark-Models/blob/hackathon/hackathon_contributions_new_data_format/Lucarelli_CellSystems2018/General_info.xlsx

FFroehlich commented 2 years ago

Great, thanks for the fixes overall!

  • Do the bounds on S*tot need to be adjusted?

I don't think so. What makes you think this may be necessary?

Before, the initial assignments were some transformation of S*tot. Now, they are simply S*tot. If bounds were taken from the publication, then fine as is. If bounds were chosen to be "reasonable", then perhaps fine as is, or maybe they need to be adjusted to account for the (now lack of) transformation.

e.g. the nominal value for S2tot is "close" (on log10 scale) to a bound. Looks like this value comes from here so I guess it should still be OK. Fine to merge if you think it's not an issue/can reproduce the log-likelihood from here. https://github.com/LeonardSchmiester/Benchmark-Models/blob/hackathon/hackathon_contributions_new_data_format/Lucarelli_CellSystems2018/General_info.xlsx

The transformation only served the purpose of switching between the provided numerical value and the value of S*tot, which now is implemented in the condition table. The Implementation was tested by comparing chi2 values for all conditions between amici and d2d implementations.