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

Fix Modelica.Media.R134a.R134a_ph.setState_pTX (only applicable for one-phase) #3463

Merged
merged 3 commits into from
Mar 2, 2020

Conversation

beutlich
Copy link
Member

@beutlich beutlich commented Feb 29, 2020

After #3317, this is another attempt to fix Modelica.Media.R134a.R134a_ph.setState_pTX.

The property tables, especially for setState_pTX, created by T3276.mo look as expected now, if the density start value for the Newton iteration is increased.

setState_phX() (ref props in parentheses)

row T p d h
1 374.21 4.05911e+06 511.899
(511.9)
389639
(389639)
2 169.85 389.563 1517.14
(1591.11)
71455.2
(71455.2)
3 298.15 3e+06 1220.13
(1220.13)
234687
(234687)
4 305.987 3e+06 1190.76
(1190.76)
245776
(245776)
5 273.15 292803 1294.76
(1294.78)
200000
(200000)
6 298.15 665380 969.978
(969.916)
235741
(235741)

setState_pTX() (ref props in parentheses)

row T p d h
1 374.21 4.05911e+06 501.213
(511.9)
390932
(389639)
2 169.85 389.563 n.a.
(1591.11)
n.a.
(71455.2)
3 298.15 3e+06 ✔️ 1220.13
(1220.13)
✔️ 234687
(234687)
4 305.987 3e+06 ✔️ 1190.76
(1190.76)
✔️ 245776
(245776)
5 273.15 292803 n.a.
(1294.78)
n.a.
(200000)
6 298.15 665380 n.a.
(969.916)
n.a.
(235741)

I also added the test model to ModelicaTest.

Resolves #3276.

@beutlich beutlich added bug Critical/severe issue L: Media Issue addresses Modelica.Media labels Feb 29, 2020
@beutlich beutlich self-assigned this Feb 29, 2020
@@ -1943,12 +1943,10 @@ The function cannot be inverted in a numerical way. Please use functions <a href
if liquid then
damping := 0.3;
//damping on the liquid side
d := R134aData.data.FDCRIT*Common.CubicSplineEval(localx, dl_coef[int,
1:4])*1.02;
d := R134aData.data.FDCRIT*Common.CubicSplineEval(localx, dl_coef[int,1:4])*1.5;
Copy link
Contributor

Choose a reason for hiding this comment

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

So the main change really is using 1.5 instead of 1.02?

As long term solution, the Newton iteration should probably be bounded and have asserts in it for the slope.
Also, we should have two tests: a Modelica test comparing forward/backward consistency of the Newton iterations; and a python script comparing values against RefProp (from python, the most convenient way is to use CoolProp to call RefProp).

Copy link
Member Author

Choose a reason for hiding this comment

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

So the main change really is using 1.5 instead of 1.02?

Yes, it is.

@beutlich beutlich added this to the MSL4.0.0 milestone Mar 2, 2020
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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Possible Modelica.Media bug
3 participants