Formate families#729
Conversation
drop this commit before merging to main. merge at same time as ReactionMechanismGenerator/RMG-database#729
|
Well one problem is that the failing database test has a super cryptic message that doesn't even seem to say which family is broken, nor what the problem is. |
|
Yeah, this is weird. I have to investigate more. The unit tests pass locally when I'm on the vdW-bonds branch |
drop this commit before merging to main. merge at same time as ReactionMechanismGenerator/RMG-database#729
|
Ugh. I was just delving in to debugging, based on the error from the CI, while the tests ran locally. Then the tests passed locally. turns out the CI wasn't using the correct RMG branch.. Notice although we do successfully have So the error messages and things can probably be tweaked to improve diagnostics, but it might be that the database is well formed, when used with the correct RMG-Py branch. (i think the changes to forbidden molecule checking are crucial) |
|
Found it (I think). Hopefully these tests now pass |
This comment was marked as outdated.
This comment was marked as outdated.
|
@bjkreitz would it be possible to add a fifth family to have a dissociation of a vdW species to form a vdWbidentate? I am thinking formic acid dissociation of O-H bond to form formate vdWbidentate. |
| If a monodentate adsorbate has an internal double or triple bond, then it can fall over onto a vacant site, creating a bidentate. | ||
|
|
||
| *1-*2=*3 *1=*2-*3 | ||
| | ----> : | |
There was a problem hiding this comment.
Why do you make the vdW between *1 and *4? Is there a more simple recipe that puts the vdW bond between *3 and *5?
There was a problem hiding this comment.
Good catch. Yes, I could simplify the recipe
| #!/usr/bin/env python | ||
| # encoding: utf-8 | ||
|
|
||
| name = "Surface_Monodentate_to_Bidentate/rules" |
There was a problem hiding this comment.
update to include "vdW". Maybe update comment for the rule so that it is not identical to monodentate to bidentate, but probably fair for now to use the same rule.
| #!/usr/bin/env python | ||
| # encoding: utf-8 | ||
|
|
||
| name = "Surface_Monodentate_to_vdW_Bidentate" |
There was a problem hiding this comment.
Perhapse remove the '_' between vdW and bidentate to be consistent with the folder name. Maybe check the other groups.py and rules.py for this too.
There was a problem hiding this comment.
Fixed, and the others as well
| longDesc = u""" | ||
| Surface bond fission of one species into two distinct adsorbates. Atom *1 is bonded to the surface (*3). The image below shows a single bond, but single, double, and triple are possible. What matters is that the bond between *1 and *2 must be single. | ||
| # NOTE: we should probably include vdW, too! | ||
|
|
There was a problem hiding this comment.
The molecularity here is interesting. I bet at the TS it might look like it's using a third site. This is not simple unimolecular bidentate dissociation that we dealt with before. But In RMG world I guess this is the best way to describe it unless we left another vacant site in the initial and final state.
There was a problem hiding this comment.
Yeah, these reactions are odd. I'm also not a 100% sure what the correct order of this reaction would be. This is what I have seen others do in the literature. I'm not sure if this is the correct assumption though. However, I think it should be fine.
|
@bjkreitz I looked through the files and left some comments. The numbers and functionality seem to work, but there are a few places that comments could be made more consistent. |
|
Thanks @kirkbadger18! I incorporated your feedback and added another family to react formic acid to formate. Please review and let me know if anything else needs to be changed. |
|
@bjkreitz there is one remaining failing unit test - could you take a look? We would like to get this merged today, if possible. |
drop this commit before merging to main. merge at same time as ReactionMechanismGenerator/RMG-database#729
|
Fixed the bug. Hopefully it passes the unit tests now |
kirkbadger18
left a comment
kirkbadger18
left a comment
There was a problem hiding this comment.
Looks good! I ran the formic acid decomposition mechanism, and it found the path to form formate vdWBidentate from formic acid. It still left it out of the core, but that is okay for this PR. The functionality is there.
Regression Testing ResultsERROR conda.cli.main_run:execute(148): observablesobservable( species definition used in the reactor setup specificationspecies( species( reactor setupsreactorSetups( INFO:root:Thermo file has default temperature range 300.0 to 1000.0 and 1000.0 to 5000.0 observable( species( reactorSetups( INFO:root:Thermo file has default temperature range 300.0 to 1000.0 and 1000.0 to 5000.0 observable( observable( species( species( reactorSetups( INFO:root:Thermo file has default temperature range 300.0 to 1000.0 and 1000.0 to 5000.0 observablesobservable( species definition used in the reactor setup specificationspecies( species( species( species( reactor setupsreactorSetups( INFO:root:Thermo file has default temperature range 300.0 to 1000.0 and 1000.0 to 5000.0 observable( species( species( reactorSetups( INFO:root:Thermo file has default temperature range 300.0 to 1000.0 and 1000.0 to 5000.0 observable( observable( reactorSetups( observable( species( reactorSetups( INFO:root:Thermo file has default temperature range 300.0 to 1000.0 and 1000.0 to 5000.0 Detailed regression test results.Regression test aromatics:Reference: Execution time (DD:HH:MM:SS): 00:00:00:51 aromatics Passed Core Comparison ✅Original model has 15 species. aromatics Failed Edge Comparison ❌Original model has 106 species. Non-identical thermo! ❌
thermo: Thermo group additivity estimation: group(Cs-(Cds-Cds)CsCsH) + group(Cs-(Cds-Cds)CsCsH) + group(Cs-(Cds-Cds)(Cds-Cds)CsH) + group(Cs-(Cds-Cds)CsHH) + group(Cds-CdsCsCs) + group(Cds-CdsCsH) + group(Cds-CdsCsH) + group(Cds-CdsCsH) + polycyclic(s2_4_5_diene_1_5) + polycyclic(s3_4_5_ene_3) + polycyclic(s3_5_5_ene_1) - ring(Cyclobutene) - ring(Cyclopentene) - ring(Cyclopentane) + radical(cyclopentene-allyl) Non-identical thermo! ❌
thermo: Thermo group additivity estimation: group(Cs-(Cds-Cds)(Cds-Cds)CsCs) + group(Cs-(Cds-Cds)CsCsH) + group(Cs-(Cds-Cds)CsCsH) + group(Cs-CsCsHH) + group(Cds-CdsCsH) + group(Cds-CdsCsH) + group(Cds-CdsCsH) + group(Cds-CdsCsH) + polycyclic(s2_4_4_ene_1) + polycyclic(s1_4_5_diene_1_6) + polycyclic(s3_4_5_ene_1) - ring(Cyclobutene) - ring(Cyclobutane) - ring(Cyclopentene) + radical(bicyclo[2.1.1]hex-2-ene-C5) Non-identical thermo! ❌
thermo: Thermo group additivity estimation: group(Cs-CsCsCsH) + group(Cs-(Cds-Cds)CsCsH) + group(Cs-(Cds-Cds)CsCsH) + group(Cs-(Cds-Cds)CsHH) + group(Cds- Cds(Cds-Cds)Cs) + group(Cds-CdsCsH) + group(Cds-CdsCsH) + group(Cds-Cds(Cds-Cds)H) + polycyclic(s2_3_5_ene_1) + polycyclic(s2_3_6_diene_1_3) + Estimated bicyclic component: polycyclic(s3_5_6_ane) - ring(Cyclohexane) - ring(Cyclopentane) + ring(1,3-Cyclohexadiene) + ring(Cyclopentene) - ring(Cyclopropane) - ring(Cyclopentene) - ring(1,3-Cyclohexadiene) + radical(cyclopentene-allyl) Non-identical thermo! ❌
Identical thermo comments: Non-identical thermo! ❌
thermo: Thermo group additivity estimation: group(Cs-(Cds-Cds)(Cds-Cds)(Cds-Cds)H) + group(Cds-Cds(Cds-Cds)(Cds-Cds)) + group(Cds-CdsCsH) + group(Cds-CdsCsH) + group(Cds-Cds(Cds-Cds)H) + group(Cds-Cds(Cds-Cds)H) + group(Cds-CdsCsH) + group(Cdd-CdsCds) + Estimated bicyclic component: polycyclic(s4_6_6_ane) - ring(Cyclohexane) - ring(Cyclohexane) + ring(124cyclohexatriene) + ring(1,4-Cyclohexadiene) Non-identical thermo! ❌
thermo: Thermo group additivity estimation: group(Cs-(Cds-Cds)CsCsH) + group(Cs-(Cds-Cds)CsCsH) + group(Cs-(Cds-Cds)CsCsH) + group(Cs-CsCsHH) + group(Cds- Cds(Cds-Cds)Cs) + group(Cds-CdsCsH) + group(Cds-CdsCsH) + group(Cds-Cds(Cds-Cds)H) + polycyclic(s2_4_4_ene_1) + polycyclic(s3_4_6_ene_1) + Estimated bicyclic component: polycyclic(s2_4_6_ane) - ring(Cyclohexane) - ring(Cyclobutane) + ring(Cyclohexene) + ring(Cyclobutene) - ring(Cyclobutane) - ring(Cyclobutene) - ring(Cyclohexene) + radical(cyclobutane) Non-identical kinetics! ❌
kinetics: Non-identical kinetics! ❌
kinetics: Non-identical kinetics! ❌
kinetics: Non-identical kinetics! ❌
kinetics: Non-identical kinetics! ❌
kinetics: Non-identical kinetics! ❌
kinetics: Non-identical kinetics! ❌
kinetics: Non-identical kinetics! ❌
kinetics: Non-identical kinetics! ❌
kinetics: Non-identical kinetics! ❌
kinetics: Non-identical kinetics! ❌
kinetics: Non-identical kinetics! ❌
kinetics: Non-identical kinetics! ❌
kinetics: Non-identical kinetics! ❌
kinetics: Non-identical kinetics! ❌
kinetics: Non-identical kinetics! ❌
kinetics: Non-identical kinetics! ❌
kinetics: Non-identical kinetics! ❌
kinetics: DetailsObservables Test Case: Aromatics Comparison✅ All Observables varied by less than 0.500 on average between old model and new model in all conditions! aromatics Passed Observable Testing ✅Regression test liquid_oxidation:Reference: Execution time (DD:HH:MM:SS): 00:00:01:53 liquid_oxidation Passed Core Comparison ✅Original model has 37 species. liquid_oxidation Failed Edge Comparison ❌Original model has 214 species. Non-identical kinetics! ❌
kinetics: DetailsObservables Test Case: liquid_oxidation Comparison✅ All Observables varied by less than 0.100 on average between old model and new model in all conditions! liquid_oxidation Passed Observable Testing ✅Regression test nitrogen:Reference: Execution time (DD:HH:MM:SS): 00:00:00:56 nitrogen Passed Core Comparison ✅Original model has 41 species. nitrogen Failed Edge Comparison ❌Original model has 133 species. Non-identical thermo! ❌
thermo: Thermo group additivity estimation: group(O2s-CdN3d) + group(N3d-OCd) + group(Cd-HN3dO) + ring(oxirene) + radical(CdJ-NdO) Non-identical kinetics! ❌
kinetics: DetailsObservables Test Case: NC Comparison✅ All Observables varied by less than 0.200 on average between old model and new model in all conditions! nitrogen Passed Observable Testing ✅Regression test oxidation:Reference: Execution time (DD:HH:MM:SS): 00:00:01:32 oxidation Passed Core Comparison ✅Original model has 59 species. oxidation Passed Edge Comparison ✅Original model has 230 species. DetailsObservables Test Case: Oxidation Comparison✅ All Observables varied by less than 0.500 on average between old model and new model in all conditions! oxidation Passed Observable Testing ✅Regression test sulfur:Reference: Execution time (DD:HH:MM:SS): 00:00:00:38 sulfur Passed Core Comparison ✅Original model has 27 species. sulfur Failed Edge Comparison ❌Original model has 89 species. DetailsObservables Test Case: SO2 Comparison✅ All Observables varied by less than 0.100 on average between old model and new model in all conditions! sulfur Passed Observable Testing ✅Regression test superminimal:Reference: Execution time (DD:HH:MM:SS): 00:00:00:23 superminimal Passed Core Comparison ✅Original model has 13 species. superminimal Passed Edge Comparison ✅Original model has 18 species. Regression test RMS_constantVIdealGasReactor_superminimal:Reference: Execution time (DD:HH:MM:SS): 00:00:02:19 RMS_constantVIdealGasReactor_superminimal Passed Core Comparison ✅Original model has 13 species. RMS_constantVIdealGasReactor_superminimal Passed Edge Comparison ✅Original model has 13 species. DetailsObservables Test Case: RMS_constantVIdealGasReactor_superminimal Comparison✅ All Observables varied by less than 0.100 on average between old model and new model in all conditions! RMS_constantVIdealGasReactor_superminimal Passed Observable Testing ✅Regression test RMS_CSTR_liquid_oxidation:Reference: Execution time (DD:HH:MM:SS): 00:00:30:08 RMS_CSTR_liquid_oxidation Failed Core Comparison ❌Original model has 35 species. RMS_CSTR_liquid_oxidation Failed Edge Comparison ❌Original model has 100 species. DetailsObservables Test Case: RMS_CSTR_liquid_oxidation Comparison✅ All Observables varied by less than 0.100 on average between old model and new model in all conditions! RMS_CSTR_liquid_oxidation Passed Observable Testing ✅beep boop this comment was written by a bot 🤖 |
drop this commit before merging to main. merge at same time as ReactionMechanismGenerator/RMG-database#729
drop this commit before merging, and merge at the same time as ReactionMechanismGenerator/RMG-Py#2706
… is used The CI workflow checks out the RMG-Py branch named by an env var, but the env var was renamed to RMG_PYBRANCH (one underscore) in a4a0c62. The reference therefore resolved to an empty string, and actions/checkout silently fell back to the default branch (main) instead of the requested branch. Restore the matching name RMG_PY_BRANCH (both underscores) on the definition so the ref expression resolves and the intended branch is checked out. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
this is not actual revert, since that previous commit (ReactionMechanismGenerator/RMG-Py@5e967e6) also fixed a typo in this variable name
JacksonBurns
force-pushed
the
formate_families
branch
from
3320859 to
3f3b978
Compare
drop this commit before merging to main. merge at same time as ReactionMechanismGenerator/RMG-database#729
4 participants
I added new families for formate and other similar species that are bidentate species with a vdW_bond. This PR needs to be reviewed in tandem with RMG-Py PR#2706.
I added the following families:
Surface_Monodentate_to_vdW_Bidentate, example: [X]OC(H)O + [X] -> [X]~OC(H)O[X]Surface_Dissociation_vdW_Bidentate, example: [X]~OC(H)O[X] -> [X]O + O=C[X]HSurface_Dissociation_vdW_Bidentate_Beta, example: [X]~OC(H)O[X] -> OCO.[X] + H[X]Surface_Abstraction_vdW_Bidentate_Beta, example: [X]~OC(H)O[X] + O=[X]-> OCO.[X] + O[X] + [X]This is by no means complete and formate can undergo further reactions. However, it should cover some of the most relevant reaction types.
I still have to clean the families up and provide reasonable kinetic parameters, but this allows testing of the RMG-Py PR.
Here is an input file that can be used input.py
I had to get a little creative with the reaction templates since RMG cannot form
vdW_bonds. For example, take a look at the template forSurface_Dissociation_vdW_Bidentate:RMG cannot break a bond between *3 and *5 since there is technically no bond. The workaround is to first increment the bond order to form a single bond and then break it. Here is the recipe: