RHEOS.jl
RHEOS.jl copied to clipboard
JuliaRheology
fill_init_params makes assumptions about model parameter names - is that safe?
Looking at fill_init_params in processing.jl, I realise that we are assuming specific symbols for the exponents of the multiple springpot models. I think this is problematic as one would assume these symbols are arbitrary. The logic used to set these parameters is unclear too.
It may be more appropriate to require that initial values are provided by the user, or included in the model definition itself (but fundamentally the values of anything that has a physical unit depend on the data, not the model - only fract exponents may benefit from default values). In processing, maybe we should only verify that the parameters satisfy the constraints as otherwise we would have to make some assumptions in code that I believe should be generic.
If I remember correctly, that function is not very important. I wrote it as a helper function to simply fill out other initial conditions if a user doesn't want to provide them all. I don't think it's a major issue, but I understand your concern.
The worst case I can envisage is that there is some future model that decides to name two springpots using the opposite naming convention, so that the auto-filled initial conditions are actually the wrong way round. I don't even know if this always a problem from memory, some of those two springpot models end up being symmetric with respect to their parameters anyway. It could be more pressing if we have a much smarter constraints system during optimization such as that I mentioned in https://github.com/JuliaRheology/RHEOS.jl/issues/86
In other scenarios I can imagine, a more harmless name clash is possible, but if the user has not specified initial guesses for those parameters then I can't imagine they mind if it is auto-set to 0.25, 0.5, 0.75 or anything else?
Edit: noticed it raises a warning aswell, in case a user does care what defaults are but forgot to specify
If, on further investigation it seems more trouble than it's worth to keep that functionality, we should just cut it out and raise an error if not all parameters' initial guesses are provided by the user. Otherwise, if you can come up with a more generic way to do it then go for it, but currently the only way I can see we keep track of how many springpots are in a model is by their naming convention.
I like your idea of including suggested initial parameters within each model, one risk there though. If the model has a condition a>b (for example) and user sets a=0.01, and the model's default for b is 0.25 then it doesn't meet constraints.
Feel free to delete if you are concerned, I don't feel strongly on it :+1:
And yes, the parameters depend on the data fundamentally. In practice, we often don't have a good sense for what sensible parameters are on first fitting iteration though so there is some benefit.
Good point re: default values in the model and the user setting one of them such that it clashes with the default value of the other. Best to make it a requirement then to specify initial values, and then check constraints are met and through an error when needed.
If empty, it was setting everything to 0.5 before this change it seems, with an index based fix for springpot parameters, probably why I tried to make something more robust:
https://github.com/JuliaRheology/RHEOS.jl/blame/41cfc646d34685379e38ff193d9d1ea5cd624f65/src/processing.jl#L207
On second look. it seems I just was probably just trying to extend a similar approach but for partially provided init conditions, hence the additional complexity.
Feel free to force user-provided params, equally valid to me :+1:
On this though, I disagree:
the values of anything that has a physical unit depend on the data, not the model - only fract exponents may benefit from default values
A counter-example: Reynolds number doesn't have physical units but depends on data and describes something inherently physical. The same is true for springpot coefficients.
I'm not saying that dimensionless parameters don't depend on the data - they would too. In the context of our rheological models, the fractional exponents are likely to be bounded by 0 and 1 in most practical cases, and therefore a default value at 0.5 does bother me as much.