-
Notifications
You must be signed in to change notification settings - Fork 18
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
Isensee_JCB2018: fix SBML parameter values #253
base: master
Are you sure you want to change the base?
Conversation
Okay, so the reference value in Isensee for the log-likelihood value in the amici tests is (and always has been) The parameters table in the matlab benchmark collection also indicates that the I ran the following code to evaluate chi2 values (would have been a bit easier using AMICI, but alas):
with values set to 0 ( adding the amici reported chi2 value for |
Thanks for looking in to this. I would be fine with changing the problem in this collection back to being the H1+H3 model, since it's still valid for the benchmark collection. However, it might actually be the H1-only model when all I see that in the supplementary to the original publication, file "Model_description.xlsx", sheet "Model Variants", it specifies that the "off" value for the However, the MATLAB code "SetupFitting_PKA_cycle__batch.m" in the same supplementary instead defaults to |
Until it has been decided which model/dataset is the correct one, and the expected llh is updated accordingly. See Benchmarking-Initiative/Benchmark-Models-PEtab#253
* Tests: Skip llh test for Isensee_JCB2018 Until it has been decided which model/dataset is the correct one, and the expected llh is updated accordingly. See Benchmarking-Initiative/Benchmark-Models-PEtab#253 * fix broken pytest invocation
In #132, this problem was changed to remove model selection parameters (if they were not selected).
All model selection parameters in the SBML model were also set to 0. This changed the value of the
xi_i_*
model selection parameters, which led to a change in the objective function.This PR reverses that change. AMICI previously expected a likelihood of
-3.9494e+03
. With the following code and this PR, I get a posterior of-3953.8289641759175
(without prior:-3949.3759644713145
, prior-only:-4.452999704602744
)However, it's unclear to me whether this PR is an improvement, because in d2d,
xi_i_*
parameters are set to0
.https://github.com/Data2Dynamics/d2d/blob/f3e55b8600529ee213eb53b3e2a1628d44210b24/arFramework3/Examples/Isensee_JCB2018/Setup.m#L84-L90