-
Notifications
You must be signed in to change notification settings - Fork 145
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add training reactions and enlarge the nodes in the tree for some surface reaction families #479
Conversation
526e7bd
to
3d96060
Compare
…eider_Pt111 libraries
…d Ishikawa_Rh111, Popa_Rh111
Surface_Abstraction_vdW family
2d35976
to
5472f86
Compare
Some A factors in Lu_Ir111 library used the default value A =10e13. Maybe we could figure out another way to calculate A with imaginary frequency...? |
…nd revised the index of Novell_Pt111, Offermans_Pt111
…1, Kraehnert_Pt111, Offermans_Pt111, Rebrov_Pt111, Roldan_Ir111, Roldan_Ru0001 and Vlachos_Pt111
…ldan_Ir111, Roldan_Ru0001 libraries into the training reactions.
Great work @Tingchenlee ! Thanks for these additions! Have you tested this branch by building a model with all of the families that you add training reactions? Some of this is not covered in unit testing, and bugs may only show up when we are adding rules from training to build models |
I think I tested most of them individually after I finished one but I didn’t test all of them at one time. I am testing all of the libraries and families in one model now! |
I had run a couple of models which included CHx, NHx, and O2 species at different temperatures (300K-1500K) and pressure (1 bar - 200 bar). All the models were okay. However, there was a secret space in the Schneider_Rh211 library (previously used " |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for these additions @Tingchenlee! Since the tests pass and we are able to build models with this branch, I approve of this PR. However, there are a few caveats and things we should address either as part of this PR or in the future
- some of the kinetics trees are too small to leverage the kinetics data added here. For example, a lot of training reactions are added to the
Surface_Adsorption_Single
family, but the tree is very small:
tree(
"""
L1: Adsorbate
L2: N
L3: N=O
L1: VacantSite
"""
We need more nodes in the tree (for C
and O
for example) for the new reactions to descend to, otherwise we will have a log jam at the top of the tree and only the first reaction with the best rank will be used. So currently some of our trees are too small to properly handle the influx of training data with this PR.
-
averaging
SurfaceArrhenius
andStickingCoefficient
kinetics. I'm not sure if this is an issue, but I suspect it may be. For some of the families (Surface_Adsorption_Single
for example), we haveStickingCoefficient
kinetics in the forward direction andSurfaceArrhenius
in the reverse direction. I don't think we properly average the kinetics when you are averaging bothStickingCoefficient
kinetics andSurfaceArrhenius
kinetics. This is something we should investigate. -
minor detail - I think we should use
X
for the species label for empty sites in training reactions and notPt
orCu
because it will get reused, and if you have a reaction on a different metal it can be confusing (reaction could be on 'Ni' but species label may be 'Pt')
…ics families. (Surface_Adsorption_Dissociative, Surface_Adsorption_Single, Surface_Dissociation and Surface_Dissociation_Beta)
…Dissociation_Beta families
For point 1 on david's comment, @Tingchenlee discovered that you get an error loading the database with the mixed surfaceArrhenius and stickingCoefficient types in the training reactions. Specifically, the error occurs when trying to average these two in the surface adsorption vdw family:
StickingCoefficientBEP(
A=1.17e-06,
n=0,
alpha=0,
E0=(0,'J/mol'),
Tmin=(200,'K'),
Tmax=(3000,'K'),
comment="""Average of [From training reaction 28 used for N2H4;VacantSite]"""),
[<Entry index=33 label="N-N">, <Entry index=2 label="VacantSite">]
SurfaceArrheniusBEP(
A=(3.464e+21,'cm^3/(mol*s)'),
n=0,
alpha=0,
E0=(4000,'J/mol'),
Tmin=(200,'K'), Tmax=(3000,'K'),
comment="""From training reaction 13 used for N#N;VacantSite"""),
[<Entry index=37 label="N#N">, <Entry index=2 label="VacantSite">] This is not caught by unit tests currently. the error that you get is because the units do not match, but the real problem is, as david said, we probably shouldn't be averaging these two reaction types. I think to determine a workaround, we need to determine how (or if) we should average these two reaction types. The same rates are reported differently by different papers so I think we should be able to average them, but I think that would mean calculating the A, b, and Ea for the sticking coefficient reaction: I can try to program a work around for this, the actual averaging routine does not look very complicated, but I'd like to know what everyone else thinks of this solution. in the context of this PR we could probably also just convert the offending reactions to surface Arrhenius form. |
I think for adsorption type reactions, the StickingCoeff kinetics model is more "physically meaningful" than SurfaceArrh. Therefore we should convert the SurfaceArrh kinetics for adsorption training reactions to StickingCoeff, then average the StickingCoeffs. What we are missing, however, is a method to convert SurfaceArrh to StickingCoeff. We would need to create this method, then use it to convert SurfaceArrh training reactions |
Thanks, @davidfarinajr and @ChrisBNEU |
looks like the |
Opened a PR to address the mixed kinetics types issue in training reactions and was able to build model with this PR DB branch ReactionMechanismGenerator/RMG-Py#2148. |
…iption (the unit conversion of A factors and the citation) 2. rearranged the number of the entries in all families 3. fixed the typos of C_X from double bonds to quadruple bonds, CH_X from single bonds to triple bonds. Then re-imported the libraries into the training reactions and removed the wrong old ones.
2. corrected adjacency list of CO species in the Mhadeshwar_Pt111, Vlachos_Pt111 and Vlachos_Rh libraries
I have added another library After adding lots of training data, I decided to re-number the entries and also took a look at our previous data. I found some of the reactions used the wrong units in the long description, for example, the calculation of
I think the unit of A factor in that paper is 1/s, and the equation becomes:
I revised these long descriptions and made some comments but I'm not sure if I'm wrong. Also corrected some of the short and long descriptions I made and fixed some typos in the some other minor revise:
|
Oops wrong pr |
index = 2, | ||
label = "O_X <=> O + X", | ||
kinetics = SurfaceArrhenius( | ||
A = (4E21, '1/s'), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
1E13
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for pointing out this issue, I was having fun with the crashed models these two days...
Just fixed it now!
Removed the comments of correcting the long description
|
||
entry( | ||
index = 10, | ||
label = "HOX <=> X + HO", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If they published this in both directions, we should use the forward Adsorption direction sticking coefficient.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Okay, I have removed all the SurfaceArrhenius reactions in the Surface_Adsorption_Dissociative
, the Surface_Adsorption_Single
, and the Surface_Adsorption_vdW
families and imported the StickingCoefficient reactions.
If the paper only calculated the reaction rates in the reverse direction (SurfaceArrhenius), I keep them in the libraries only.
We used the reactions that have the form of StickingCoefficient
…reactions in Surface_Adsorption_vdW, Surface_Adsorption_Single, and Surface_Adsorption_Dissociative training
…ilies 1.added NH3 and O2 surface adsorption reactions with sticking coefficients, and revised the value of two Ea (R5 and R10) 2. added R6, R11, and R17 from table 1
Opened RMG-database PR#512 and closed this one |
Purpose:
Adding more surface training data into kinetic families.
Method:
Notice:
Paper: Kinetic Prefactors of Reactions on Solid Surfaces