Functions with wrong type argument called in new tank models?
Opened this issue · 5 comments
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:
ThermofluidStream/ThermofluidStream/Boundaries/Internal/PartialTank.mo
Lines 126 to 129 in 38553ff
where
and
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, 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.
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.
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.
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.