Refactor reaction rate evaluators

[ischoegl]

Changes proposed in this pull request

Use newly introduced MultiBulkRates rate evaluator to handle replacement classes, and deprecate current approaches. This PR will replace everything except FalloffReaction and introduce the following new classes:

• ElementaryReaction3
• ThreeBodyReaction3
• PlogReaction3
• ChebyshevReaction3

These classes will be used by default by YAML importers, although the old classes remain accessible from YAML. CTI and XML file import will use the old classes. The 3 will be dropped from class names after 2.6, and old class names will have 2 added (Edit: likely the old classes will be removed).

Items that will be addressed in subsequent PRs are:

• Factories for ReactionRate and templated MultiRate objects.
• Various implementations of FalloffReactions
• Optimizations of ThirdBody handlers
• Reactions used by InterfaceKinetics
• Create infrastructure that allows for modifications of reaction rate parameters in memory for new reaction implementations

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

Addresses Cantera/enhancements#84 and Cantera/enhancements#87

Checklist

• There is a clear use-case for this code change
• The commit message has a short title & references relevant issues
• Build passes (scons build & scons test) and unit tests address code coverage
• The pull request is ready for review

[codecov]

Codecov Report

Merging #995 (952d113) into main (01bf417) will increase coverage by 0.33%.
The diff coverage is 93.78%.

@@            Coverage Diff             @@
##             main     #995      +/-   ##
==========================================
+ Coverage   72.59%   72.92%   +0.33%     
==========================================
  Files         356      357       +1     
  Lines       46508    47120     +612     
==========================================
+ Hits        33763    34363     +600     
- Misses      12745    12757      +12     

Impacted Files	Coverage Δ
include/cantera/kinetics/GasKinetics.h	100.00% <ø> (ø)
include/cantera/kinetics/ReactionRate.h	61.84% <62.50%> (+21.10%)	⬆️
src/kinetics/GasKinetics.cpp	93.03% <76.66%> (-4.32%)	⬇️
include/cantera/kinetics/MultiRate.h	82.60% <83.33%> (+37.15%)	⬆️
test/kinetics/kineticsFromYaml.cpp	98.37% <86.66%> (+0.12%)	⬆️
src/kinetics/BulkKinetics.cpp	91.07% <87.87%> (+2.43%)	⬆️
src/kinetics/Kinetics.cpp	83.93% <90.00%> (-0.19%)	⬇️
src/kinetics/RxnRates.cpp	92.59% <92.52%> (+2.76%)	⬆️
src/kinetics/ReactionRate.cpp	96.00% <95.71%> (-4.00%)	⬇️
src/kinetics/Reaction.cpp	90.44% <96.48%> (-0.25%)	⬇️
... and 15 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 e60f513...952d113. Read the comment docs.

[ischoegl]

@speth ... I think this is getting close to where it should be. Benchmarks (YAML vs XML) are:

Average time of 100 simulation runs for 'gri30.yaml' (CH4)
- New framework (YAML): 155.06 ms (T_final=2736.60)
- One Python reaction: 163.27 ms (T_final=2736.60) ... +5.30%
- Old framework (XML): 148.09 ms (T_final=2736.60) ... -4.50%

So performance comparisons are still in line with #982.

There are a couple of items I am aware of but would rather tackle in combination with Falloff in a follow-up PR:

• A couple of performance tweaks that I am anticipating:

  • MultiRate can be improved by using C++ std::vector<std::pair<size_t, RateType>> Edit: done
  • ThirdBodyCalc can directly store indices in composition vector

• Some of the old-style Arrhenius objects are still exposed in the new Python interface that is based on ArrheniusRate; I am, however, waiting for feedback on the interfaces before finalizing this work

• Factory classes for ReactionRate and templated MultiRate objects are obviously missing

• Serialization will depend on Enable serialization of Cantera objects #984, but should be straight-forward. The new structure has clear delineations between Reaction and ReactionRate objects.

• Edit: In Python, the new reaction objects expose rate objects themselves, rather than making rate objects methods accessible through methods and properties defined for a Reaction. Some of the old method should probably be temporarily added for the transition.

Finally, one item that came up in the UG (Change A,b,Ea in memory) ... those values are directly mutable for the new framework - as a proof of concept, I implemented test_reaction.py::TestArrhenius::test_allow_negative_pre_exponential_factor2. It would be straight-forward to add setters ... Edit: vector::push_back creates copies (in MultiRate::add), so there are still distinct copies of ReactionRate objects. It is, however, a solvable problem (e.g. std::move is supported, or the new copy could replace the original version). Edit: done

PS: over the next couple of days, I'll likely go over docstrings one more time, but the implementation itself won't change at this point. Edit: done

[ischoegl]

One wart of the implementation thus far was that MultiRate creates a copy of a ReactionRate object that was not mutable in memory (same situation as previously with Rate1). With this update, it becomes possible to modify ArrheniusRate parameters in memory, e.g. from Python

gas.reaction(2).rate.pre_exponential_factor *= 2

which modifies both the rate attached to the Reaction object, as well as the rate handled by the MultiRate evaluator.
(Edit: leaving this for later to make the review easier)

[ischoegl]

@speth ... I believe this is now (finally) converged after deciding to incorporate the ability to modify reaction rate parameters in memory. At 3.5k lines, I believe there is a stopping point ...

PS: while CI passes, there is some glitch for the test-kinetics unit test if run by itself that I haven't been able to track down (not familiar enough with googletests: it doesn't shut down properly)

[ischoegl]

Rebased to include #984 and converted back to draft. Waiting to have #968, #987 and #1014 merged.

[ischoegl]

Had to rebase after a merge conflict due to #1049. The newly failing test is unrelated (see #1055) ... everything else is taken care of (I think). Unfortunately the rebase means that getting to some of the last discussion requires a couple of clicks as it is currently hidden.

[speth]

I found just a few minor issues with the documentation. Otherwise, I think this should be finished. I'm 100% in agreement that we should address some of these secondary questions in a separate PR.

Also, I triggered a re-run of the CI to see if the failure while downloading packages was just a transient issue.

[ischoegl]

@speth - thanks for catching those. Fixed the docstrings and squashed the last couple of commits as they just reshuffled approaches for a single feature.

[ischoegl]

Thanks, @speth!