Add Blowers Masel reaction to GasKinetics and InterfaceKinetics

[12Chao]

Changes proposed in this pull request

• Create Blowers Masel reaction class in both GasKinetics and InterfaceKinetics
• The reaction rate will be updated with the enthalpy of reaction changing according to Blowers Masel approximation
• The description of this change is on enhancement repo issue 66

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

Fixes 66

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 #987 (0f4bc3b) into main (81cffde) will increase coverage by 0.12%.
The diff coverage is 90.35%.

@@            Coverage Diff             @@
##             main     #987      +/-   ##
==========================================
+ Coverage   72.39%   72.52%   +0.12%     
==========================================
  Files         364      364              
  Lines       46433    46744     +311     
==========================================
+ Hits        33616    33900     +284     
- Misses      12817    12844      +27     

Impacted Files	Coverage Δ
include/cantera/kinetics/GasKinetics.h	100.00% <ø> (ø)
include/cantera/kinetics/InterfaceKinetics.h	100.00% <ø> (+33.33%)	⬆️
include/cantera/kinetics/Kinetics.h	51.51% <ø> (ø)
src/kinetics/RxnRates.cpp	89.83% <77.77%> (-3.58%)	⬇️
src/kinetics/Reaction.cpp	89.18% <79.59%> (-1.50%)	⬇️
include/cantera/kinetics/RxnRates.h	91.97% <91.66%> (-0.17%)	⬇️
src/kinetics/InterfaceKinetics.cpp	85.71% <96.96%> (+1.90%)	⬆️
include/cantera/kinetics/RateCoeffMgr.h	100.00% <100.00%> (ø)
include/cantera/kinetics/Reaction.h	100.00% <100.00%> (ø)
src/kinetics/GasKinetics.cpp	97.42% <100.00%> (+0.14%)	⬆️
... and 6 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 81cffde...0f4bc3b. Read the comment docs.

[rwest]

Thanks. As noted in Cantera/enhancements#66...

In your pull request to cantera, as well as the docstrings don't forget the documentation files:

• https://github.com/Cantera/cantera/blob/main/doc/sphinx/yaml/reactions.rst
• https://github.com/Cantera/cantera/blob/main/doc/sphinx/cython/kinetics.rst

And make a new pull request on https://github.com/Cantera/cantera-website (with appropriate notes like "new in 2.6")

• Update https://github.com/Cantera/cantera-website/blob/main/pages/tutorials/yaml/reactions.rst
• Update https://github.com/Cantera/cantera-website/blob/main/pages/tutorials/yaml/yaml-format.rst
• Update https://github.com/Cantera/cantera-website/blob/main/pages/science/reactions.rst

[speth]

Thanks for submitting this PR, @12Chao. Can you please rebase this on to the current main branch? The recent PR #982 made some changes to reaction handling that have generated some conflicts that need to be resolved.

Overall, I think this looks like it's in pretty good shape, and I appreciate the comprehensive set of tests included.

As far as the interaction with the changes in #982 go, I think there are some things that will need to be changed in order for this to work correctly (i.e. the code for wrapping Reaction objects in Python. There is an opportunity to simplify some of the implementation of these reaction types and how they interact with the GasKinetics class using the new structure introduced in #982, but it might be simpler to merge this using the old style and pursue that update as a separate PR.

In addition to the specific comments below, please go through your commits and eliminate the changes that remove the end-of-file newline in several files, and changes that introduce trailing whitespace. If you haven't already, I would highly recommend configuring your editor to highlight whitespace at the end of lines so you can avoid introducing such changes.

[ischoegl]

As far as the interaction with the changes in #982 go, I think there are some things that will need to be changed in order for this to work correctly (i.e. the code for wrapping Reaction objects in Python. There is an opportunity to simplify some of the implementation of these reaction types and how they interact with the GasKinetics class using the new structure introduced in #982, but it might be simpler to merge this using the old style and pursue that update as a separate PR.

As the one to 'blame' for #982, I tend to agree that it would make sense to finish this PR using the old style and update in a separate PR. I am currently working through the 'basic' reactions in #995, which - once done - should provide good templates for how things can be ported. As long as everything is locked in properly with unit tests, an eventual conversion should be relatively straight-forward.

[12Chao]

Thanks for the review @speth, @bryanwweber and @ischoegl! I believe I have addressed most of the comments. The only one left is how to make the buildBMsurfaceArrhenius function shorter as most of the code is duplicated from buildsurfaceArrhenius, I was trying to make a function to construct both surface Arrhenius and surface Blowers Masel parameters, but due to the input reaction type would change for building different type of parameters, I couldn't figure out a way about how to allow a function to take flexible data types. I tried to solve it by using function template in C++ but could not get it work properly. I would really appreciate it if anyone could give me some suggestions on that.

