Have final modifier for these rescalings. #4379
Conversation
Otherwsise these parameters could (as default; depending on tool) be possible to edit after translation. Modifying the rescaling between Pascal and bar (or MW and W) clearly does not make sense.
| annotation (Placement(transformation(extent={{-54,-75.5},{-44,-64.5}}))); | ||
| Modelica.Blocks.Math.Gain Pa2bar(k=1e-5) annotation (Placement( | ||
| Modelica.Blocks.Math.Gain Pa2bar(final k=1e-5) annotation (Placement( |
There was a problem hiding this comment.
I find it rather obscure to leave it for unit inference to figure out the unit of the gain, and I find setting displayUnit a good way to make clear that the numeric value is the correct one:
| Modelica.Blocks.Math.Gain Pa2bar(final k=1e-5) annotation (Placement( | |
| Modelica.Blocks.Math.Gain Pa2bar(final k(unit = "bar/Pa", displayUnit = "1") = 1e-5) annotation (Placement( |
There was a problem hiding this comment.
(If I am not mistaken about final, there is no point putting final before unit when the value modification is already made final.)
| @@ -70,9 +70,9 @@ package DrumBoiler | |||
| Modelica.Blocks.Interfaces.RealOutput V_l(unit="m3") "Liquid volume inside drum" | |||
| annotation (Placement(transformation(extent={{100,74},{112,86}}))); | |||
| public | |||
| Modelica.Blocks.Math.Gain MW2W(k=1e6) | |||
| Modelica.Blocks.Math.Gain MW2W(final k=1e6) | |||
There was a problem hiding this comment.
Similar to Pa2bar but this should be combined with specifying the unit = "MW" for the table source:
Modelica.Blocks.Sources.TimeTable q_F_Tab(table = [0, 0; 3600, 400; 7210, 400], y(unit = "MW")) if not use_inputs annotation(Placement(transformation(extent = {{-90, -80}, {-70, -60}})));
| Modelica.Blocks.Math.Gain MW2W(final k=1e6) | |
| Modelica.Blocks.Math.Gain MW2W(final k(unit = "W/MW", displayUnit = "1") = 1e6) |
There was a problem hiding this comment.
The proposed change by @henrikt-ma makes sense to me
|
My intent was to not do anything regarding units in this PR, but only make the rescalings final - as they are clearly not intended to be changed. |
There was a problem hiding this comment.
As I see it, it is because the units are final that the conversion rates are final. If the intent is to use these blocks for conversion between other units I don't think it makes sense to make the rates final, so I don't see the point of proceeding without also specifying unit. Maybe you can find another reviewer who likes the changes without unit, and if you do I can make a follow-up PR to only add the missing units?
That makes sense. So can some fluid-experts look at the changes? |
| @@ -70,9 +70,9 @@ package DrumBoiler | |||
| Modelica.Blocks.Interfaces.RealOutput V_l(unit="m3") "Liquid volume inside drum" | |||
| annotation (Placement(transformation(extent={{100,74},{112,86}}))); | |||
| public | |||
| Modelica.Blocks.Math.Gain MW2W(k=1e6) | |||
| Modelica.Blocks.Math.Gain MW2W(final k=1e6) | |||
There was a problem hiding this comment.
The proposed change by @henrikt-ma makes sense to me
| annotation (Placement(transformation(extent={{-54,-75.5},{-44,-64.5}}))); | ||
| Modelica.Blocks.Math.Gain Pa2bar(k=1e-5) annotation (Placement( | ||
| Modelica.Blocks.Math.Gain Pa2bar(final k=1e-5) annotation (Placement( |
Otherwise these parameters could (as default; depending on tool) be possible to edit after translation.
Modifying the rescaling between Pascal and bar (or MW and W) clearly does not make sense.