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

Reliable input file errors #1330

Merged
merged 14 commits into from
Jul 12, 2022
Merged

Conversation

ischoegl
Copy link
Member

@ischoegl ischoegl commented Jul 3, 2022

Changes proposed in this pull request

  • Address InputFileError issues
  • Simplify ReactionRate::check
  • Avoid NAN as sentinel values for ReactionRate initialization status (see --fast-math enhancement issue)

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

Closes #1281, closes #1283, partially addresses Cantera/enhancements#125

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

Loading the following YAML

phases:
- name: gas
  thermo: ideal-gas
  species: [{h2o2.yaml/species: all}]
  kinetics: gas
  reactions: all
  state: {T: 300.0, P: 1 atm, X: {O2: 0.21, N2: 0.79}}

reactions:
- equation: O + H2 <=> H + OH
  rate-constant: {A: 3.87e+04 cm^6/mol^2/s, b: 2.7, Ea: 6260.0}
- equation: 2 OH (+M) <=> H2O2 (+M)
  type: falloff
- equation: CH3 + OH => CH3O + H
  rate-constant: {A: 1.0, b: 0, Ea: 0}

results in the following error:

CanteraError:
*******************************************************************************
CanteraError thrown by addReactions:

*******************************************************************************
InputFileError thrown by UnitSystem::convert:
Error on line 11 of ./test.yaml:
Incompatible units:
    Units(1.0000000000000002e-06 m^6 / kmol^2 / s) and
    Units(m^3 / kmol / s)
|  Line |
|     6 |   reactions: all
|     7 |   state: {T: 300.0, P: 1 atm, X: {O2: 0.21, N2: 0.79}}
|     8 |
|     9 | reactions:
|    10 | - equation: O + H2 <=> H + OH
>    11 >   rate-constant: {A: 3.87e+04 cm^6/mol^2/s, b: 2.7, Ea: 6260.0}
                               ^
|    12 | - equation: 2 OH (+M) <=> H2O2 (+M)
|    13 |   type: falloff
|    14 | - equation: CH3 + OH => CH3O + H
*******************************************************************************

*******************************************************************************
InputFileError thrown by FalloffRate::validate:
Error on line 12 of ./test.yaml:
Rate object for reaction '2 OH (+M) <=> H2O2 (+M)' is not configured.
|  Line |
|     7 |   state: {T: 300.0, P: 1 atm, X: {O2: 0.21, N2: 0.79}}
|     8 |
|     9 | reactions:
|    10 | - equation: O + H2 <=> H + OH
|    11 |   rate-constant: {A: 3.87e+04 cm^6/mol^2/s, b: 2.7, Ea: 6260.0}
>    12 > - equation: 2 OH (+M) <=> H2O2 (+M)
            ^
|    13 |   type: falloff
|    14 | - equation: CH3 + OH => CH3O + H
|    15 |   rate-constant: {A: 1.0, b: 0, Ea: 0}
*******************************************************************************

*******************************************************************************
InputFileError thrown by Reaction::checkSpecies:
Error on line 14 of ./test.yaml:
Reaction 'CH3 + OH => CH3O + H'
contains undeclared species: 'CH3', 'CH3O'
|  Line |
|     9 | reactions:
|    10 | - equation: O + H2 <=> H + OH
|    11 |   rate-constant: {A: 3.87e+04 cm^6/mol^2/s, b: 2.7, Ea: 6260.0}
|    12 | - equation: 2 OH (+M) <=> H2O2 (+M)
|    13 |   type: falloff
>    14 > - equation: CH3 + OH => CH3O + H
            ^
|    15 |   rate-constant: {A: 1.0, b: 0, Ea: 0}
*******************************************************************************
*******************************************************************************

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 force-pushed the reliable-input-file-errors branch 2 times, most recently from 35e260a to e47acf0 Compare July 3, 2022 19:00
@ischoegl ischoegl force-pushed the reliable-input-file-errors branch 3 times, most recently from c55ebfd to d223006 Compare July 4, 2022 16:36
@ischoegl ischoegl marked this pull request as ready for review July 4, 2022 16:47
@ischoegl ischoegl requested a review from a team July 4, 2022 16:48
Copy link
Member

@bryanwweber bryanwweber left a comment

Choose a reason for hiding this comment

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

Just a couple small comments I noticed quickly. I haven't gone through everything 😄

include/cantera/kinetics/Arrhenius.h Outdated Show resolved Hide resolved
interfaces/cython/cantera/test/test_kinetics.py Outdated Show resolved Hide resolved
interfaces/cython/cantera/test/test_kinetics.py Outdated Show resolved Hide resolved
@ischoegl ischoegl force-pushed the reliable-input-file-errors branch 4 times, most recently from c0f05db to 31f82cc Compare July 5, 2022 17:23
@ischoegl ischoegl requested a review from bryanwweber July 5, 2022 19:17
@ischoegl ischoegl added the Input Input parsing and conversion (for example, ck2yaml) label Jul 6, 2022
@bryanwweber
Copy link
Member

@ischoegl This needs a rebase to fix the merge conflict. I'd also like @speth to take a look at this when he has a chance.

Copy link
Member

@bryanwweber bryanwweber left a comment

Choose a reason for hiding this comment

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

A few comments for consideration 😄

src/kinetics/Arrhenius.cpp Show resolved Hide resolved
src/kinetics/ChebyshevRate.cpp Outdated Show resolved Hide resolved
src/kinetics/PlogRate.cpp Outdated Show resolved Hide resolved
src/kinetics/PlogRate.cpp Outdated Show resolved Hide resolved
src/kinetics/PlogRate.cpp Outdated Show resolved Hide resolved
@ischoegl ischoegl force-pushed the reliable-input-file-errors branch 2 times, most recently from f5bc7b6 to 253b6ea Compare July 11, 2022 05:22
@ischoegl
Copy link
Member Author

@bryanwweber / @speth ... rebased and ready for a review.

@speth speth self-requested a review July 11, 2022 15:24
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.

Thanks for this, @ischoegl. Your solution to #1283 was quite a bit cleaner than what I had been expecting to have to do. I just had a couple of minor suggestions.

include/cantera/kinetics/Arrhenius.h Outdated Show resolved Hide resolved
interfaces/cython/cantera/test/test_kinetics.py Outdated Show resolved Hide resolved
src/kinetics/PlogRate.cpp Show resolved Hide resolved
@ischoegl
Copy link
Member Author

@speth ... thanks for the review! I updated per suggestions.

src/kinetics/PlogRate.cpp Outdated Show resolved Hide resolved
Co-Authored-By: Bryan Weber <bryan.w.weber@gmail.com>
Co-Authored-By: Ray Speth <speth@mit.edu>
@ischoegl ischoegl requested a review from speth July 12, 2022 19:51
@ischoegl
Copy link
Member Author

I believe that things are taken care of now.

@speth speth merged commit 89f2c7c into Cantera:main Jul 12, 2022
@ischoegl ischoegl deleted the reliable-input-file-errors branch July 31, 2022 22:43
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
3 participants