ModelicaStandardLibrary icon indicating copy to clipboard operation
ModelicaStandardLibrary copied to clipboard

Avoid wildcard effect in default unity gains

Open henrikt-ma opened this issue 7 months ago • 10 comments

Too often, I have been bitten by the hidden gains in blocks such as Integrator and Add, where unit errors are absorbed by inferring unexpected units for these gains with value 1. This PR comes in three parts corresponding to the initial three commits:

  • Introduce a constant to use instead of a literal 1 when one wants a 1 with unit "1".
  • Use the new constant for the default gains where one doesn't want the wildcard effect.
  • Fix detected unit errors where the affected blocks are used.

I must warn that I will soon be off for vacation, and may not be able to respond to feedback until July 7.

henrikt-ma avatar Jun 10 '25 16:06 henrikt-ma

I agree with you on the fact of getting weird units ... Fine with me to have a constant "one" (1 with unit "1"), but rather dangerous or misleading for integrator and some other. Let's look at the integrator: der(y) = k*u; So if u has unit "1" and you expect y to have unit "1" k has to have unit "1/s". That's the drawback of having signals without units (or having unit "1"). In other cases unit "1" for k is ok: If u has unit "m/s" and you expect y to have unit "m" k has to have unit "1". I need some more time to check the usage in control applications where normally we use k = 1/(integral time constant).

AHaumer avatar Jun 10 '25 17:06 AHaumer

By the way, I believe that those removed unitTime were an accidental consequence of the unfortunate period during which the k in Gain was given unit = "1".

henrikt-ma avatar Jun 10 '25 17:06 henrikt-ma

So if u has unit "1" and you expect y to have unit "1" k has to have unit "1/s". That's the drawback of having signals without units (or having unit "1").

To me this is not a drawback, it is precisely how it should be!

henrikt-ma avatar Jun 10 '25 17:06 henrikt-ma

I need some more time to check the usage in control applications where normally we use k = 1/(integral time constant).

I recommend taking a look at the bugfixes for PID and LimPID in this PR.

henrikt-ma avatar Jun 10 '25 17:06 henrikt-ma

So if u has unit "1" and you expect y to have unit "1" k has to have unit "1/s". That's the drawback of having signals without units (or having unit "1").

To me this is not a drawback, it is precisely how it should be!

That's ok (Sometimes I prefer having signals with units). But in that case integrator.k should have unit "1/s" instead of "1".

AHaumer avatar Jun 10 '25 19:06 AHaumer

That's ok (Sometimes I prefer having signals with units). But in that case integrator.k should have unit "1/s" instead of "1".

Right, and the new default is not preventing this. One alternative is to modify the unit along with the value of the gain:

  Modelica.Blocks.Sources.Constant setpoint(k(unit = "m") = 3.0);
  Modelica.Blocks.Math.Feedback feedback;
  Modelica.Blocks.Continuous.Integrator integrator(k(unit = "1/s") = 10.0);
equation
  connect(setpoint.y, feedback.u1);
  connect(feedback.y, integrator.u);
  connect(integrator.y, feedback.u2);

A better alternative in my opinion is to introduce a new time constant:

  parameter Modelica.Units.SI.Time tau = 0.1;
  Modelica.Blocks.Continuous.Integrator integrator(final k = 1 / tau);

Note how the modification of k completely replaces the default modifier with Modelica.Constants.one which doesn't even have the same unit. This would not have been possible if Integrator would have come with k(unit = "1") = 1 as an attempt to avoid the wildcard effect, since a modifier for k alone would not replace the unit.

Edit: Not that I would recommend it, but it is also possible to opt in for the wildcard effect and have the unit of k determined by unit inference (I strongly dislike specifying numeric values which will become associated with units that are not obvious from the immediate vicinity of the literal):

Modelica.Blocks.Continuous.Integrator integrator(k = 10);

The important part of the change in this PR is that when all you want is pure integration and you don't modify k, this wildcard effect is avoided.

henrikt-ma avatar Jun 10 '25 20:06 henrikt-ma

I should also mention the relation to https://github.com/modelica/ModelicaSpecification/pull/3688. With that syntax, the definition of Modelica.Constants.one would become obsolete, as one could just write 1'1' instead of referring to the constant. There would also be additional convenient alternatives for how to parameterize the integrator in my example above:

  Modelica.Blocks.Continuous.Integrator integrator(k = 10'1/s');

or

  Modelica.Blocks.Continuous.Integrator integrator(k = 1 / 0.1's');

(Unfortunately, I am not allowed to explain how the syntax can be enabled in Wolfram System Modeler, but let me know if you would like a demonstration.)

henrikt-ma avatar Jun 10 '25 21:06 henrikt-ma

  Modelica.Blocks.Continuous.Integrator integrator(k = 10'1/s');

I like that idea! Additionally, it should be possible to adapt the unit not only textually but also in the graphical menu.

AHaumer avatar Jun 11 '25 16:06 AHaumer

The proposal looks good to me, but since I'm not a language specialist I'd like to hear the opinion of e.g. @HansOlsson

AHaumer avatar Jun 11 '25 16:06 AHaumer

I like that idea! Additionally, it should be possible to adapt the unit not only textually but also in the graphical menu.

So far so good, but now we must make sure that this side-topic is not growing too big in the discussion of this PR. To be continued in https://github.com/modelica/ModelicaSpecification/pull/3688.

henrikt-ma avatar Jun 11 '25 21:06 henrikt-ma