Expose Peng-Robinson EoS parameters to Python interface by wandadars · Pull Request #1664 · Cantera/cantera

wandadars · 2024-01-21T20:51:23Z

This pull request seeks to expose the Peng-Robinson equation of state parameters, such as aAlpha, b, and the first and second derivatives of aAlpha with respect to temperature so that these values are available through the Python interface of the ThermoPhase object. These parameters are often used in the context of compressible flamelet table tabulations.

See: https://arxiv.org/abs/1704.02639

I'm attempting to use the AnyValue object for the return type to generalize a bit. The overall idea I had was to have a catch-all function that can dispatch to method that return values from the eos objects.

def add_additional_data(flame, gas):
    for i in range(flame.flame.n_points):
        # Set the state of the gas to match the local state
        T = flame.T[i]
        P = flame.P
        X = flame.X[:,i]
        gas.TPX = T, P, X

        # Compute additional data
        aAlpha_mix = gas.compute_extra_method('aAlpha_mix')
        b_mix = gas.compute_extra_method('b_mix')
        daAlpha_dT = gas.compute_extra_method('daAlpha_dT')
        d2aAlpha_dT2 = gas.compute_extra_method('d2aAlpha_dT2')
        print(f'aAlpha_mix: {aAlpha_mix}, b_mix: {b_mix}, daAlpha_dT: {daAlpha_dT}, d2aAlpha_dT2: {d2aAlpha_dT2}')

Checklist

The pull request includes a clear description of this code change
Commit messages have short titles and reference relevant issues
Build passes (scons build & scons test) and unit tests address code coverage
Style & formatting of contributed code follows contributing guidelines
The pull request is ready for review

codecov · 2024-01-21T21:11:33Z

Codecov Report

Attention: Patch coverage is 91.66667% with 1 lines in your changes are missing coverage. Please review.

Project coverage is 72.78%. Comparing base (64ed2b7) to head (d0b9a82).
Report is 1 commits behind head on main.

Files	Patch %	Lines
src/thermo/PengRobinson.cpp	87.50%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1664      +/-   ##
==========================================
+ Coverage   72.77%   72.78%   +0.01%     
==========================================
  Files         375      375              
  Lines       56632    56679      +47     
  Branches    20590    20607      +17     
==========================================
+ Hits        41212    41255      +43     
- Misses      12398    12400       +2     
- Partials     3022     3024       +2

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

ischoegl

I am not sure whether I am fully behind hard-coded names as parameters, as users need to know what those are (an alternative would be to return an AnyMap populated with all applicable entries, which can then be mapped to a Python dict). I am also unsure whether the name compute_extra_method is intuitive.

PS: for the failing examples, there's a pending fix in 62300d3

ischoegl · 2024-01-21T21:27:37Z

include/cantera/thermo/PengRobinson.h

+     * @return Computed value wrapped in AnyValue type for Peng-Robinson phase
+     *         quantities
+     */
+    AnyValue compute_extra_method(const std::string& name) override;


Suggested change

AnyValue compute_extra_method(const std::string& name) override;

AnyValue computeExtraMethod(const std::string& name) override;

In C++, Cantera uses mostly camelCase methods rather than snake_case.

ischoegl · 2024-01-21T21:28:18Z

src/thermo/PengRobinson.cpp


+AnyValue PengRobinson::compute_extra_method(const std::string& name)
+{
+    if(name == "aAlpha_mix") {


Suggested change

if(name == "aAlpha_mix") {

if (name == "aAlpha_mix") {

Missing whitespace after if

speth · 2024-01-21T22:32:04Z

I recall us having discussed this idea at some point before. Can you please update the initial post to describe the use case for making these parameters accessible?

wandadars · 2024-01-21T22:36:03Z

I recall us having discussed this idea at some point before. Can you please update the initial post to describe the use case for making these parameters accessible?

I have updated with a reference to a paper showing the compressible flamelet methodology that utilizes tabulations of real-gas parameters to create better flamelet tables.

wandadars · 2024-01-21T23:34:40Z

Now that I've worked on the implementation, essentially all of the additional methods are just calling already defined functions in the PREOS implementation, so those could be exposed instead of adding this new virtual method. I don't know if that is better or worse.

ischoegl · 2024-01-22T00:00:12Z

Now that I've worked on the implementation, essentially all of the additional methods are just calling already defined functions in the PREOS implementation, so those could be exposed instead of adding this new virtual method. I don't know if that is better or worse.

From the perspective of the Python API implementation (or any other language interface), it is easier (preferable?) to have a single function signature that is common to all ThermoPhase models (i.e. the virtual method), which returns implementation-specific values. As mentioned above, it would likewise be good to not force the user to guess or look up what keywords are being used internally, so I'd recommend using the AnyMap route pointed out before. Of course, these are only my 2 cents ...

wandadars · 2024-01-24T01:34:49Z

I am not sure whether I am fully behind hard-coded names as parameters, as users need to know what those are (an alternative would be to return an AnyMap populated with all applicable entries, which can then be mapped to a Python dict). I am also unsure whether the name compute_extra_method is intuitive.

PS: for the failing examples, there's a pending fix in 62300d3

So for an AnyMap return type, you're saying that the function should just collect all parameters and return them all to user instead of letting them choose?

ischoegl · 2024-01-24T02:24:48Z

So for an AnyMap return type, you're saying that the function should just collect all parameters and return them all to user instead of letting them choose?

Correct.

wandadars · 2024-01-24T03:39:11Z

Ok, I switched to the AnyMap, and that is a more user-friendly option for sure.

wandadars · 2024-01-24T03:42:55Z

I have an example here:

import cantera as ct

gas = ct.Solution('co2_PR_example.yaml', 'CO2-PR')
gas.TPX = 300, 101325, 'H2:1.0'
params = gas.get_eos_parameters()

print(params)

This just uses the Cantera co2_PR_example.yaml and the data in critical-properties.yaml

The CO2 file is at: /test/data/ and the critical file is at /build/data

speth

Thanks for implementing this, @wandadars. I think it will be a useful feature, especially in some debugging applications. Besides the specific comments below, this also needs a test, ideally for the Python interface.

include/cantera/thermo/ThermoPhase.h

interfaces/cython/cantera/thermo.pyx

include/cantera/thermo/PengRobinson.h

src/thermo/PengRobinson.cpp

interfaces/cython/cantera/thermo.pyx

…nterface

…trings

wandadars · 2024-03-09T03:33:54Z

Ok. I made some final changes that I had missed when we renamed the method. I added a Python test to the ThermoPhase Python testing file. I'm not well versed in handling of YAML files that have multiple definitions in them (like the co2_PR one), but I think it does the right thing. One thing I'm not 100% sure on with the Peng Robinson specifications is how we can set the Peng Robinson equation of state to be used, but to give it a species that is ideal-like. That way we could potentially test the values of the parameters that are returned, as I believe for a single species, those values would all be zero. I just haven't quite figured out how to formalize such a configuration using Cantera yet.

speth

Thanks for the updates, @wandadars. I only have a couple of minor formatting/style requests.

For the test, I think you should check that the keys returned are what you expect, rather than just the number of keys is correct. I don't think you necessarily need to worry about whether the numerical values are correct in this test. But if you want to check something, you could use the returned values of aAlpha_mix and b_mix along with the current temperature and molar volume to compute the pressure of the mixture using the PR equation of state and see if it matches.

include/cantera/thermo/PengRobinson.h

test/python/test_thermo.py

wandadars · 2024-03-11T22:03:12Z

I have made those formatting updates and added that much better pressure testing unit test.

speth

Thanks, @wandadars. This looks good to me!

ischoegl reviewed Jan 21, 2024

View reviewed changes

ischoegl added the Thermo label Jan 22, 2024

speth self-requested a review February 5, 2024 03:47

speth requested changes Feb 23, 2024

View reviewed changes

wandadars force-pushed the preos_parameter_exposing branch from 9a2fe2a to a2b1615 Compare February 23, 2024 05:05

wandadars added 4 commits March 8, 2024 22:26

[thermo] initial exposing of Peng-Robinson eos parameters to python i…

5eb7a9a

…nterface

changed to AnyMap return type & updated method names

14cd488

[thermo] changed name of auxiliary data access methods & updated docs…

7f1cc52

…trings

updates to interface and added a test for the Python interface

2f751d3

wandadars force-pushed the preos_parameter_exposing branch from a2b1615 to 2f751d3 Compare March 9, 2024 03:29

speth requested changes Mar 9, 2024

View reviewed changes

include/cantera/thermo/PengRobinson.h Outdated Show resolved Hide resolved

test/python/test_thermo.py Outdated Show resolved Hide resolved

improved python test and formatting changes to doc strings

d0b9a82

speth approved these changes Mar 11, 2024

View reviewed changes

speth merged commit 64df109 into Cantera:main Mar 11, 2024

wandadars deleted the preos_parameter_exposing branch March 13, 2024 00:11

	AnyValue compute_extra_method(const std::string& name) override;
	AnyValue computeExtraMethod(const std::string& name) override;

Uh oh!

Conversation

wandadars commented Jan 21, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codecov bot commented Jan 21, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

ischoegl left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ischoegl Jan 21, 2024

Choose a reason for hiding this comment

Uh oh!

ischoegl Jan 21, 2024

Choose a reason for hiding this comment

Uh oh!

speth commented Jan 21, 2024

Uh oh!

wandadars commented Jan 21, 2024

Uh oh!

wandadars commented Jan 21, 2024

Uh oh!

ischoegl commented Jan 22, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

wandadars commented Jan 24, 2024

Uh oh!

ischoegl commented Jan 24, 2024

Uh oh!

wandadars commented Jan 24, 2024

Uh oh!

wandadars commented Jan 24, 2024

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!

Uh oh!

wandadars commented Mar 9, 2024

Uh oh!

speth left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

wandadars commented Mar 11, 2024

Uh oh!

speth left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

wandadars commented Jan 21, 2024 •

edited

Loading

codecov bot commented Jan 21, 2024 •

edited

Loading

ischoegl left a comment •

edited

Loading

ischoegl commented Jan 22, 2024 •

edited

Loading