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 misleading error message from invalid reaction #1356

Merged
merged 2 commits into from
Aug 7, 2022

Conversation

speth
Copy link
Member

@speth speth commented Aug 7, 2022

Changes proposed in this pull request

  • Raise exceptions resulting from invalid reactions before checking for issues with duplicate reactions.
  • Use the "fast-fail in debug mode" strategy for all possible sources of reactions

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

Fixes #1353

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

This extends the behavior of failing fast on invalid reactions when in
debug mode to the case where the reactions are in a different file from
the phase definition.
@speth speth added Input Input parsing and conversion (for example, ck2yaml) Kinetics labels Aug 7, 2022
@codecov
Copy link

codecov bot commented Aug 7, 2022

Codecov Report

Merging #1356 (919d990) into main (b35ed2f) will not change coverage.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##             main    #1356   +/-   ##
=======================================
  Coverage   68.15%   68.15%           
=======================================
  Files         327      327           
  Lines       42659    42659           
  Branches    17180    17180           
=======================================
  Hits        29075    29075           
  Misses      11291    11291           
  Partials     2293     2293           
Impacted Files Coverage Δ
src/kinetics/KineticsFactory.cpp 93.40% <100.00%> (ø)

📣 Codecov can now indicate which changes are the most critical in Pull Requests. Learn more

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.

🎉 ... wouldn't have thought of that. Confirming that this fixes the underlying issue in #1353.

@ischoegl ischoegl merged commit ee1ad0b into Cantera:main Aug 7, 2022
@speth speth deleted the fix-1353 branch July 23, 2024 15:33
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) Kinetics
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Duplicate reaction not found when marked properly
2 participants