Reaction rate factories

[ischoegl]

Changes proposed in this pull request

Create factory constructors in anticipation of refactoring of Falloff reactions and corresponding rates

• Implement C++ ReactionRateBase::newMultiRate instantiation
• Implement C++ newRate factory constructor
• Generalize Python ReactionRate interface

The Interface now mostly replicates what already exists for Reaction objects. Addresses Cantera/enhancements#87.

Closes #1071 (associated discussion)

Example

YAML input for ReactionRate uses the same information as Reaction. Non-rate specific information is ignored, and rate types are recognized using aliases.

In [1]: import cantera as ct
   ...:  gas = ct.Solution('kineticsfromscratch.yaml')

In [2]: yaml = """
   ...:     equation: H2 + O2 <=> 2 OH
   ...:     type: pressure-dependent-Arrhenius
   ...:     rate-constants:
   ...:     - {P: 0.01 atm, A: 1.2124e+16, b: -0.5779, Ea: 1.08727e+04 cal/mol}
   ...:     - {P: 1.0 atm, A: 4.9108e+31, b: -4.8507, Ea: 2.47728e+04 cal/mol}
   ...:     - {P: 10.0 atm, A: 1.2866e+47, b: -9.0246, Ea: 3.97965e+04 cal/mol}
   ...:     - {P: 100.0 atm, A: 5.9632e+56, b: -11.529, Ea: 5.25996e+04 cal/mol}
   ...:     """

In [3]: rr = ct.ReactionRate.fromYaml(yaml, gas)
   ...: rr
Out[3]: <PlogRate at 7f73992fa060>

In [4]: dd = rr.input_data
   ...: dd
Out[4]: 
{'rate-constants': [{'A': 1.2124e+16,
   'Ea': 45491376.80000001,
   'P': 1013.2500000000003,
   'b': -0.5779},
  {'A': 4.9108e+31, 'Ea': 103649395.2, 'P': 101324.99999999999, 'b': -4.8507},
  {'A': 1.2866e+47, 'Ea': 166508556.0, 'P': 1013249.9999999992, 'b': -9.0246},
  {'A': 5.9632e+56, 'Ea': 220076726.4, 'P': 10132499.999999985, 'b': -11.529}],
 'type': 'PlogRate'}

In [5]: ct.ReactionRate.from_dict(dd, gas)
Out[5]: <PlogRate at 7f73ce3d7660>

For the time being, I am sticking with the usual nomenclature (see #1053)

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]

Codecov Report

Merging #1061 (39d4577) into main (3dc940d) will increase coverage by 0.51%.
The diff coverage is 92.55%.

@@            Coverage Diff             @@
##             main    #1061      +/-   ##
==========================================
+ Coverage   72.92%   73.44%   +0.51%     
==========================================
  Files         357      364       +7     
  Lines       47120    47595     +475     
==========================================
+ Hits        34363    34955     +592     
+ Misses      12757    12640     -117     

Impacted Files	Coverage Δ
include/cantera/kinetics/MultiRate.h	82.60% <ø> (ø)
include/cantera/kinetics/Reaction.h	100.00% <ø> (ø)
src/kinetics/Reaction.cpp	89.20% <85.71%> (-1.24%)	⬇️
src/kinetics/ReactionRateFactory.cpp	93.33% <93.33%> (ø)
include/cantera/kinetics/ReactionRate.h	61.33% <100.00%> (-0.51%)	⬇️
include/cantera/kinetics/ReactionRateFactory.h	100.00% <100.00%> (ø)
src/kinetics/BulkKinetics.cpp	91.00% <100.00%> (-0.08%)	⬇️
src/kinetics/ReactionRate.cpp	94.25% <100.00%> (-1.75%)	⬇️
src/kinetics/RxnRates.cpp	92.66% <100.00%> (+0.06%)	⬆️
src/base/global.cpp	72.38% <0.00%> (-2.86%)	⬇️
... and 44 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 3dc940d...39d4577. Read the comment docs.

[speth]

We can avoid the need for the MultiRateFactory class by adding a member function to the ReactionRate class, as suggested in my original outline for the MultiRate structure in #982. Namely, for each type of ReactionRate, all that's needed is a function:

virtual shared_ptr<MultiRateBase> newMultiRate() const {
    return make_shared<MultiBulkRate<ArrheniusRate, ArrheniusData>>();
}

Then, the replacement else-if tree in addReaction would work by just calling rate->newMultiRate().

[ischoegl]

@speth ... thanks for the suggestion - this is indeed simpler.

[speth]

Thanks, @ischoegl, most of this looks good to me. I think the only substantial concern I have is with figuring out the rate constant units in the newRate(AnyMap, Kinetics) function.

[bryanwweber]

A few small style comments, thanks @ischoegl

[ischoegl]

@speth and @bryanwweber - thank you for your reviews ... I addressed / responded to all comments at this point.