[speth]

Hi @12Chao, thanks for making these updates to your PR. Besides the specific comments below, I had a couple of other requests:

• Please add yourself to the AUTHORS file
• Regarding the code duplication you noted in InterfaceKinetics::buildBMSurfaceArrhenius, can you
  just add a comment starting with //! @todo there, and briefly mention that this could potentially be refactored to reduce code duplication, with the challenge being that the two methods end up returning slightly different object types. I think as we adopt the new Reaction framework from Make reactions extendable #982 in the InterfaceKinetics class, the opportunity to refactor this will become apparent, so I'm not in a rush to do it now.
• Can you write a Python example that uses the Blowers-Masel rate type that might help users understand how this is useful? Maybe an example drawn from the original B-M paper could be an option.

[12Chao]

Thanks for the review @speth! I have addressed all the comments and working on the Python example about Blowers-Masel approximation.

[rwest]

Hi @speth. We're discussing several usage examples, but all our meaningful ideas involve a decent amount of work, and will probably lead to a paper. Can I suggest we don't hold up this pull request until they're ready, else we'll forever be rebasing it and fixing merge conflicts? We'll definitely add examples to the examples repository as part of that work, which is the next thing that we'll work on, so I don't think it'll get forgotten.

[bryanwweber]

The CI on Windows and macOS are failing due to some upstream failures, not your fault.

Regarding 55e10c6, if you make it a raw string, it will work out.

[12Chao]

@bryanwweber Thanks for the help!

[bryanwweber]

@12Chao In Python, a raw string is prefixed with r before the opening quote, so you can include slashes:

plt.title(r"Ea versus $\Delta H$ For O+H2<=>H+OH", y=1.1)

You can also escape the backslash (which is usually used to give control characters like \n), then you don't need the r, but if there are a lot of slashes it can be hard to read:

plt.title("Ea versus $\\Delta H$ For O+H2<=>H+OH", y=1.1)

[speth]

Thanks for all the work on this, @12Chao. I'm looking forward to seeing the more interesting use cases for this model. Will you be able to open the companion PR for on the website repo to update the "Science" docs soon? I think what you wrote in the class docstrings here is already a good start for that.

@Cantera/committers - Since the last commit here is mine (modifying the example), I'd like someone else's approval before merging.

[rwest]

Your last two commits look good to me.

[speth]

Further updates: I fixed the conflicts that were introduced by merging #984, and added the methods necessary to enable serialization of B-M rates. Also thought this was a reasonable time to advance the alpha version number.

[decaluwe]

I will take a look, but IIRC--we decided that the website needs at least a PR before merging codebase changes--is that correct?

[12Chao]

Thanks for all the feedbacks, I will add a PR on the website ASAP

[decaluwe]

Hi @12Chao -- very nice work! I have a few very minor comments, below, but nothing that shouldn't prevent this from being merged very soon. I appreciate all the thought and detail that went into this.

[decaluwe]

Is there a way to use the theory to figure out what their ratio should be so that we could use an assertNear test, here? I'll admit I'm not familiar with how assertNotAlmostEqual works, but my naive sense is that it is not a equality test of any sort. Apologies if I'm wrong, there...

[12Chao]

According to the python doc , assertNotAlmostEqual compares the difference between 2 numbers which are rounded to 7 decimal places by default to zero, the assertion is true if the difference is not approximately equal to zero. I am not sure about how to modify the code here because I basically just copied test_modify_interface.

[speth]

I think @decaluwe's point is that it's possible to know how different k1 and k2 should be, so you can do a much more meaningful test than just checking that the numbers aren't the same. In this particular case, they should differ by the ratio exp( delta_theta * E_k / RT ).

[12Chao]

Thanks for the review @decaluwe, I have resolved most of the comments, except the one about test_modify_BMinterface. I'm wondering could you please give me a little more clue about how to modify the code there, like what kind of theory should I use to figure out the ratio of the 2 rate constants?

[speth]

@12Chao, I squashed your various cleanup commits back into the commits they were fixing up. Please reset to this version of the branch before making a change to resolve @decaluwe's last remaining comment.

[12Chao]

Thanks for the help @speth ! I have resolved the comment.

[speth]

Thanks, @12Chao! This looks good to me.

[decaluwe]

Very nice work, @12Chao !