Skip to content
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

Halogen training reactions #515

Open
wants to merge 55 commits into
base: main
Choose a base branch
from
Open

Conversation

davidfarinajr
Copy link
Contributor

@davidfarinajr davidfarinajr commented Jul 28, 2021

added training reactions from halogen kinetic models include NIST HFC and 2-BTP/CF3Br mech and Westmoreland YF mech. Also corrected CH3 in disproportionation training

twin with ReactionMechanismGenerator/RMG-Py#2135

@davidfarinajr davidfarinajr force-pushed the halogen_training_reactions branch 2 times, most recently from a10dea8 to 604712c Compare August 2, 2021 19:55
@davidfarinajr
Copy link
Contributor Author

This is the only test that is failing

1744
FAIL: test that the right number of reactions are in output network
1745
----------------------------------------------------------------------
1746
Traceback (most recent call last):
1747
  File "/home/runner/work/RMG-database/RMG-Py/arkane/explorerTest.py", line 81, in test_reactions
1748
    self.assertEqual(len(self.explorer_job.networks[0].path_reactions), 7)
1749
AssertionError: 6 != 7

This test seems to bounce back and forth between 6 and 7. When we added CO to primaryThermoLib, we changed it from 6 to 7, now it want to be 6 again. Can we change this test to
self.assertIn(len(self.explorer_job.networks[0].path_reactions), [6,7])

@davidfarinajr davidfarinajr requested a review from rwest August 2, 2021 21:38
@davidfarinajr davidfarinajr force-pushed the halogen_training_reactions branch from dd66f46 to e0d02b7 Compare August 9, 2021 22:38
@sevyharris sevyharris self-requested a review September 1, 2021 22:17
@davidfarinajr davidfarinajr force-pushed the halogen_training_reactions branch from 053cd4e to 6fa5908 Compare September 15, 2021 18:43
@davidfarinajr davidfarinajr force-pushed the halogen_training_reactions branch from 6fa5908 to 8f7ba91 Compare September 23, 2021 15:28
@davidfarinajr davidfarinajr changed the base branch from master to main October 1, 2021 17:30
@davidfarinajr davidfarinajr force-pushed the halogen_training_reactions branch 2 times, most recently from b27d3b6 to 1cada62 Compare October 14, 2021 18:25
@davidfarinajr
Copy link
Contributor Author

Cross validation for R_Addition_MultipleBond tree

R_add_tree_cross_val.pdf

@davidfarinajr davidfarinajr force-pushed the halogen_training_reactions branch from caec39b to cb4a63c Compare October 15, 2021 16:10
@davidfarinajr
Copy link
Contributor Author

looks like the tests dies here
Kinetics family H_Abstraction: groups are not identical? ... make: *** [Makefile:82: test-database] Killed
maybe ~7500 groups is too many to run this test 😂, and if its autogenerated tree, do we need to run this test?

@davidfarinajr davidfarinajr force-pushed the halogen_training_reactions branch 3 times, most recently from f4aeaee to 80a0f4f Compare October 23, 2021 15:15
@davidfarinajr davidfarinajr force-pushed the halogen_training_reactions branch from 80a0f4f to 5838493 Compare October 30, 2021 01:12
@davidfarinajr davidfarinajr requested a review from mazeau November 2, 2021 17:01
@davidfarinajr davidfarinajr force-pushed the halogen_training_reactions branch from 5838493 to 908eecb Compare November 2, 2021 19:55
Copy link
Contributor

@sevyharris sevyharris left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good to me. I looked over the files and ran a bunch of examples including 2-BTP, ethane pyrolysis (minimal), cpox (surface), Nitrogen, and Sulfur, and none of them crashed. I picked the Nitrogen regression test as the one to look at for a more thorough review. I was slightly concerned that the halogens version finds fewer species and reactions than the main branch because my lizard brain is convinced that more=better. But most of the "missing" reactions are from Disproportionation, which was regenerated for all those new training reactions, so that makes sense that it would be different. I also ran a few simulations in Cantera for the Nitrogen example to see how the results changed, and you get nearly identical results at high temperatures. The halogens model is marginally slower than the main branch's version, especially at lower temperatures. I don't know much about the chemistry, but it seems reasonable to me.

Well done David! This PR must have taken a lot of work. I'm excited for it to be merged in.

@davidfarinajr
Copy link
Contributor Author

Thanks for the awesome review @sevyharris, and thanks for getting the rmg regressions tests working! I will also take a look at the regression tests to see if there is anything that catches my eye.

@davidfarinajr
Copy link
Contributor Author

I'm thinking about dropping this commit 4a17fe7 which makes intra_H_migration intro intra_H/Val7_migration, and instead making a new intra_halogen_migration family. I did not have any halogen migration training reactions at the time, but I am calculating some now. F migration is a lot slower than H (much higher barrier) and we are overestimated rates for these in our models.

@rwest
Copy link
Member

rwest commented Nov 7, 2021

Sounds like a reasonable idea. Would leave H migration unperturbed; speed up generation of models with no halogens; and avoid the very different rates from influencing each other.

@davidfarinajr davidfarinajr force-pushed the halogen_training_reactions branch from 908eecb to fed41cb Compare November 7, 2021 21:31
@davidfarinajr
Copy link
Contributor Author

The new intra_halogen_migration family is here #546. I dropped the commit changing the groups for the intra_H_migration family for this PR

@davidfarinajr davidfarinajr force-pushed the halogen_training_reactions branch from fed41cb to 814180d Compare November 15, 2021 22:27
@davidfarinajr
Copy link
Contributor Author

Cross validation for H Abstraction tree

tree_cross_val.pdf
tree_cross_val.csv

@davidfarinajr
Copy link
Contributor Author

Disprop tree

tree_cross_val.pdf
tree_cross_validation.csv

This is outlier

entry(
    index = 69,
    label = "C2H5S-2 + C3H7-2 <=> C2H6S + C3H6-2",
    degeneracy = 2.0,
    kinetics = Arrhenius(A=(6.74e-06,'cm^3/(mol*s)'), n=4.35, Ea=(4.76976,'kJ/mol'), T0=(1,'K'), Tmin=(300,'K'), Tmax=(1500,'K')),
    rank = 6,
    shortDesc = """CAC calc CBS-QB3 1dhr""",
    longDesc = 
"""
Converted to training reaction from rate rule: C_rad/H/CsS;C/H2/Nd_Csrad
""",
)

ln(kest/ktrain)=10.2

davidfarinajr and others added 22 commits December 14, 2021 17:49
added reverseMap, reactant and product Num for H Abs
ran a script to relabel spcs in H Abs training
…ction

H2-2 + O2 <=> HO2-3 + H-2 violates the collision limit:
     reverse    ratio=120558544715676.7     condition=500.0 K, 1.0 bar
     reverse    ratio=55.46194345811393     condition=1500.0 K, 1.0 bar
Imported reactions from literature reactions are changed to rank 10 (since we don't always know the source) and AutoTST calculated rates are changed to rank 7 since most are DFT level without rotor scans
@davidfarinajr davidfarinajr force-pushed the halogen_training_reactions branch from 0142778 to e635e1d Compare December 14, 2021 22:50
changing to C2s prevents matching of C2sc groups which raise atomtype error when template is applied:
Error: Could not update atomtypes for this molecule:
multiplicity -187
1    F u0 p3 c0 {4,S}
2    O u0 p1 c+1 {3,D} {4,S}
3    O u0 p2 c0 {2,D}
4 *3 C u0 p0 c-1 {1,S} {2,S} {5,S} {6,S} {8,S}
5    H u0 p0 c0 {4,S}
6 *2 O u0 p2 c0 {4,S} {8,S}
7    O u0 p2 c0 {8,D}
8 *1 C u0 p0 c0 {4,S} {6,S} {7,D}

Error: Problem family: 1+2_Cycloaddition
Error: Problem reactants: (Molecule(smiles="O=C=O"), Molecule(smiles="O=[O+][C-]F"))
Error: <Molecule "O=C=O">
1 *2 O u0 p2 c0 {3,D}
2    O u0 p2 c0 {3,D}
3 *1 C u0 p0 c0 {1,D} {2,D}

Error: <Molecule "O=[O+][C-]F">
1    F u0 p3 c0 {4,S}
2    O u0 p1 c+1 {3,D} {4,S}
3    O u0 p2 c0 {2,D}
4 *3 C u0 p1 c-1 {1,S} {2,S} {5,S}
5    H u0 p0 c0 {4,S}
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants