Closed wandadars closed 3 months ago
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 :warning: |
:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.
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 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.
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.
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 ...
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 Pythondict
). I am also unsure whether the namecompute_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?
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.
Ok, I switched to the AnyMap, and that is a more user-friendly option for sure.
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
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.
I have made those formatting updates and added that much better pressure testing unit test.
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.
Checklist
scons build
&scons test
) and unit tests address code coverage