Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Common unit inconsistency pattern #3416

Open
qlambert-pro opened this issue Sep 18, 2023 · 13 comments
Open

Common unit inconsistency pattern #3416

qlambert-pro opened this issue Sep 18, 2023 · 13 comments

Comments

@qlambert-pro
Copy link
Collaborator

Here are concrete examples that I believe should inform our unit specification effort. Here is a small model capturing the inconsistencies in Modelica.Electrical.Machines.Utilities.ParameterRecords.SM_ReluctanceRotorData:

record R "Common parameters for synchronous machines with reluctance rotor"
  import Modelica.Units.SI;
  import Modelica.Constants.pi;
  parameter SI.Frequency fsNominal = 50 "Nominal frequency";
  parameter SI.Inductance Lmd = 2.9 / (2 * pi * fsNominal) "Stator main field inductance per phase in d-axis";
  parameter SI.Inductance Lmq = 0.9 / (2 * pi * fsNominal) "Stator main field inductance per phase in q-axis";
  parameter SI.Inductance Lrsigmad = 0.05 / (2 * pi * fsNominal) "Damper stray inductance in d-axis";
end R;

The bindings of Lmd, Lmq and Lrsigmad are found to be expressed in "1/Hz", which is obviously different from the expected unit.

Similarly Modelica.Electrical.Analog.Examples.CompareTransformers has the following pattern:

model M
  import Modelica.Units.SI;
  import Modelica.Constants.pi;

  parameter SI.Frequency f = 10 "Frequency of voltage source";
  parameter Real n = 2 "Turns ratio primary:secondary voltage";

  parameter SI.Inductance L2sigma = 0.05 / (2 * pi * f) / n ^ 2 "Secondary leakage inductance w.r.t. secondary side";
  parameter SI.Resistance R2 = 0.01 / n ^ 2 "Secondary resistance w.r.t. secondary side";
  parameter SI.Resistance RL = 1 / n ^ 2 "Load resistance";
end M;

Here our unit inference finds a unit for n which is not "1".

I suspect both cases come down to the ability to attach a unit to a literal value with a dedicated operator. But in any case our standardization effort should propose a nice solution to this pattern.

@HansOlsson
Copy link
Collaborator

For CompareTransformers it would be possible to have Modelica.Units.SI.Resistance RBase=1 and change the others to L2sigma = 0.05*RBase / (2 * pi * f) / n ^ 2;, R2=0.01*RBase;, and L1sigma=0.05*BaseLoad/(2*pi*f). (Yes, the last one is without n.)

I assume the explanation is that some electrical engineers give inductance as resistance at nominal-frequency (or something like that).

@qlambert-pro
Copy link
Collaborator Author

Right but the pattern is used in many places, let me know if you want me to try and compile an exhaustive list.

@maltelenz
Copy link
Collaborator

If this is a common usage pattern, it at least should be considered if modelers would like to keep using it, even at the cost of getting less unit checking.

Maybe one could have special rules for the units of literals in binding equations (for parameters, or in general). I realize that would mean less unit checking when literals occur in binding equations.

Consider:

model BindingUnits
  constant Real n(unit = "1") = 1;
  parameter Real o1(unit = "Ohm") = 2; // I assume everybody thinks this is fine. 2 has the unit "Ohm"
  parameter Real o2(unit = "Ohm") = 2 / n; // I just divided the 2 "Ohm" by unit "1", so should be fine, right? Is it?
end BindingUnits;

@qlambert-pro
Copy link
Collaborator Author

Calling @casella as I think he might want to opine here at some point.

@mestinso
Copy link

mestinso commented Sep 21, 2023

I think the root cause/issue is the assumption that all bare numbers in formulas should have unit="1".

I just ran into a random example the other day where I had a length parameter and a mass parameter. In that particular situation, the mass was expected to scale roughly with the length, so for convenience I set the default parameter for the mass as follows:

parameter SI.Length L = 3;
parameter SI.Mass m = 20*L/1.5;

Unfortunately, using the assumption that 20 and 1.5 have unit="1", this wouldn't pass unit checks.

Now, to fix this, I could have been verbose and had written the following:

parameter SI.Length L = 3;
parameter SI.Mass m_nominal = 20;
parameter SI.Length L_nominal = 1.5;
parameter SI.Mass m = m_nominal*L/L_nominal;

And sometimes, when there is some precise physical theory, I might want to be this verbose. However, when it's just a rough rule of thumb for the application at hand, I'd really prefer the first implementation.

So, the issue is in the first implementation is that we have m=20*L/1.5 and clearly the values 20 and 1.5 actually have units, so it seems to me that we either need a way to specify what those units are, or a way to specify that the units shouldn't be assumed to be "1" for 20 and 1.5.

Some potential solutions:

  1. Add units with constants in all the appropriate places:
    parameter SI.Mass m = (20*UnitMass)*L/(1.5*UnitLength);
  2. Similar to above, but build something into the language
    parameter SI.Mass m = 20(unit="kg")*L/1.5(unit="m");
    This same feature could even be used to do something like the following to not have to carefully work anything out by the modeler:
    parameter SI.Mass m = 20(unit="")*L/1.5;
  3. Add an annotation to no longer assume the units for 20 and 1.5 are equal to "1"
    parameter SI.Mass m = 20*L/1.5 annotation(relaxDimensionlessAsssumption=true);
    Note that I am specifically suggesting to not assume that the unit for 20 and 1.5 are "1" rather than saying relax unit checks completely, so in other words this would set this would set unit="" for all bare numbers and then perform unit checks. There are various situations where that would be meaningfully different.

I would strongly prefer something like (3) or even (2b) since the very situation where I want to do this, I specifically and intentionally do not want to have to be precise and specify the units for every value like in (1) or (2a).

Anyways, that's my 2 cents here. Looking to see some resolution.

@henrikt-ma
Copy link
Collaborator

henrikt-ma commented Sep 22, 2023

2. Similar to above, but build something into the language
parameter SI.Mass m = 20(unit="kg")*L/1.5(unit="m");

We keep coming back to the need for something like this. I've mentioned it elsewhere, but I'll repeat myself here; I think it would be fairly straight-forward to support a very readable syntax like this:

parameter SI.Mass m = 20["kg"] * L / 1.5["m"]; 

(The syntax could be generalized once we have the general subcripting syntax from #3395, as in (2 * 10)["m"]. The requirement would be that the expression inside the parentheses is a Real expression (possibly after coercion), having empty unit.)

Edit: Considering how compactly the solution can be expressed in this way, I would say it qualifies as a solution for those asking for a quick way to jot down an expression that will pass unit checking.

@henrikt-ma
Copy link
Collaborator

3. Add an annotation to no longer assume the units for 20 and 1.5 are equal to "1"
parameter SI.Mass m = 20*L/1.5 annotation(relaxDimensionlessAsssumption=true);

I think the use of a an annotation counteracts the purpose of catering for the quick'n dirty equations, as it probably wouldn't be very easy for tools to support adding such annotations when writing equations. (A component declaration annotation would be easier, but still unlikely to be within easy reach for quick'n dirty declaration equations.)

On the other hand, I can't think of any simple solution to express that one wants to opt in for the wildcard effect. Although I'm generally not in favor of having an annotation for turning unit checking off locally, I'd just like to note that if we had such an annotation, it would make more sense to make use of it in this situation, than to introduce another annotation specifically for opting in for the wildcard effect.

@mestinso
Copy link

On the other hand, I can't think of any simple solution to express that one wants to opt in for the wildcard effect. Although I'm generally not in favor of having an annotation for turning unit checking off locally, I'd just like to note that if we had such an annotation, it would make more sense to make use of it in this situation, than to introduce another annotation specifically for opting in for the wildcard effect.

@henrikt-ma What about what I mentioned in 2b?
parameter SI.Mass m = 20(unit="")*L/1.5; or in the syntax you suggested:
parameter SI.Mass m = 20[""]*L/1.5;

In general, somebody could just drop a 1.0[""] out front to do this.

@henrikt-ma
Copy link
Collaborator

henrikt-ma commented Sep 22, 2023

The problem I see with 1.0[""] is the following:

constant Real one(unit = "") = 1;
Real x(unit = "m") = 1;
Real y1(unit = "m2") = x * 1.0; /* Unit error. */
Real y2(unit = "m2") = x * one; /* Same semantics as for y1, according to current design plans. */
Real y3(unit = "m2") = x * 1.0[""]; /* Surprisingly, no error despite similarity to y2 above. */

@henrikt-ma
Copy link
Collaborator

I would find it more natural to let something like "*" act as the special wildcard unit, but I still wonder if the construct would really be needed if it is easy enough to attach proper units instead?

@mestinso
Copy link

I would find it more natural to let something like "*" act as the special wildcard unit, but I still wonder if the construct would really be needed if it is easy enough to attach proper units instead?

In my simple mass and length example, i might just drop in the real units as it's pretty obvious and lightweight, but for the example that started this discussion, that sounds a little annoying, I might just drop the "" or "*" on the 2.9 here:
parameter SI.Inductance Lmd = 2.9 / (2 * pi * fsNominal) "Stator main field inductance per phase in d-axis";

@mestinso
Copy link

The problem I see with 1.0[""] is the following:

constant Real one(unit = "") = 1;
Real x(unit = "m") = 1;
Real y1(unit = "m2") = x * 1.0; /* Unit error. */
Real y2(unit = "m2") = x * one; /* Same semantics as for y1, according to current design plans. */
Real y3(unit = "m2") = x * 1.0[""]; /* Surprisingly, no error despite similarity to y2 above. */

By the way, regarding this concern: had the potential feature we are talking about been available, wouldn't you have done the following to resolve the issue with the y2 case:
constant Real one = 1[""];
If agreed, I'm not sure I see the real need/distinction in adding "*", so "" should be sufficient I think...

@henrikt-ma
Copy link
Collaborator

By the way, regarding this concern: had the potential feature we are talking about been available, wouldn't you have done the following to resolve the issue with the y2 case:
constant Real one = 1[""];

To me it seems very important that there is a way to define a constant as an abstraction for a literal value or a subexpression without any declared units, and I really like the idea we've had that this is what unit = "" should mean for a constant.

I also think it would be very unfortunate if 1[""] did not mean the same as 1, so constant Real one = 1[""]; would go against the part of the design that deprecates being clear about whether a constant has the special empty unit or a concrete unit that is either declared or obtained from the declaration equation.

If agreed, I'm not sure I see the real need/distinction in adding "*", so "" should be sufficient I think...

That I see the two as different is also a reason for me to not introduce "*", as it opens a new can of design worms. For example, what would the unit inference semantics of this be?

Real x = 1["*"];
Real y(unit = "s") = x;

I'm not looking for the answer to this particular question, but just want to point at the sort of complexity we would add to the design if introducing a new concept for wildcard units.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants