Units for mathematical constants?
...reading through some of the recent activity in the Modelica spec(https://github.com/modelica/ModelicaSpecification/issues/3259 and https://github.com/modelica/ModelicaSpecification/issues/2127 and others), it seems like it might be a natural step to specify the units for the various mathematical constants in Modelica.Constants.
I might propose the following, which seems a bit more correct:
final constant Real e(unit="1")=Modelica.Math.exp(1.0);
final constant Real pi(unit="1")=2*Modelica.Math.asin(1.0); // 3.14159265358979;
final constant Real D2R(unit="rad/deg")=pi/180 "Degree to Radian";
final constant Real R2D(unit="deg/rad")=180/pi "Radian to Degree";
final constant Real gamma(unit="1")=0.57721566490153286061
Without this step, it seems to me like we are missing out on some unit checking opportunities since the units for pi (and others) can be inferred incorrectly.
Thoughts/comments?
I wouldn't worry about the ones with unit = "1" until we know what is obtained automatically by a refined language specification. (Personally, I'd prefer a language where it is perfectly fine to leave out the explicit unit = "1" as a way of defining a dimensionless variable, but time will tell…)
Regarding the others, what is the expected status of "deg" in Modelica? Consider:
Real a(unit = "1") = 1.0;
Real b(unit = "rad") = 1.0;
Real c(unit = "deg") = 1.0;
Real y = sin(a) + sin(b) + sin(c); /* OK? */
Are tools expected to know that sin operates both on unit "1" and "rad", but not on "deg"?
Are tools expected to know that
sinoperates both on unit"1"and"rad", but not on"deg"?
The original idea was that variables shall be in base-SI-units, so variables shall not have unit="deg".
Right, but then someone came up with Angle_deg and used it in Modelica.Mechanics.MultiBody.Parts.FixedRotation, so "deg" has a pretty widespread use by now…
Right, but then someone came up with Angle_deg and used it in Modelica.Mechanics.MultiBody.Parts.FixedRotation, so "deg" has a pretty widespread use by now…
And, of course, there was originally a bug in using them. The better solution is to correct that at some point in the future; as indicated by https://github.com/modelica/ModelicaStandardLibrary/issues/3158
@henrikt-ma For pi and other unit="1" items: if you think it's prudent to wait then that seems reasonable. I will just say I hope/expect I should be able to catch the following unit errors in the near future:
Real T(unit="K") = 300 "Temperature";
Real r(unit="m") = 1 "Radius";
Real A(unit="m2") = Modelica.Constants.pi*r^2*T "Area"; // T shouldn't be here
Real C(unit="m") = 2*Modelica.Constants.pi*r*T "Circumference"; // T shouldn't be here
As is, at least in the Modelica tool I'm using today, neither of those unit issues would be caught. The first seems to largest offender in my mind because conceptually there is no good reason it's not caught. The proposed change would address that today without waiting for changes to spec and waiting for agreement on how to handle situations when unit="". The 2nd seems potentially a little trickier because now we have two terms (2, pi) that each doesn't have units. The fact that adding unit="1" addresses at least the first case today and potentially simplifies the assumptions in the 2nd case (only need to assume units for 2 and not both 2 and pi), is the main reason and thinking that prompted me to create this issue ticket.
Regarding the R2D and D2R items, I have a few more comments:
- These don't appear to be in use in MSL. Probably they should show up in the to_deg and from_deg conversions at least. Otherwise, why even keep them if they aren't used in the most natural spot (conversion function)?
- Even if they are kept, they are redundant (just reciprocals), do we need both of them? We don't have any other reciprocal constants.
- Since nonSI
degunits are in use in MSL and even this file (considering T_zero withdegC), it does seem ok and correct from my standpoint to tack on the units.
@henrikt-ma For pi and other unit="1" items: if you think it's prudent to wait then that seems reasonable. I will just say I hope/expect I should be able to catch the following unit errors in the near future:
I cannot imagine that we'd consider MCP-0027 resolved without being able to reject both of these with support in the specification. For example, this is how 2 * Modelica.Constants.pi * r * T would be rejected according to #3257:
- Left associativity means that the expression is parsed as
(((2 * Modelica.Constants.pi) * r) * T)(but the end result should be the same also if the factors are reordered). 2is implicitly cast toRealin order for the multiplication with theRealModelica.Constants.pito be defined. Hence2has empty unit.Modelica.Constants.piis a (constant) component reference outside of a function, where the declaredunitis"". Hence, the expressionModelica.Constants.pihas unit"1".- Since one of the operands in
2 * Modelica.Constants.pihas non-empty unit (namelyModelica.Constants.piwith unit""), the inferred unit of2defaults to"1"in order obtain the unit for the product. - The unit of
2 * Modelica.Constants.piis the product"1"*"1"="1". - The unit of
2 * Modelica.Constants.pi * r * Tis the product("1"*"m") *"K"="m.K". - This is a violation of dimensional homogenity for a declaration equation.
Regarding the
R2DandD2Ritems, I have a few more comments:
- These don't appear to be in use in MSL. Probably they should show up in the to_deg and from_deg conversions at least. Otherwise, why even keep them if they aren't used in the most natural spot (conversion function)?
- Even if they are kept, they are redundant (just reciprocals), do we need both of them? We don't have any other reciprocal constants.
Perhaps only to not introduce a backwards incompatible change in the MSL?
Note that it could be tricky do introduce a library conversion annotation that would describe what to do as a replacement for referencing these constants, as it's currently not on the table to perform unit conversions using a Modelica expression. For example, we are currently not considering anything like unit(1, "rad/deg"), which would allow the 1 with empty unit (due to being implicitly cast to Real) to get inferred unit "rad/deg".
Regarding the mention of #2127 above, the unit checking discussions are now going in the direction of having special semantics for constants, deprecating declaration without explicitly setting unit. The idea is that we should have:
final constant Real inf(unit = "") = ModelicaServices.Machine.inf "Biggest Real number such that inf and -inf are representable on the machine";
as the only way of saying that inf will act similarly to a Real literal when used in expressions, for example:
parameter SI.Time startTime = -Modelica.Constants.inf "Output = false for time < startTime";
This would be totally in agreement in the original suggestions made in this issue. I have a hard time seeing that we would regret introducing these explicit units already today, even if the unit checking discussions in MCP-0027 are far from settled.
For completeness and to support the unit checking work, we could also consider adding explicit unit = "" for the remaning constants without unit, and then also add it to corresponding constants in ModelicaServices.Machine. However, I would prefer having separate pull requests for non-empty and empty unit modifications, as the deprecation of omitting the empty ones is a more open unit checking design problem.
Regarding the mention of #2127 above, the unit checking discussions are now going in the direction of having special semantics for constants, deprecating declaration without explicitly setting
unit. The idea is that we should have:final constant Real inf(unit = "") = ModelicaServices.Machine.inf "Biggest Real number such that inf and -inf are representable on the machine";as the only way of saying that
infwill act similarly to aRealliteral when used in expressions, for example:parameter SI.Time startTime = -Modelica.Constants.inf "Output = false for time < startTime";This would be totally in agreement in the original suggestions made in this issue. I have a hard time seeing that we would regret introducing these explicit units already today, even if the unit checking discussions in MCP-0027 are far from settled.
We can also separate them into the ones with explicit unit="1" like pi. I don't see any harm in that at all.
Whether we should add unit="" for the other ones seems less clear.
One possibility is that we introduce a rule that an explicit unit is required, and in that case it sort of circumvents that check, another is that there are no special rules for constants (in which case it would be redundant; but sort of ok), and a third is that the special rule is that we for constants (or possibly all variables with a definition equation) the unit can only be derived from its definition equation (in which case it is redundant; but sort of ok).
We can also separate them into the ones with explicit
unit="1"like pi. I don't see any harm in that at all.
Sure, I'd welcome a PR on that to start with.
Whether we should add
unit=""for the other ones seems less clear.One possibility is that we introduce a rule that an explicit unit is required, and in that case it sort of circumvents that check, another is that there are no special rules for constants (in which case it would be redundant; but sort of ok), and a third is that the special rule is that we for constants (or possibly all variables with a definition equation) the unit can only be derived from its definition equation (in which case it is redundant; but sort of ok).
How would you fix the following if the unit can only be derived from the definition equation?
final constant Real G(final unit = "m3/(kg.s2)") = 6.67408e-11 "Newtonian constant of gravitation (previous value: 6.6742e-11)";
We can also separate them into the ones with explicit
unit="1"like pi. I don't see any harm in that at all.Sure, I'd welcome a PR on that to start with.
Good that we have agreement.
How would you fix the following if the unit can only be derived from the definition equation?
final constant Real G(final unit = "m3/(kg.s2)") = 6.67408e-11 "Newtonian constant of gravitation (previous value: 6.6742e-11)";
I meant that it can be given a unit or derived from the definition equation, but not inferred in other ways.
How would you fix the following if the unit can only be derived from the definition equation?
final constant Real G(final unit = "m3/(kg.s2)") = 6.67408e-11 "Newtonian constant of gravitation (previous value: 6.6742e-11)";I meant that it can be given a unit or derived from the definition equation, but not inferred in other ways.
That makes more sense. However, I would only want this when the definition equation comes with a concrete unit, so that constant Real pi = 3.14 still wouldn't be allowed.
I think we might revisit this for pi after looking at Modelica.Electrical.Analog.Examples - with examples such as CompareTransformers, SeriesResonance where inductance equations involve pi in a way that seems unit-odd (clearly there other unit issues with CompareTransformers). If user models has similar models this change may cause the impression that this is not a minor upgrade for users.
I don't see a similar issue with the other constants.
To clarify how significant: I enabled the proposed unit-checking in Dymola and tested the master-branch, and there were:
- 2301 warnings on master branch.
- 1224 warnings on master if I removed pi.unit="1" (Most (or almost all) are unit-related.)
Clearly it is in one sense good to make the change, but I don't think we can focus that much on those issues.