Fix bounds and scales for some parameters in Isensee_JCB2018

[dilpath]

No description provided.

[elbaraim]

Hi @dilpath
Why were these changes included? The original bounds and scales for the modified parameters in this pull request were originally set to be in linear scale with the given boundaries.
See https://github.com/Data2Dynamics/d2d/blob/master/arFramework3/Examples/Isensee_JCB2018/Setup.m

EDIT: Therefore, unless this was specifically requested for some reason, I think there is nothing to fix here.

[dilpath]

Hi @dilpath Why were these changes included? The original bounds and scales for the modified parameters in this pull request were originally set to be in linear scale with the given boundaries. See https://github.com/Data2Dynamics/d2d/blob/master/arFramework3/Examples/Isensee_JCB2018/Setup.m

EDIT: Therefore, unless this was specifically requested for some reason, I think there is nothing to fix here.

The model doesn't appear to unscale the parameter, so estimating in [-5, 3] is incorrect, and instead should be [0.00001, 1000].

[elbaraim]

The model doesn't appear to unscale the parameter, so estimating in [-5, 3] is incorrect, and instead should be [0.00001, 1000].

I see what you mean. However, the parameter in the d2d implementation is set to zero in a linear scale and set as fixed. Therefore I think that adjusting the bounds to the values that you propose is correct, however the scale shall be kept to lin and the nominalValue and estimate to zero. Otherwise probably while loading the problem in pyPESTO will try to convert the zero to logarithmic scale and probably fail (?).

[dilpath]

The model doesn't appear to unscale the parameter, so estimating in [-5, 3] is incorrect, and instead should be [0.00001, 1000].

I see what you mean. However, the parameter in the d2d implementation is set to zero in a linear scale and set as fixed. Therefore I think that adjusting the bounds to the values that you propose is correct, however the scale shall be kept to lin and the nominalValue and estimate to zero. Otherwise probably while loading the problem in pyPESTO will try to convert the zero to logarithmic scale and probably fail (?).

If an error occurs in AMICI/pyPESTO, it could be fixed I think, since zero can still be used on log-scale with NumPy:

>>> import numpy as np
>>> np.log(0.0)
-inf
>>> np.exp(np.log(0.0))
0.0

Inclusion of zero in the bounds would still be problematic.

[FFroehlich]

The model doesn't appear to unscale the parameter, so estimating in [-5, 3] is incorrect, and instead should be [0.00001, 1000].

I see what you mean. However, the parameter in the d2d implementation is set to zero in a linear scale and set as fixed. Therefore I think that adjusting the bounds to the values that you propose is correct, however the scale shall be kept to lin and the nominalValue and estimate to zero. Otherwise probably while loading the problem in pyPESTO will try to convert the zero to logarithmic scale and probably fail (?).

I believe this is an artifact from the d2d implementation where it seems like parameters can only be fixed on a linear scale (see for example reduced/regular version of Lucarelli)

[elbaraim]

I believe this is an artifact from the d2d implementation where it seems like parameters can only be fixed on a linear scale (see for example reduced/regular version of Lucarelli)

@FFroehlich would then fix the parameter value at zero + defining log10 scale work in the AMICI/pyPESTO import ? As far as I know, pyPESTO works with the parameters in the defined scale, therefore np.log10(0) could be problematic (probably for other tools, like d2d too?)
If so, I do not see anything against approving these changes.

[FFroehlich]

I believe this is an artifact from the d2d implementation where it seems like parameters can only be fixed on a linear scale (see for example reduced/regular version of Lucarelli)

@FFroehlich would then fix the parameter value at zero + defining log10 scale work in the AMICI/pyPESTO import ? As far as I know, pyPESTO works with the parameters in the defined scale, therefore np.log10(0) could be problematic (probably for other tools, like d2d too?) If so, I do not see anything against approving these changes.

Ah, thats a good point. I did not check but I can definitely see this being problematic for other tools (e.g., d2d) , even if it works in AMICI/pyPESTO. So I wouldn't recommend changing this.

[FFroehlich]

I believe this is an artifact from the d2d implementation where it seems like parameters can only be fixed on a linear scale (see for example reduced/regular version of Lucarelli)

@FFroehlich would then fix the parameter value at zero + defining log10 scale work in the AMICI/pyPESTO import ? As far as I know, pyPESTO works with the parameters in the defined scale, therefore np.log10(0) could be problematic (probably for other tools, like d2d too?) If so, I do not see anything against approving these changes.

Ah, thats a good point. I did not check but I can definitely see this being problematic for other tools (e.g., d2d) , even if it works in AMICI/pyPESTO. So I wouldn't recommend changing this.

I do however see the issue that this causes trouble when for example trying to run model selection where some parameters are fixed/unfixed. I would say the best way to address this is to add additional indicator variables (e.g., representing the matlab variables model_options.partial_import, model_options.different_KD, model_options.PDE_inhibition, model_options.AC_inhibition) that are on a linear scale with bounds [0,1] and then set the scale for the fixed parameters to log10 with nonzero value and multiplying all occurences in the model with the respective indicator variables.