Skip to content

Implement serialization of Peng-Robinson phases#1180

Merged
bryanwweber merged 6 commits intoCantera:mainfrom
speth:serialize-PengRobinson
Feb 8, 2022
Merged

Implement serialization of Peng-Robinson phases#1180
bryanwweber merged 6 commits intoCantera:mainfrom
speth:serialize-PengRobinson

Conversation

@speth
Copy link
Copy Markdown
Member

@speth speth commented Jan 20, 2022

Changes proposed in this pull request

  • Implement serialization of Peng-Robinson phases
  • Update Redlich-Kwong serialization to save critical parameters for species where those were provided instead of R-K specific parameters
  • Some formatting cleanup of the PengRobinson class
  • Fix incorrect acentric factors for O2 and N2 in critical-properties.yaml

If applicable, provide an example illustrating new features this pull request is introducing

import cantera as ct
p = ct.Solution('test/data/thermo-models.yaml', 'CO2-PR')
print(p.write_yaml(skip_user_defined=True))
...
species:
  - name: CO2
    composition: {C: 1.0, O: 2.0}
    thermo:
      model: NASA7
      temperature-ranges: [200.0, 1000.0, 3500.0]
      data:
        - [2.35677352, 8.98459677e-03, -7.12356269e-06, 2.45919022e-09,
        -1.43699548e-13, -4.83719697e+04, 9.90105222]
        - [3.85746029, 4.41437026e-03, -2.21481404e-06, 5.23490188e-10,
        -4.72084164e-14, -4.8759166e+04, 2.27163806]
    equation-of-state:
      model: Peng-Robinson
      a: 3.958134e+05
      b: 0.0266275
      acentric-factor: 0.228
      binary-a:
        H2O: 7.897e+06
...

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
Copy link
Copy Markdown

codecov bot commented Jan 20, 2022

Codecov Report

Merging #1180 (0d4cef3) into main (fcff592) will increase coverage by 0.05%.
The diff coverage is 96.26%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #1180      +/-   ##
==========================================
+ Coverage   65.35%   65.40%   +0.05%     
==========================================
  Files         315      318       +3     
  Lines       45999    46178     +179     
  Branches    19531    19639     +108     
==========================================
+ Hits        30062    30203     +141     
- Misses      13445    13470      +25     
- Partials     2492     2505      +13     
Impacted Files Coverage Δ
include/cantera/thermo/MixtureFugacityTP.h 0.00% <ø> (ø)
include/cantera/thermo/RedlichKwongMFTP.h 100.00% <ø> (ø)
include/cantera/thermo/ThermoPhase.h 30.57% <ø> (ø)
src/thermo/MixtureFugacityTP.cpp 37.50% <ø> (+0.13%) ⬆️
src/thermo/RedlichKwongMFTP.cpp 71.18% <88.88%> (+0.66%) ⬆️
src/thermo/PengRobinson.cpp 85.62% <98.73%> (+0.79%) ⬆️
include/cantera/thermo/PengRobinson.h 100.00% <100.00%> (ø)
src/transport/TransportBase.cpp 25.00% <0.00%> (-6.71%) ⬇️
src/base/Solution.cpp 79.51% <0.00%> (-5.53%) ⬇️
include/cantera/kinetics/Kinetics.h 36.52% <0.00%> (-2.28%) ⬇️
... and 14 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 fcff592...0d4cef3. Read the comment docs.

Copy link
Copy Markdown
Member

@decaluwe decaluwe left a comment

Choose a reason for hiding this comment

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

Thanks so much, @speth, both for the new capabilities and the significant cleanup of PengRobinson.

Everything looks good to me. I haven't downloaded and run installation to test, figuring that the test coverage would catch issues, but LMK if you'd like me to take that step.

No "real" changes below, only two comment lines that got moved that I think we'd probably be fine deleting.

@speth speth force-pushed the serialize-PengRobinson branch from 18cdc8c to 0d4cef3 Compare January 22, 2022 18:21
@speth speth requested a review from decaluwe January 22, 2022 18:55
@bryanwweber
Copy link
Copy Markdown
Member

@decaluwe Is this good to merge?

Copy link
Copy Markdown
Member

@decaluwe decaluwe left a comment

Choose a reason for hiding this comment

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

LGTM - thanks, @speth

@bryanwweber bryanwweber merged commit 8c3a1b5 into Cantera:main Feb 8, 2022
@speth speth deleted the serialize-PengRobinson branch February 8, 2022 23:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants