Skip to content
New issue

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

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

Already on GitHub? # to your account

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

HansOlsson
Copy link
Contributor

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.

There are multiple alternatives here, but this seemed like the smallest change.
Compare to Modelica.Electrical.Analog.Interfaces.IdealSemiconductor
@HansOlsson HansOlsson added the L: Fluid Issue addresses Modelica.Fluid (excl. Dissipation) label Apr 24, 2024
Copy link
Member

@beutlich beutlich left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

@HansOlsson
Copy link
Contributor Author

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
Copy link
Member

beutlich commented Jun 5, 2024

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
Copy link
Contributor Author

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
Copy link
Member

beutlich commented Jun 5, 2024

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?

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
L: Fluid Issue addresses Modelica.Fluid (excl. Dissipation)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants