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

Functions with wrong type argument called in new tank models? #195

Open
casella opened this issue Jun 19, 2024 · 5 comments
Open

Functions with wrong type argument called in new tank models? #195

casella opened this issue Jun 19, 2024 · 5 comments

Comments

@casella
Copy link

casella commented Jun 19, 2024

Many of the recently added models, e.g. ThermofluidStream.Boundaries.Tests.TankExtendedTest2 fail in OMC with this error

[ThermofluidStream 1.1.0-main/Boundaries/Internal/partialTank.mo:126:3-127:39:writable]
Error: Type mismatch for positional argument 1 in 
ThermofluidStream.Boundaries.Tests.TankExtendedTest2.tank.Medium.Liquid.density(state=tank.medium.state). 
The argument has type:
  tank.Medium.ThermodynamicState
expected type:
  tank.Medium.Liquid.ThermodynamicState

This is the offending code:

Modelica.Units.SI.Density liquidDensity=Medium.Liquid.density(medium.state)
"density of the liquid in the tank";
Modelica.Units.SI.Density gasDensity=Medium.Gas.density(medium.state)
"density of the gas in the tank";

where

replaceable package Medium =
ThermofluidStream.Media.myMedia.GasAndIncompressible.PartialGasAndIncompressible
"Medium model" annotation (

and

Medium.BaseProperties medium(preferredMediumStates=usePreferredMediumStates);

I'm not sure how Dymola handles this code, but as far as I understand, OMC is right to complain: tank.medium.state is a ThermodynamicState record from an instance of BaseProperties from package ThermofluidStream.Media.myMedia.GasAndIncompressible.JP8DryAir, while the function density() is called once from package Medium.Liquid, i.e. ThermofluidStream.Media.myMedia.Incompressible.Examples.JP8 and once from package Medium.Gas, i.e. ThermofluidStream.Media.myMedia.Air.DryAirNasa.

This cannot work: the density() function and the BaseProperties model instance should both be taken from the same package to be consistent, otherwise how could they compute the right density?

@perost, can you please double-check and comment?

As I understand, you should probably define PartialGasAndIncompressible.BaseProperties to contain two state records, e.g.

Gas.ThermodynamicState state_gas;
Liquid.ThermodynamicState state_liquid;

then, you could have something like

Modelica.Units.SI.Density liquidDensity=Medium.Liquid.density(medium.state_liquid) 
  "density of the liquid in the tank"; 
 Modelica.Units.SI.Density gasDensity=Medium.Gas.density(medium.state_gas) 
  "density of the gas in the tank"; 

of course PartialGasAndIncompressible.BaseProperties can also contain

ThermodynamicState state;

but this state should contain the information about the overall properties of the gas-liquid mixture.

@perost
Copy link

perost commented Jun 19, 2024

@perost, can you please double-check and comment?

Yes, Medium.Liquid.density and Medium.Gas.density both expect a ThermodynamicState record that looks like:

record ThermodynamicState
  AbsolutePressure p;
  Temperature T;
end ThermodynamicState;

But medium.state is an instance of JP8DryAir.ThermodynamicState which looks like:

record ThermodynamicState
  AbsolutePressure p;
  Temperature T;
  MassFraction X[ns];
end ThermodynamicState;

I.e. it has an extra field X. The specification says that function arguments "must agree with the type of the corresponding parameter", which could mean pretty much anything. But we interpret it to mean that they must be type compatible as defined in 6.7, which states that the records must have the same named elements.

@IngelaLind
Copy link
Contributor

IngelaLind commented Jun 20, 2024 via email

@casella
Copy link
Author

casella commented Jun 20, 2024

But we interpret it to mean that they must be type compatible as defined in 6.7, which states that the records must have the same named elements.

I also understood that. The mixture medium has two components, hence X[nS] is a two-dimensional array, while the liquid and gas have X[nS] containing a single scalar. Hence, they cannot be used interchangeably.

Bottom line: this probably works in Dymola in the specific case of two pure components, because you can compute the air and liquid properties as a function of p and T, so X[nS] does not matter. But I'd say this is a bit borderline, and looks to be working a bit like magic. I definitely suggest to define two separate ThermodynamicState records for the two phases, which can also be trivially extended to the case one or both of them are mixtures.

@IngelaLind
Copy link
Contributor

IngelaLind commented Jun 24, 2024

I think the function call should be
Modelica.Units.SI.Density liquidDensity=Medium.Liquid.density(Medium.Liquid.setState_pTX(medium.state.p,medium.state.T,{1}))
"density of the liquid in the tank";
Modelica.Units.SI.Density gasDensity=Medium.Gas.density(Medium.Gas.setState_pTX(medium.state.p,medium.state.T,{1}))
"density of the gas in the tank";
This simulates in Dymola. But that did the previous version as well, without any warnings at all, even with pedantic check.
I have checked that the change works for both the directed and undirected version of the partialTank.

I only work part time and I do not have time to formally suggest the changes until earliest Thursday.

@casella
Copy link
Author

casella commented Jun 24, 2024

Sounds good, it should also work in OMC. BTW, you should be able to skip the {1} input, as the setState_pTX function has a default input value reference_X for it.

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

3 participants