add Tsang closed-form falloff

[mefuller]

Changes proposed in this pull request
Add Wing Tsang's falloff rate expression (explicit value of F_cent provided with one or two parameters for calculation of F via Troe falloff)

• Add Tsang falloff where F_cent = A + B*T
• Falloff parameter F is calculated from F_cent according to Troe
• Allows for insertion of rates in the format specified by Tsang and coworkers in various databases, e.g. doi: 10.1063/1.555890

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

Closes #

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

[ischoegl]

@mefuller ... thanks for submitting! While I am in the midst of refactoring some of Cantera's reaction rate calculation routines (see Cantera/enhancements#87 - adding new classes should become much easier at that point), it should be possible to merge this before I start rewriting all of the Falloff classes. I'll leave this decision to @speth (at the moment, #1088 and #1089 are higher on my own priority list).

[codecov]

Codecov Report

Merging #1090 (0009a77) into main (703fd1d) will increase coverage by 0.01%.
The diff coverage is 82.75%.

❗ Current head 0009a77 differs from pull request most recent head 8ca6aca. Consider uploading reports for the commit 8ca6aca to get more accurate results

@@            Coverage Diff             @@
##             main    #1090      +/-   ##
==========================================
+ Coverage   73.45%   73.46%   +0.01%     
==========================================
  Files         364      364              
  Lines       47611    47667      +56     
==========================================
+ Hits        34972    35018      +46     
- Misses      12639    12649      +10     

Impacted Files	Coverage Δ
include/cantera/kinetics/Falloff.h	67.64% <55.55%> (-6.43%)	⬇️
src/kinetics/Reaction.cpp	88.90% <60.00%> (-0.31%)	⬇️
src/kinetics/Falloff.cpp	86.79% <92.00%> (+1.60%)	⬆️
src/kinetics/FalloffFactory.cpp	100.00% <100.00%> (ø)
test/kinetics/kineticsFromYaml.cpp	98.42% <100.00%> (+0.08%)	⬆️

---

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 703fd1d...8ca6aca. Read the comment docs.

[bryanwweber]

Hi @mefuller thanks for submitting this and the related Enhancements issue! A couple of notes from me too as you're working on it, which will hopefully make your life easier:

1. You should not add this capability to the CTI/CTML formats, including the ck2cti, ctml_writer, and related scripts. We're going to be removing CTI/CTML in an upcoming version, so we're not adding any new features to those input formats.
2. On a related note, is this format specified in the CHEMKIN manual, and is there a defined format accepted by CHEMKIN-PRO? If not, then you do not need to add it to the ck2yaml converter either, it can be processed straight out of the YAML files.
3. Please make sure to add some tests that check that the functionality is implemented correctly. Tests from Python would go into interfaces/cython/tests and otherwise, they would go in tests/kinetics I think.

Thanks!

[ischoegl]

@bryanwweber … thank you for the comments. From my perspective, getting unit tests and import right is the most important aspect. If those are in place, porting to the new framework will be straightforward.

@mefuller … thanks again for the contribution. As mentioned, I am planning to port Falloff reactions soon, but as this is already implemented, it may be easiest if this is adopted before the transition. Looking forward to your feedback!

[mefuller]

@ischoegl @bryanwweber if either of you has a chance, would you mind taking a look at test Reaction.FalloffFromYaml3 in test/kinetics/kineitcsFromYaml.cpp? I have tried commenting out different lines in the test block and simply cannot get it to pass, so I think I am missing something fundamental.
I am able to load a YAML mechanism containing reactions in the Tsang format, so there is nothing obvious to me.
I'll also admit to not knowing anything about programming in C/C++

Once I get the test to pass, I'll clean up the unnecessary changes and squash commits before finalizing the PR.
Thanks

[ischoegl]

Hi @mefuller ... the one remaining error appears to be due to Cantera's internal units checkers. Internally, everything is handled in kg-m-kmol-s, but you use cm^3/mol/s for some of the reaction rate parameters, where conversions are not as straight-forward (same for gri30.yaml, which implicitly sets the default behavior). If you look at the Troe parameterization just above, you'll see that units are different for high and low rates.

For your new reaction, you need to make sure that the output dimensions check out (they are a function of reaction order) - rate of progress should have the correct dimensions in terms of mol/cm^3/s (which is converted to kmol/m^3/s), where rate constant dimensions have to be adjusted to ensure everything matches once species concentrations are multiplied.

[mefuller]

I undid changes for converting from cti and ctml that @bryanwweber indicated were not required and removed the ck2yaml converter changes as the Tsang format has never been officially supported in CHEMKIN, just hacked in by some researchers.
I have not squashed commits yet just in case something comes up in review.
Thanks for all your help @ischoegl with identifying the test error.

[ischoegl]

This looks pretty good to me - added code builds on existing templates for other Falloff reactions. All of the comments I left should be easy to address.

Beyond, files ck2cti, ck2yaml and cti2yaml don't have any changes and thus should not show up in the diff. Finally, feel free to add yourself to the AUTHORS file.

[mefuller]

Made changes and squashed commits;
Converter files are still showing as changed but without changes - how can I eliminate these or do we just ignore them?

[bryanwweber]

@mefuller The change in the converter files is that they've been made executable by your changes. You can fix it by (for each of the files):

git checkout main -- interfaces/cython/cantera/ck2cti.py

and commit the resulting changes with git commit --amend --no-edit and then a force-push. Just make sure your main branch is up-to-date.

[mefuller]

@ischoegl @bryanwweber hopefully everything is now done
Thank you both very much for helping me with this PR

[ischoegl]

@mefuller ... thank you! - it indeed looks mostly good to me, although there are some minor quibbles. Cantera recommends limiting line length to 88 characters, which some of the docstrings exceed. (yaml input file, but also in Falloff.h for the docstring of class Tsang)

[ischoegl]

@mefuller .. thank you for taking care of the requests. Unless @bryanwweber has any concerns, I will merge later today.

[bryanwweber]

@ischoegl @mefuller LGTM! 👍