modelica / ModelicaStandardLibrary

Free (standard conforming) library to model mechanical (1D/3D), electrical (analog, digital, machines), magnetic, thermal, fluid, control systems and hierarchical state machines. Also numerical functions and functions for strings, files and streams are included.
https://doc.modelica.org
BSD 3-Clause "New" or "Revised" License
452 stars 165 forks source link

Unit consistency for s-parameter in Modelica.Fluid.Vessels.PartialLumpedVessel #4398

Open HansOlsson opened 2 months ago

HansOlsson commented 2 months ago

There are multiple alternatives here, but this seemed like the smallest change. Compare to Modelica.Electrical.Analog.Interfaces.IdealSemiconductor

The new indentation of "protected" is consistent with style-guide, previously it was indented less than the class.

HansOlsson commented 3 weeks ago

Why is the curve parameter s unitless (of type Real) and not of type SI.Length or SI.PathLength?

It will be deduced to be of type mass-flow-rate ("kg/s") not "1". There are multiple ways of doing it, but I thought this was the less intrusive.

beutlich commented 3 weeks ago

I am struggeling with the (deduced) unit of s. If the start value fluidLevel_max is of type SI.Height ist should be a length based unit, right?

HansOlsson commented 3 weeks ago

I am struggeling with the (deduced) unit of s. If the start value fluidLevel_max is of type SI.Height ist should be a length based unit, right?

Ah, yes, I messed up.

Yes, it will be length-based i.e. "m", based on the other branch and for the mass-flow branch we do: s[i] = ports[i].m_flow*(unitHeight/unitMassFlow); which means s[i] will be length-based here as well.

The alternative would be something like:

   ... s[..](each unit="1");
...
            s[i] = (fluidLevel - portsData_height[i])/unitHeight;
          elseif inFlow[i] then
            // ports[i] is above fluidLevel and has inflow
            ports[i].p = vessel_ps_static[i];
            s[i] = ports[i].m_flow;
            s[i] = ports[i].m_flow/unitMassFlow;
beutlich commented 3 weeks ago

What about SI.Length[nPorts] s(each start = fluidLevel_max) to make it explicit and to not deduce the min attribute of SI.Height of the start value?