Fix open WaterPropsIAPWS issues / add WaterSSTP as alternative Water backend by ischoegl · Pull Request #921 · Cantera/cantera

ischoegl · 2020-08-27T23:19:40Z

There are some open issues with WaterPropsIAWSS; adding access to WaterSSTP via an alternative backend to the Water wrapper facilitates interactions with underlying C++ objects.

The WaterPropsIAPWS helper classes implement a highly accurate Industrial Formulation for the equation of state for pure water, which is currently used by WaterSSTP and PDSS_Water classes. The former is loadable via YAML as part of the ThermoFactory approach, but only implements the liquid portion. Per @bryanwweber ’s observations (in #721):

The WaterSSTP phase, as implied by the name in the XML file (PureLiquidWater), is intended to represent only the liquid states of water. It is based on the WaterPropsIAPWS class, which can represent liquid to vapor states, but the WaterSSTP class is used in a few places where liquids only are the assumption. Hypothetically, a different class that implements the full behavior of the EOS could be written using WaterPropsIAPWS, but this ain't it.

The current loader is cumbersome and involves a custom input file, e.g.

water = ct.PureFluid('test/data/liquid-water.xml')

from within the cantera source folder (see #721), i.e. suitable input files aren’t distributed outside the test suite. The proposed change enables loading of the WaterSSTP model via

water = ct.Water(backend='IAPWS95')

while the existing behavior remains the default. Beyond, this PR also fixes some open issues.

Changes proposed in this pull request

Add phase using liquid-water-IAPWS95 to liquidvapor.yaml
Add an alternative backend=iawps95 switch to the existing Python Water interface to facilitate usage to the liquid-water-IAPWS95 thermo model; the existing behavior corresponds to backend=default
Update Python docstrings with information already available in MATLAB wrapper classes
Clarify role of WaterSSTP helper classes (based on @24sharkS 's solution in Improve WaterSSTP helper class documentation. #809)
Ensure that state of WaterSSTP remains liquid (or supercritical) - this checks the classical criterion for saturated conditions.

If applicable, fill in the issue number this pull request is fixing

Fixes #577, fixes #646, fixes #721

Related Issues

Cantera/enhancements#6, #809

Checklist

There is a clear use-case for this code change
The commit message has a short title & references relevant issues
Build passes (scons build & scons test) and unit tests address code coverage
The pull request is ready for review

Other thoughts

Short-term, this fixes some issues and (hopefully) helps clarify the IAPWS formulation.
Long-term, it may make sense to add CoolProp objects as additional alternative backend s for existing wrapper objects, see Implement an interface to the CoolProp library enhancements#35.

codecov · 2020-08-28T19:12:49Z

Codecov Report

Merging #921 (d66d32d) into main (0a69c0a) will increase coverage by 0.02%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##             main     #921      +/-   ##
==========================================
+ Coverage   71.26%   71.28%   +0.02%     
==========================================
  Files         377      377              
  Lines       46299    46323      +24     
==========================================
+ Hits        32993    33020      +27     
+ Misses      13306    13303       -3

Impacted Files	Coverage Δ
include/cantera/thermo/WaterPropsIAPWS.h	`100.00% <ø> (ø)`
src/tpx/HFC134a.h	`100.00% <ø> (ø)`
include/cantera/oneD/OneDim.h	`52.45% <100.00%> (+1.61%)`	⬆️
include/cantera/thermo/WaterSSTP.h	`66.66% <100.00%> (+6.66%)`	⬆️
src/equil/vcs_prep.cpp	`76.92% <100.00%> (ø)`
src/oneD/Domain1D.cpp	`85.38% <100.00%> (ø)`
src/oneD/Sim1D.cpp	`81.10% <100.00%> (ø)`
src/thermo/ThermoPhase.cpp	`76.57% <100.00%> (+0.14%)`	⬆️
src/thermo/WaterPropsIAPWS.cpp	`43.37% <100.00%> (+1.96%)`	⬆️
src/thermo/WaterSSTP.cpp	`81.28% <100.00%> (+0.86%)`	⬆️
... and 2 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 0a69c0a...62f9d7d. Read the comment docs.

speth

While I appreciate some of the improvements here, and think they're worth including, I am concerned about making the current implementation of the IAPWS95 equation of state more user accessible because it is so incomplete when it comes to the two phase and vapor regions. Without being able to handle those portions of the phase diagram, I don't think it makes sense to include a phase definition in liquidvapor.yaml.

And given the prospect of using CoolProp (Cantera/enhancements#35), I'm not sure it would be worth extending the implementation of this phase definition to handle those regions.

ischoegl · 2020-09-02T03:20:28Z

@speth ... thank you for the review! I believe all is taken care of ... see 1069692

While I appreciate some of the improvements here, and think they're worth including, I am concerned about making the current implementation of the IAPWS95 equation of state more user accessible because it is so incomplete when it comes to the two phase and vapor regions. Without being able to handle those portions of the phase diagram, I don't think it makes sense to include a phase definition in liquidvapor.yaml.

With the changes of this PR, appropriate errors are raised if a user attempts to set a state outside of the permissible region (and it's only possible to deactivate in C++, where caveats are now clearly stated). Since IAPWS is presumably more accurate, I believe it's in the interest of users to be able to access what's already implemented. Perhaps someone will find this of use and take initiative to expand further: the Python front-end is certainly a more convenient starting point.

And given the prospect of using CoolProp (Cantera/enhancements#35), I'm not sure it would be worth extending the implementation of this phase definition to handle those regions.

I personally agree, but am not sure what will become of PDSS_Water (as much of the code is shared with WaterSSTP, the Python access would still be useful). My obvious recommendation would be to simply allow for different backends for all wrappers once the CoolProp connection is implemented.

ischoegl · 2020-09-06T19:20:34Z

A small change takes care of #577. The updated Python Water wrapper was convenient to track this issue down 😉

ischoegl · 2020-12-02T21:23:25Z

Inactive for 3 months - closing.

ischoegl · 2020-12-04T03:53:07Z

Based on a UG post there may be value in wrapping this up after all

ischoegl · 2021-01-19T02:14:05Z

@bryanwweber and @speth ... let me know if this is of interest for 2.5 (I believe all feedback is taken care of). It’s the last of my pending PRs.

- Docstrings are updated to include information already available in MATLAB (or C++ base classes)

Co-authored by: Shekhar Shukla <47356149+24sharkS@users.noreply.github.com>

While the current 'liquid-water-IAPWS95' thermo model only implements liquid (and supercritical) states, the underlying IAPWS formulation can be extended beyond in future revisions. - avoid 'liquid' as part of the nomenclature - YAML phase name is updated to `water-iapws95` - Instead of creating a new wrapper, `iapws95` is added as an alternative backend to the existing `Water` class

Some initial guesses fail (see GitHUb issue Cantera#577, or starting with an initial density guess equl to the critical density); running an alternative initial guess prevents spurious failures (fix is a band-aid).

ischoegl · 2021-01-29T14:55:42Z

.... Triggering CI

speth

Overall, I think this looks pretty good. I'm happy to merge this pending my one suggestion below.

ischoegl · 2021-01-29T17:11:45Z

@speth ... thanks for the review (and the ones that came before) as well as all the work towards 2.5 ...

speth · 2021-01-29T18:39:18Z

No problem. As you can tell from some of the recent activity, @bryanwweber and I are just working out the last few things that need to be taken care of for the 2.5.0 release.

ischoegl force-pushed the liquid-water branch from 0b0a57a to 9b90556 Compare August 28, 2020 02:51

ischoegl changed the title ~~Add Python convenience wrapper for liquid-water-IAPWS95~~ Convenience wrapper for WaterSSTP / improve documentation Aug 28, 2020

ischoegl force-pushed the liquid-water branch 4 times, most recently from 45c9a0e to 422792c Compare August 28, 2020 18:57

ischoegl changed the title ~~Convenience wrapper for WaterSSTP / improve documentation~~ Add Python wrapper for WaterSSTP / enforce model assumption Aug 28, 2020

ischoegl force-pushed the liquid-water branch from 52d4cfb to f630a4d Compare August 30, 2020 13:31

ischoegl changed the title ~~Add Python wrapper for WaterSSTP / enforce model assumption~~ Add WaterSSTP as alternative Water backend / enforce model assumption Aug 30, 2020

ischoegl force-pushed the liquid-water branch from f630a4d to 5c6a7df Compare August 30, 2020 13:53

speth reviewed Sep 1, 2020

View reviewed changes

ischoegl force-pushed the liquid-water branch from fa548e2 to d5c5d1b Compare September 2, 2020 03:09

ischoegl requested a review from speth September 2, 2020 03:49

ischoegl force-pushed the liquid-water branch 2 times, most recently from 9c83577 to 1069692 Compare September 2, 2020 12:26

ischoegl mentioned this pull request Sep 6, 2020

PDSS Water calculation error at 287.9K #577

Closed

ischoegl changed the title ~~Add WaterSSTP as alternative Water backend / enforce model assumption~~ Fix open WaterPropsIAPWS issues / add WaterSSTP as alternative Water backend Sep 6, 2020

ischoegl closed this Dec 2, 2020

ischoegl reopened this Dec 4, 2020

ischoegl force-pushed the liquid-water branch from 1a644ff to 79e3616 Compare December 11, 2020 03:05

ischoegl mentioned this pull request Dec 11, 2020

[CI] Update CI workflow #946

Merged

4 tasks

ischoegl added 3 commits January 19, 2021 14:23

[thermo] Add convenience class for liquid-water-IAPWS95

ac59ebe

[thermo] Update docstrings for PureFluid convenience wrapper classes

e696836

- Docstrings are updated to include information already available in MATLAB (or C++ base classes)

Minor formatting changes

d08e0b1

ischoegl added 10 commits January 19, 2021 14:23

[thermo] Improve documentation of WaterSSTP helper classes

b929c02

Co-authored by: Shekhar Shukla <47356149+24sharkS@users.noreply.github.com>

[thermo] Ensure that WaterSSTP satisfies model assumptions

7a8407b

[thermo] Ensure that WaterPropsIAPWS::density works for critical point

721e6e0

[thermo] Implement allowGasPhase override for WaterSSTP

25dfb2b

[unittests] Add/update tests for LiquidWater object

ffef9e3

[thermo] Implement phaseOfMatter for WaterSSTP class

b54ba6e

[thermo] Clarify WaterSSTP::type

fbc2657

[thermo] Finalize liquid-water-IAPWS95 updates

7abfaa1

[thermo] Avoid spurious WaterPropsIAPWS::density failures

4b3bf78

Some initial guesses fail (see GitHUb issue Cantera#577, or starting with an initial density guess equl to the critical density); running an alternative initial guess prevents spurious failures (fix is a band-aid).

ischoegl force-pushed the liquid-water branch from 79e3616 to 4b3bf78 Compare January 19, 2021 20:33

ischoegl closed this Jan 29, 2021

ischoegl reopened this Jan 29, 2021

speth requested changes Jan 29, 2021

View reviewed changes

Comment thread include/cantera/thermo/WaterSSTP.h Outdated

Mark _allowGasPhase override as not intended for general use

62f9d7d

speth approved these changes Jan 29, 2021

View reviewed changes

speth merged commit 85b8f23 into Cantera:main Jan 29, 2021

ischoegl deleted the liquid-water branch October 17, 2021 02:41

Uh oh!

Conversation

ischoegl commented Aug 27, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codecov Bot commented Aug 28, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

speth left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

ischoegl commented Sep 2, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ischoegl commented Sep 6, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ischoegl commented Dec 2, 2020

Uh oh!

ischoegl commented Dec 4, 2020

Uh oh!

ischoegl commented Jan 19, 2021

Uh oh!

ischoegl commented Jan 29, 2021

Uh oh!

speth left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

ischoegl commented Jan 29, 2021

Uh oh!

speth commented Jan 29, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

ischoegl commented Aug 27, 2020 •

edited

Loading

codecov Bot commented Aug 28, 2020 •

edited

Loading

ischoegl commented Sep 2, 2020 •

edited

Loading

ischoegl commented Sep 6, 2020 •

edited

Loading