ModelicaStandardLibrary
ModelicaStandardLibrary copied to clipboard
Issues with Modelica.Blocks.Math.ContinuousMean
I see two problems:
- It is not possible to set another
startTime
as for most blocks, see https://stackoverflow.com/questions/70124980/dymola-modelica-how-can-i-calculate-the-mean-of-a-signal-only-in-the-steady-st/70127878 (a previous issue was thatstartTime
had no effect). - It is using an odd formula - without any explanation. I would guess it is for performance reasons, but I don't see that it actually gains anything. It might also be to gain precision - see comments at the end. The downside of the formulation is that it has a singularity that will mess with integrators, even if worked around.
https://github.com/modelica/ModelicaStandardLibrary/blob/8d090810980e1e2e51559721cdd4c267a4c849ae/Modelica/Blocks/Math.mo#L2318-L2319
I would propose the simpler:
parameter Real startTime=0;
protected
Real mu(start=0, fixed=true) "Internal integrator variable";
equation
der(mu) = if time>=startTime then u else 0;
y = noEvent(if time > startTime + t_eps then mu/(time-startTime) else u);
Basically just integrating the signal and dividing by time. Note that we can safely skip "t_eps" with this formulation due to the subtle change.
If we for some reason really need the extra precision I would propose switching between the two formulas after some time, i.e. use the simple formula for the first part and then switch to the current formula.
W.r.t. units the proposed change LGTM. Should mu
and startTime
be of type Modelica.Units.SI.Time
?
mu
shall be of type Real, so that the unit can be deduced depending on y
.
I am in favor of the proposed change for the sake of having startTime
consistently implemented.
I would yet like to comment that this change could cause a non-backwards compatible effect. If a simulation model of user has a simulation start time less than zero, the new implementation of the ContinuousMean model would behave differently as the start time is 0 by default but it was time
at the initial time previously.
We might overcome this (unlikely case) by
-
parameter SI.Time startTime = -Modelica.Constant.inf; // Which I do not like
- Describe the possible effect in the documentation
I would yet like to comment that this change could cause a non-backwards compatible effect. If a simulation model of user has a simulation start time less than zero, the new implementation of the ContinuousMean model would behave differently as the start time is 0 by default but it was
time
at the initial time previously.
To me it would seem reasonable to try to restore this behavior rather than documenting the degraded functionality as a known potential issue.
(But I don't see exactly how initializing with -Modelica.Constant.inf
could solve the problem. To me, it looks like this would just make der(mu)
more or less zero.)
I am in favor of the proposed change for the sake of having
startTime
consistently implemented.I would yet like to comment that this change could cause a non-backwards compatible effect. If a simulation model of user has a simulation start time less than zero, the new implementation of the ContinuousMean model would behave differently as the start time is 0 by default but it was
time
at the initial time previously.
Ok, that is tricky - I assume something like the following will work:
parameter Real startTime=-Modelica.Constants.inf;
protected
parameter Real t_0(fixed=false);
Real mu(start=0, fixed=true) "Internal integrator variable";
parameter Real actualStartTime=max(t_0, startTime);
initial equation
t_0=time;
equation
der(mu) = if time>=actualStartTime then u else 0;
y = noEvent(if time > actualStartTime then mu/(time-actualStartTime) else u);
(Or possibly having actualStartTime with fixed=false as well, might be more readable).
In general I see two problems with sources:
- They have
startTime=0
so we get this problem with starting before time=0 everywhere, e.g., when trying to test it with sine-source I stumbled on the same issue with the sine-source. - Many/most have a discontinuity at
startTime
- that is problematic; I noted sinc as a particular bad example.