Regarding the unit controversy, I believe that there are some inconsistencies that go beyond this PR (see new issue #1071) - the constructor in question is useful for the creation of Reaction objects from scratch, where unit handling is not clear.

Thanks @ischoegl, my comments have been address so I'm dismissing my review. @speth can approve since his comments were more about the implementation.

[ischoegl]

Thanks, @bryanwweber!

[ischoegl]

@speth ... I believe I resolved the issue with units by removing the Kinetics parameter. Regarding the question about where the creation of a stand-alone ReactionRate object could be useful, I believe it is critical for the creation of a Reaction object from scratch, e.g.

In [1]: import cantera as ct
   ...: 
   ...: reactants = {'O': 1., 'H2': 1}
   ...: products = {'H': 1., 'OH': 1.}
   ...: rxn = ct.ElementaryReaction(reactants, products)
   ...: rxn
   ...: 
Out[1]: <ElementaryReaction: H2 + O <=> H + OH>

In [2]: rxn.rate # reaction rate is not set up (yet)
Out[2]: <ArrheniusRate at 7f38eb99ef00>

In [3]: rxn.rate(300.)
Out[3]: nan

In [4]: rxn.rate = ct.ReactionRate.from_yaml("rate-constant: {A: 38.7, b: 2.7, Ea: 6260.0 cal/mol}")
   ...: rxn.rate
   ...: 
Out[4]: <ArrheniusRate at 7f38eb99ef60>

In [5]: rxn.rate(300.)
Out[5]: 5195.44737317942

[ischoegl]

@speth ... the last commit blocks an edge case where a user would specify a unit system that would produce rate_units that have a factor other than 1.0 (which is probably the main take-away of the discussion in #1071):

In [2]: rxn.rate = ct.ReactionRate.from_yaml("""
   ...: rate-constant: {A: 38.7, b: 2.7, Ea: 6260.0 cal/mol}
   ...: units: {length: cm, quantity: mol}
   ...: """)

[...]

CanteraError: 
*******************************************************************************
InputFileError thrown by ReactionRateFactory::newReactionRate:
Error on line 2 of input string:
Alternative units for 'length' or 'quantity` are not supported when creating
a standalone 'ReactionRate' object.
|  Line |
|     1 | 
|     2 | rate-constant: {A: 38.7 kmol/cm^3, b: 2.7, Ea: 6260.0 cal/mol}
>     3 > units: {length: cm, quantity: mol}
                 ^
*******************************************************************************

With this update, I believe that all comments are taken care of.

PS: there is an exception though:

ct.ReactionRate.from_yaml("rate-constant: {A: 4.0e+21 cm^6/mol^2/s, b: 0.0, Ea: 1207.72688}")

which is a bit harder to catch (see units-custom.yaml).

[ischoegl]

Not too hard after all. I believe this is done now ...

In [2]: rate = ct.ReactionRate.from_yaml("rate-constant: {A: 4.0e+21 cm^6/mol^2/s, b: 0.0, Ea: 1207.72688}")
---------------------------------------------------------------------------
CanteraError                              Traceback (most recent call last)
<ipython-input-2-d77e48358ab6> in <module>()
----> 1 rate = ct.ReactionRate.from_yaml("rate-constant: {A: 4.0e+21 cm^6/mol^2/s, b: 0.0, Ea: 1207.72688}")

/src/interfaces/cython/cantera/reaction.pyx in cantera._cantera.ReactionRate.from_yaml()
    109         cdef CxxAnyMap any_map
    110         any_map = AnyMapFromYamlString(stringify(text))
--> 111         cxx_rate = CxxNewReactionRate(any_map)
    112         return ReactionRate.wrap(cxx_rate)
    113 

CanteraError: 
*******************************************************************************
InputFileError thrown by Arrhenius::setParameters:
Error on line 1 of input string:
Specification of units is not supported for pre-exponential factor when
creating a standalone 'ReactionRate' object.
|  Line |
>     1 > rate-constant: {A: 4.0e+21 cm^6/mol^2/s, b: 0.0, Ea: 1207.72688}
                                   ^
*******************************************************************************

[speth]

Thanks for the updates to this, @ischoegl. After the extended discussion on #1071, I think most of the complicated issues here have already been worked out without introducing too much new complexity. My remaining comments are mostly places on things that should either simplify the current implementation slightly or make some further generalizations that I suspect we both have in mind easier.

[ischoegl]

@speth ... thank you for your re-review and the interesting comments. I ended up adopting all your suggested changes.

[...] one of the key benefits of the generic ReactionRate class is breaking this requirement that there is a specific rate parameterization that goes along with a given Reaction class.

I honestly was still thinking along the old lines too much here. I certainly realized that the revised Reaction3 specializations were pretty much hollowed out (i.e. not many differences), but following that thought it will become possible to at least partially ditch having specialized Reaction classes altogether. As an example, there's nothing wrong with an elementary reaction having a Chebyshev rate object. This opens up a lot of interesting possibilities and simplifications ...

[ischoegl]

Realized that shifting the emphasis away from Reaction (using consistent naming) makes specialized getParameters mostly unnecessary (except for ThreeBodyReaction3, at least for the moment). Which means that ElementaryReaction3, PlogReaction3 and ChebyshevReaction3 are essentially the same (except for minor differences in constructors). Not bad ... 🎉

I believe this is ready to merge now. Getting rid of some Reaction specializations (while possible) is likely a longer discussion for another PR.

PS: Changing the type string output created a minor paper cut, which is also fixed.

[speth]

Thanks, @ischoegl, this looks good to me. I had just one very minor nitpick, commented on below.

Regarding the fate of the Reaction class, I expect that the only distinction ultimately worth keeping will be between bulk phase reactions and interface/electrochemical reactions. But even then, some of the extra parameters and functionality may fit better within the ReactionRate classes.

[ischoegl]

@speth ... thanks for the reviews! I removed the redundant returns with the last commit.

Regarding the fate of the Reaction class, I expect that the only distinction ultimately worth keeping will be between bulk phase reactions and interface/electrochemical reactions. But even then, some of the extra parameters and functionality may fit better within the ReactionRate classes.

👍 ... I see it the same way - the YAML type field will essentially be pushed through to the ReactionRate object, which should be able to take care of (almost) everything.