ModelicaStandardLibrary
ModelicaStandardLibrary copied to clipboard
modelica
Fix interpretation of coefficients of complex transfer function
Also here my comment. If it is a bugfix then something was calculated wrongly before and need to be fixed. It would be very strange that we "fix" bugs by introducing new models and keep the buggy models around. That is if it is a clear bug and not just a different interpretation of the implementation.
I agree with @dietmarw here. Bugfixes shall be fixed in place and documented in the revision annotation. I also doubt that anyone used such a function if it was plainly wrong.
OK. The block will not be renamed or marked as obsolete: The bug fix will be implemented and documented.
@beutlich Why is it asking again for signing the CLA? I just signed it at https://github.com/modelica/ModelicaStandardLibrary/pull/3742#issuecomment-792252602
@beutlich Why is it asking again for signing the CLA? I just signed it at #3742 (comment)
Because you (accidently ?) committed as Christian Kral <christian@Yogix>. I will squash all commits as one later.
@beutlich Why is it asking again for signing the CLA? I just signed it at #3742 (comment)
Because you (accidently ?) committed as
Christian Kral <christian@Yogix>. I will squash all commits as one later.
Thanks for the hint. This happened accidentally. For some reason my git user.name and user.email got lost...
I will squash all commits as one later.
Done. I also resolved the merge conflict of #3677 and HTML tag error when rebasing. Please check again.
@beutlich Thanks for fixing the merge conflict. On my opinion the performed changes look all good.
@HansOlsson Is it legal Modelica code to use different units in the elements of vector a?
parameter Real d(unit = "kg/s")=0.01 "Damping coefficient in kg/s (not the damping ratio)";
parameter Modelica.Units.SI.Mass m=0.2 "Mass in kg";
parameter Modelica.Units.SI.TranslationalSpringConstant c=0.1 "Stiffness in N/m";
parameter Real b[:]={-m} "Numerator polynomial coefficients {-m} of the transfer function";
parameter Real a[:]={m,d,c} "Denominator polynomial coefficients {m,d,c} of the transfer function";
It, however, does check and simulate OK in Dymola.
@HansOlsson Is it legal Modelica code to use different units in the elements of vector
a?
Yes. Whether it is good style is another issue.
parameter Real d(unit = "kg/s")=0.01 "Damping coefficient in kg/s (not the damping ratio)"; parameter Modelica.Units.SI.Mass m=0.2 "Mass in kg"; parameter Modelica.Units.SI.TranslationalSpringConstant c=0.1 "Stiffness in N/m"; parameter Real b[:]={-m} "Numerator polynomial coefficients {-m} of the transfer function"; parameter Real a[:]={m,d,c} "Denominator polynomial coefficients {m,d,c} of the transfer function";It, however, does check and simulate OK in Dymola.
The specification does not make it illegal, and you could even explicitly create vectors with different unit elements in Modelica like: parameter Real a[3](unit={"kg","kg/s","kg.s-2"}); although Dymola currently ignores the unit in that case.
The specification does not make it illegal, and you could even explicitly create vectors with different unit elements in Modelica like:
parameter Real a[3](unit={"kg","kg/s","kg.s-2"});although Dymola currently ignores the unit in that case.
While the specification doesn't make it illegal today, we also know that the specification is rather incomplete when it comes to defining a system for unit handling that would give tools the necessary foundation to reject models with unclear/erroneous unit consistency. Hence, I think one should at least be prepared that arrays with heterogenous unit could be something we'll be forced to drop support for once we take on unit checking more seriously.
If one really wants to do transfer functions with units in a future-proof way, I have no better idea than making a fixed order wrapper model with only scalar coefficients that takes care of all the units, with the actual dynamics internally implemented using a unit-less transfer function.
@beutlich I suggest to keep the implementation with Real parameters then. OK?
A third option to unitless parameters or array with heterogenous unit would be something like:
parameter Real d(unit = "kg/s")=0.01 "Damping coefficient in kg/s (not the damping ratio)";
parameter Modelica.Units.SI.Mass m=0.2 "Mass in kg";
parameter Modelica.Units.SI.TranslationalSpringConstant c=0.1 "Stiffness in N/m";
final parameter oneReciprocalDamping(unit="s/kg") = 1;
final parameter oneReciprocalMass(unit="kg-1") = 1;
final parameter oneReciprocalSpringConstant(unit="m/N") = 1;
parameter Real a[:]={m*oneReciprocalMass, d*oneReciprocalDamping, c*oneReciprocalSpringConstant}
"Unitless denominator polynomial coefficients {m,d,c} of the transfer function";
I think this is the cleanest Modelica can offer for now. I also propose to utilize TranslationalDampingConstant for d.
I think this is the cleanest Modelica can offer for now.
Looks like a splendid place to use final constant instead of final parameter, doesn't it?
Right, here's a nicer proposal, inspired from https://github.com/modelica/ModelicaStandardLibrary/blob/39fe8050c0e7f487720d2f3ea5d91e25ef359ffe/Modelica/Electrical/Analog/Examples/Utilities/SwitchedCapacitor.mo#L29-L30:
parameter Modelica.Units.SI.Mass m = 0.2 "Mass in kg";
parameter Modelica.Units.SI.TranslationalDampingConstant d = 0.01 "Damping coefficient in kg/s (not the damping ratio)";
parameter Modelica.Units.SI.TranslationalSpringConstant c = 0.1 "Stiffness in N/m";
protected
constant Modelica.Units.SI.Mass oneKilogram = 1
"Helping constant to satisfy unit check";
constant Modelica.Units.SI.TranslationalDampingConstant oneUnitDampingConstant = 1
"Helping constant to satisfy unit check";
constant Modelica.Units.SI.TranslationalSpringConstant oneUnitSpringConstant = 1
"Helping constant to satisfy unit check";
public
final parameter Real a[:]={m/oneKilogram, d/oneUnitDampingConstant, c/oneUnitSpringConstant}
"Unitless denominator polynomial coefficients {m,d,c} of the transfer function";
Right, here's a nicer proposa, inspired from
Yes, this is nicer. Still, in principle, these constants should also be final, as they shouldn't even be modified in derived classes. Of course, I can also see that the need for final is much bigger when dealing with public elements.