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

[yaml2ck] Fix #1415 #1420

Merged
merged 1 commit into from Jan 4, 2023
Merged

[yaml2ck] Fix #1415 #1420

merged 1 commit into from Jan 4, 2023

Conversation

ischoegl
Copy link
Member

@ischoegl ischoegl commented Dec 31, 2022

Changes proposed in this pull request

Skip third body efficiencies in CK output when third-body collider is explicitly specified

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

Closes #1415

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

@ischoegl ischoegl added the Input Input parsing and conversion (for example, ck2yaml) label Dec 31, 2022
@codecov
Copy link

codecov bot commented Dec 31, 2022

Codecov Report

Merging #1420 (94b6d0a) into main (2ce92ea) will increase coverage by 0.00%.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##             main    #1420   +/-   ##
=======================================
  Coverage   70.78%   70.78%           
=======================================
  Files         373      373           
  Lines       53872    53873    +1     
  Branches    17579    17579           
=======================================
+ Hits        38131    38133    +2     
+ Misses      13339    13338    -1     
  Partials     2402     2402           
Impacted Files Coverage Δ
interfaces/cython/cantera/yaml2ck.py 72.24% <100.00%> (+0.09%) ⬆️
interfaces/cython/cantera/reaction.pyx 80.41% <0.00%> (+0.18%) ⬆️

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

@ischoegl ischoegl requested review from bryanwweber and a team and removed request for bryanwweber January 3, 2023 08:41
speth
speth previously approved these changes Jan 3, 2023
Copy link
Member

@speth speth left a comment

Choose a reason for hiding this comment

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

Looks good to me. My only request is for a descriptive commit message rather than just a reference to the GitHub issue.

@ischoegl
Copy link
Member Author

ischoegl commented Jan 4, 2023

Looks good to me. My only request is for a descriptive commit message rather than just a reference to the GitHub issue.

👍 Thanks for the review - commit message is updated!

Prior to this fix, third body efficiencies were erroneously specified
for third body reactions with explicit collision partners (see Cantera#1415).
@speth speth merged commit 957dd24 into Cantera:main Jan 4, 2023
@ischoegl ischoegl deleted the fix-1415 branch February 13, 2023 02:31
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
No open projects
Development

Successfully merging this pull request may close these issues.

yaml2ck converting format error with third body reaction
2 participants