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

Wrong input type in specificEnthalpy_pTX, temperature_phX, and density_phX #4389

Closed
casella opened this issue Apr 16, 2024 · 8 comments
Closed
Assignees
Labels
bug Critical/severe issue L: Media Issue addresses Modelica.Media P: high High priority issue
Milestone

Comments

@casella
Copy link
Contributor

casella commented Apr 16, 2024

Consider this simple MWE:

model TestFunctionDefault
  package Medium = Modelica.Media.Air.SimpleAir;
  Medium.SpecificEnthalpy h = Medium.specificEnthalpy_pT(1e5, 300);
end TestFunctionDefault;

This model compiles in Dymola 2024X and produces the expected result. However, when I try to compile it with OpenModelica, I get this error:

[1] 18:31:01 Translation Error
[Modelica.Media: 4706:7-4709:26]: Type mismatch for positional argument 3 in TestMedia.TestFunctionDefault.Medium.specificEnthalpy_pTX(X=fill(0.0, 0)). The argument has type:
  Integer[0]
expected type:
  Real[1]

This output looks a bit odd at first sight, what's wrong in calling specificEnthalpy_pT(1e5, 300)? I thought it was an OMC bug, but in fact it turns out to be an MSL issue.

specificEnthalpy_pT, as defined in PartialPureSubstance, calls specificEnthalpy_pTX(p, T, fill(0, 0)), which is defined in PartialMedium as specificEnthalpy(setState_pTX(p, T, X)), so it calls setState_pTX with a third argument X = fill(0, 0).

Unfortunately, setState_pTX for SimpleAir is defined in PartialSimpleIdealGasMedium as:

  redeclare function specificEnthalpy_pTX
    "Return specific enthalpy from p, T, and X or Xi"
    extends Modelica.Icons.Function;
    input AbsolutePressure p "Pressure";
    input Temperature T "Temperature";
    input MassFraction X[nX] "Mass fractions";
    output SpecificEnthalpy h "Specific enthalpy at p, T, X";
  algorithm
    h := cp_const*(T - T0);
  end specificEnthalpy_pTX;

with input MassFraction X[nX] instead of MassFraction X[:]. Since by default nX = size(substanceNames, 1) = 1, it then expects an array of dimension one, but instead gets an array of dimension zero, coming from fill(0,0).

I'm not sure why Dymola does not complain about this issue, but for sure it needs to be fixed. This holds for functions specificEnthalpy_pTX, temperature_phX, and density_phX in both PartialSimpleMedium and PartialSimpleIdealGasMedium.

@beutlich
Copy link
Member

SimulationX also reports the dimension error

Error in TestFunctionDefault: Wrong argument dimension.
Help: Check.DimCallArg
 
h=Medium.specificEnthalpy_pT(100000.,300);
The argument 3 (fill(0,0)) in the function call has the wrong dimension ([0]-vector). Expected dimension [1]-vector.

I wonder why OMC deduced type Integer[0] from fill(0,0). Should it be fill(0.0,0)?

@perost
Copy link
Contributor

perost commented Apr 16, 2024

I wonder why OMC deduced type Integer[0] from fill(0,0). Should it be fill(0.0,0)?

OMC would normally convert it to fill(0.0, 0) automatically, but that's done after checking if the types match. So the error message just uses the actual types rather than what the types would've been had they been compatible.

@beutlich
Copy link
Member

beutlich commented Apr 19, 2024

OK, there also is #4393 now which is independent of the dimension error.

@beutlich beutlich modified the milestones: MSL4.1.0, MSL4.2.0 May 22, 2024
@casella
Copy link
Contributor Author

casella commented Jun 26, 2024

This issue is causing regressons in OMC, so I would put it in MSL 4.1.0, I don't see any reason to postpone it to 4.2.0. The proposed modification is actually less restrictive, so there should be no backwards compatibility problems.

@casella casella added the P: high High priority issue label Jun 26, 2024
@casella casella modified the milestones: MSL4.2.0, MSL4.1.0 Jun 26, 2024
@casella casella reopened this Jun 27, 2024
@casella
Copy link
Contributor Author

casella commented Jun 27, 2024

We need to cherry pick #4390 to maint/4.1.x so we get this fixed in the forthcoming release.

@casella
Copy link
Contributor Author

casella commented Jul 31, 2024

We need to cherry pick #4390 to maint/4.1.x so we get this fixed in the forthcoming release.

@Esther-Devakirubai can you please take care of that? Thanks!

@Esther-Devakirubai
Copy link
Contributor

We need to cherry pick #4390 to maint/4.1.x so we get this fixed in the forthcoming release.

@Esther-Devakirubai can you please take care of that? Thanks!

@casella Yes Sure! Thanks

Esther-Devakirubai pushed a commit that referenced this issue Aug 16, 2024
@Esther-Devakirubai
Copy link
Contributor

#4390 backported to maint/4.1.x by #4453

@casella casella closed this as completed Aug 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Critical/severe issue L: Media Issue addresses Modelica.Media P: high High priority issue
Projects
None yet
Development

No branches or pull requests

4 participants