RheoModel revamp #111
RheoModel revamp #111
Conversation
| # Storage modulus | ||
| Gp = quote | ||
| if a!=1.0 | ||
| #if a!=1.0 # This is to be removed as the condition contradicts the constraints |
There was a problem hiding this comment.
Which constraint? This will cause unexpected non-zero results for users not familiar with floating point error (cos(pi/2)!=0). Small error but noticeable in log-log plots.
There was a problem hiding this comment.
This one:
# Constraints
constraint = quote
(a<1) & (a>0)
end,
For the floating-point error, maybe it would make sense to use the specialised functions that solve this problem?
julia> cos(pi/2)
6.123233995736766e-17
julia> cospi(1/2)
0.0
There was a problem hiding this comment.
That constraint is only enforced in a handful of places though (e.g. checking initial conditions before the fit process starts). Importantly, it's not enforced when making a direct call to the modulus (I don't think, unless you/Ale changed it), or during optimisation process. (Indeed, we wouldn't need the special mittleff wrapper if the constraint was enforced throughout optimisation.)
Same applies to the other changes you made in a number of the the other models. (I didn't comment on them all, but I noticed there were several other changes you made getting rid of edge case handling.
I do not have skin in the game here because I'm unlikely to use RHEOS in future, but what I can say for certain is that those edge cases were in their specifically because we hit them at various times using RHEOS in practice, and they caused unwanted behaviour - so we need to think carefully about each removal. Worth discussing with Ale also.
edit: sure if you can fix it using cospi then that's fine, but it doesn't address the other edge case handling you removed from other moduli, and I'm not sure if there any speed/readability consequences from using cospi instead of cos.
There was a problem hiding this comment.
That is a good point. It is not very important here, but this touches on a broader set of problems about how we check the constraints when we call the moduli. I'd prefer to keep it clean and fast for the low level access like _G and _Ga, but we should definitely make sure that the parameters sent to functions such as relaxmod are checked properly, for instance for the relative values of the exponents, etc. There are at the moment many ways it could go wrong, and not all of them can be hacked as we do here - we need to throw exceptions I think. There are indeed other issues with the fact that NLopt does not enforce the constraints beyond checking parameters at the start. Maybe this deserves its own issue...
There was a problem hiding this comment.
I did some work in that direction already, which is documented in the open issue #86
Exceptions make sense in some contexts, but definitely not all of them (the above code with cos(pi/2) being a good example). A 'smooth' transition from springpot to spring/dashpot specialisation causes fundamental difficulties in computational implementation, but the conversation around to what extent we want to support it and how is still an important one.
There was a problem hiding this comment.
Yes, not all contexts can be addressed like that indeed.
| _Gppa::TVec | ||
|
|
||
| _constraint::FWConstraint | ||
| _Gramp::Bool |
There was a problem hiding this comment.
I still don't understand the ramp nomenclature, might be worth having a longer discussion about it. Also, do we need a Jramp? Assuming it's a flag for handling singularities I'm not sure that creep moduli have any - happy to be corrected.
There was a problem hiding this comment.
You are right about Jramp, it is not required for the creep functions indeed.
The flags is just a placeholder for future use at the moment. Nothing implemented. So yes, the long discussion is still pending!
|
@moustachio-belvedere |
No description provided.