RMG-Py icon indicating copy to clipboard operation
RMG-Py copied to clipboard

SurfaceArrhenius <=> StickingCoefficient

Open davidfarinajr opened this issue 3 years ago • 4 comments

Motivation or Problem

Some surface reaction families have training data with mixed kinetics types (SurfaceArrhenius and StickingCoeff). We need methods to convert SurfaceArrhenius to StickingCoeff and vice versa so that we can convert the training data to the same kinetics type so that we can average them.

Description of Changes

  • created methods to convert StickingCoeff to SurfaceArrhneius and SurfaceArrhenius to StickingCoeff
  • added a check to make sure that the kinetics types are all the same when averaging kinetics

Testing

  • Built a model with the DB branch for this PR https://github.com/ReactionMechanismGenerator/RMG-database/pull/479, where we have mixed kinetics types in training
  • tested the Surf Arrh <--> StickingCoeff method in Jupyter notebook, but have not added unit tests yet

davidfarinajr avatar May 31 '21 16:05 davidfarinajr

I also ran a test(which included CH4, NH3, and O2species) with all new libraries and surface families, and the model was completed.

Tingchenlee avatar May 31 '21 17:05 Tingchenlee

Codecov Report

Merging #2148 (46141b1) into master (336273d) will increase coverage by 0.05%. The diff coverage is 60.00%.

:exclamation: Current head 46141b1 differs from pull request most recent head bc78dbb. Consider uploading reports for the commit bc78dbb to get more accurate results Impacted file tree graph

@@            Coverage Diff             @@
##           master    #2148      +/-   ##
==========================================
+ Coverage   47.84%   47.89%   +0.05%     
==========================================
  Files         102      102              
  Lines       27120    27069      -51     
  Branches     6958     6946      -12     
==========================================
- Hits        12976    12966      -10     
+ Misses      12753    12716      -37     
+ Partials     1391     1387       -4     
Impacted Files Coverage Δ
rmgpy/data/kinetics/rules.py 49.87% <60.00%> (-0.13%) :arrow_down:
rmgpy/data/kinetics/library.py 41.52% <0.00%> (-0.83%) :arrow_down:
rmgpy/data/solvation.py 56.92% <0.00%> (-0.46%) :arrow_down:
rmgpy/rmg/main.py 22.51% <0.00%> (-0.11%) :arrow_down:
rmgpy/data/kinetics/family.py 47.38% <0.00%> (-0.01%) :arrow_down:
arkane/kinetics.py 12.24% <0.00%> (ø)
rmgpy/molecule/filtration.py 88.33% <0.00%> (+0.13%) :arrow_up:
rmgpy/rmg/input.py 41.26% <0.00%> (+0.25%) :arrow_up:
arkane/encorr/bac.py 75.77% <0.00%> (+0.47%) :arrow_up:
... and 5 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 336273d...bc78dbb. Read the comment docs.

codecov[bot] avatar May 31 '21 17:05 codecov[bot]

These are useful methods to have, to convert back and forth, and we should keep them, but I feel like training reactions should all be in the same format in the database, and the person adding them to the database should convert at that point, rather than when loading the database at run time. This will greatly simplify the averaging, and the interpretation of averaging, and Matt's auto tree generation, etc.

So I suggest a unit test added to the database checks to ensure all training reactions in adsorption reaction families are in StickingCoefficient form (and perhaps prints the helpful conversion in the error report if it's in SurfaceArrhenius, to facilitate changing it). It could also warn you if the converted value exceeds 1.0 at some temperatures.

rwest avatar Jun 04 '21 17:06 rwest

Worth checking we do this: things entered in the reverse direction (i.e. desorption) will be a SurfaceArrhenius with units of 1/s. These will need converting into a StickingCoefficient, not just a reversed SurfaceArrhenius.

rwest avatar Jun 04 '21 17:06 rwest

This pull request is being automatically marked as stale because it has not received any interaction in the last 90 days. Please leave a comment if this is still a relevant pull request, otherwise it will automatically be closed in 30 days.

github-actions[bot] avatar Jun 21 '23 22:06 github-actions[bot]