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 converting surface mechanisms that use a separate thermo data file #1637

Merged
merged 2 commits into from
Dec 7, 2023

Conversation

speth
Copy link
Member

@speth speth commented Oct 21, 2023

Changes proposed in this pull request

  • Read the surface input file before the "thermo" file so the full list of species for which thermo data will be known when the thermo file is read.
  • Fix the summary printed at the end so the species count includes both bulk and surface phase species

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

This fixes an issue that was originally reported in this Users' Group post

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

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

@speth speth added the Input Input parsing and conversion (for example, ck2yaml) label Oct 21, 2023
@codecov
Copy link

codecov bot commented Oct 21, 2023

Codecov Report

Merging #1637 (0fc24ea) into main (3a9bc6a) will decrease coverage by 0.01%.
The diff coverage is 57.89%.

@@            Coverage Diff             @@
##             main    #1637      +/-   ##
==========================================
- Coverage   72.69%   72.68%   -0.01%     
==========================================
  Files         370      370              
  Lines       56298    56302       +4     
  Branches    20375    20403      +28     
==========================================
- Hits        40925    40923       -2     
+ Misses      12376    12371       -5     
- Partials     2997     3008      +11     
Files Coverage Δ
interfaces/cython/cantera/ck2yaml.py 84.48% <57.89%> (-0.18%) ⬇️

... and 4 files with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@speth speth force-pushed the fix-ck2yaml-surf-thermo branch from 74d0d6d to 0fc24ea Compare October 21, 2023 22:06
@speth speth requested a review from a team December 6, 2023 15:40
Copy link
Member

@ischoegl ischoegl left a comment

Choose a reason for hiding this comment

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

Thanks, @speth! This looks good to me.

PS: there is a failing Linter test, but as it is in the input file it’s not a major issue to me.

@speth
Copy link
Member Author

speth commented Dec 7, 2023

The failing linter test is expected -- the point was to test messy input with tabs and trailing spaces. I didn't think it was worth trying to add exclude rules, given this won't show up again unless there are modifications to this or other similar input files.

@speth speth merged commit 8563be1 into Cantera:main Dec 7, 2023
@speth speth deleted the fix-ck2yaml-surf-thermo branch December 7, 2023 02:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Input Input parsing and conversion (for example, ck2yaml)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants