adding resonance for adsorbates #2511
Conversation
bjkreitz
force-pushed
the
adsorbate_resonance
branch
from
b29e890
to
a97d2d3
Compare
rmgpy/molecule/pathfinder.py
Outdated
| Find all bidentate adsorbates which have a bonding configuration X-C-C-X. | ||
| Examples: | ||
|
|
||
| - XCXC, XCHXCH, XCXCH, where X is the surface site |
There was a problem hiding this comment.
What is the meaning of, e.g., XCXC, is the second X connected to the 1st or 2nd carbon? Or maybe it doesn't matter?
There was a problem hiding this comment.
The notation for (bidentate) adsorbates is a little tricky and there is no real consensus in the literature. I always place the adsorption site (the 'X') on the left side of the adatom, e.g. XC means that a carbon atom is bonded to the surface and XCXC means that the first and the second carbon atom have a bond to the surface. That said, CXCX or XCCX would be perfectly fine, too. I add some text to explain my notation.
rmgpy/molecule/pathfinder.py
Outdated
| for atom2, bond12 in atom1.edges.items(): | ||
| if atom2.is_carbon(): | ||
| for atom3, bond23 in atom2.edges.items(): | ||
| if atom1 is not atom3 and atom3.is_carbon(): |
There was a problem hiding this comment.
since we know that atom1 is a surface atom, then if atom3.is_carbon() is True, we also know that atom1 is not atom3, so I think we can remove the atom1 is not atom3 check for better readability and maintenance. Same for atom2 is not atom4 below.
There was a problem hiding this comment.
I agree and removed the check
rmgpy/molecule/pathfinder.py
Outdated
| for atom4, bond34 in atom3.edges.items(): | ||
| if atom2 is not atom4 and atom4.is_surface_site(): | ||
| path.append([atom1, atom2, atom3, atom4, bond12, bond23, bond34]) | ||
| return path |
There was a problem hiding this comment.
I'm not sure regarding returning here. If we'll case about tridentates in the future, then this function will not explore all paths. Maybe OK to just use the return at the end? Are we worried that otherwise it'll be expensive?
(same comment for the function below)
There was a problem hiding this comment.
Thanks for the hint. I think it is fine to use return only at the end.
rmgpy/molecule/pathfinder.py
Outdated
| """ | ||
| cython.declare(paths=list, atom2=Vertex, atom3=Vertex, atom4=Vertex, bond12=Edge, bond23=Edge, bond34=Edge) | ||
|
|
||
| path=[] |
There was a problem hiding this comment.
minor: please add a white space before and after operator
rmgpy/molecule/pathfinder.py
Outdated
| path=[] | ||
| if atom1.is_surface_site(): | ||
| for atom2, bond12 in atom1.edges.items(): | ||
| for atom3, bond23 in atom2.edges.items(): |
There was a problem hiding this comment.
perhaps we're missing an if atom2.is_carbon() before this loop
There was a problem hiding this comment.
Yep, that is missing.
rmgpy/molecule/pathfinder.py
Outdated
| for atom3, bond23 in atom2.edges.items(): | ||
| if atom1 is not atom3 and atom3.is_carbon(): | ||
| for atom4, bond34 in atom3.edges.items(): | ||
| if atom2 is not atom4 and atom4.is_carbon(): |
There was a problem hiding this comment.
This if atom2 is not atom4 condition is very important, but I think we should remove all other if atom_X is not atom_Y checks for better readability and maintenance
There was a problem hiding this comment.
I removed the checks
| or ``False`` otherwise. | ||
| """ | ||
| cython.declare(atom=Atom) | ||
| if len([atom for atom in self.atoms if atom.is_surface_site()])>=2: |
There was a problem hiding this comment.
we also return True if the molecule is tridentate. Would we like to reflect that in the docstring (if not in the function name)?
There was a problem hiding this comment.
Good catch. I renamed the function to is_multidentate since all multidentate species can have resonance.
| """ | ||
| Generate all of the resonance structures formed by the shift of two electrons in a conjugated pi bond system of | ||
| a bidentate adsorbate with a bridging C atom in between. | ||
| Example XCHCHXC: [X]#CC=C[X] <=> [X]=C=CC=[X] |
There was a problem hiding this comment.
A base case example would help understanding (and good to also keep the preset example as well)
There was a problem hiding this comment.
What do you mean? I thought XCHCHXC: [X]#CC=C[X] <=> [X]=C=CC=[X] was the example.
rmgpy/molecule/pathfinder.py
Outdated
| Find all bidentate adsorbates which have a bonding configuration X-C-C-C-X. | ||
| Examples: | ||
|
|
||
| - XCHCHXCH/XCHCHXC, where X is the surface site |
There was a problem hiding this comment.
It's a bit confusing for me that X is not always listed at the end. Is this customary?
There was a problem hiding this comment.
Unfortunately, the notation for adsorbates is a little confusing since it is difficult to represent the adsorbate structure as a string of letters. This is the current notation that we and some other groups use.
rmgpy/molecule/resonance.py
Outdated
| if bond23.is_single(): | ||
| if (bond12.is_double() or bond12.is_triple()): | ||
| if (bond34.is_double() or bond34.is_triple()): | ||
| if (bond45.is_single() or bond45.is_double()): |
There was a problem hiding this comment.
Consider turning these four nested if conditions into a single if condition with and in between.
Not a strong recommendation though, if you think the and version will be less readable, then OK to keep the nested version
rmgpy/molecule/resonance.py
Outdated
| for atom in mol.vertices: | ||
| paths = pathfinder.find_adsorbate_conjugate_delocalization_paths(atom) | ||
| for atom1, atom2, atom3, atom4, atom45, bond12, bond23, bond34, bond45 in paths: | ||
| if bond23.is_single(): |
There was a problem hiding this comment.
Do you think this pathway could also be valid if bond_23 is double?
If I apply the recipe for
X1 {D/T} C2 {S} C3 {D/T} C4 {S/D} X5
I get
X1 {S/D} C2 {D} C3 {S/D} C4 {D/T} X5
reversing the atom labels this is:
X1 {D/T} C2 {S/D} C3 {D} C4 {S/D} X5
I expected that the 3rd line will be identical to the 1st line, since the recipe could be equally applied for it as well.
So maybe we should check for these bonds to get a mirror image:
X1 {D/T} C2 {S/D} C3 {D/T} C4 {S/D} X5
There was a problem hiding this comment.
Thanks, Alon. You are right, bond_23 could also be a double bond. I changed the if statement.
|
Thank you for the thorough review, Alon. I incorporated all your comments. Also, the functions |
bjkreitz
force-pushed
the
adsorbate_resonance
branch
from
da17918
to
ff95b0e
Compare
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #2511 +/- ##
==========================================
- Coverage 55.32% 55.18% -0.15%
==========================================
Files 125 125
Lines 37139 37263 +124
==========================================
+ Hits 20547 20562 +15
- Misses 16592 16701 +109 ☔ View full report in Codecov by Sentry. |
Regression Testing Results
Detailed regression test results.Regression test aromatics:Reference: Execution time (DD:HH:MM:SS): 00:00:01:37 aromatics Passed Core Comparison ✅Original model has 15 species. aromatics Passed Edge Comparison ✅Original model has 106 species.
Observables 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:03:14 liquid_oxidation Passed Core Comparison ✅Original model has 37 species. liquid_oxidation Passed Edge Comparison ✅Original model has 202 species.
Observables 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:01:56 nitrogen Passed Core Comparison ✅Original model has 41 species. nitrogen Failed Edge Comparison ❌Original model has 132 species. Non-identical thermo! ❌
thermo: Thermo group additivity estimation: group(O2s-CdN3d) + group(N3d-OCd) + group(Cd-HN3dO) + ring(Cyclopropene) + radical(CdJ-NdO) Non-identical kinetics! ❌
kinetics:
Observables 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:03:08 oxidation Passed Core Comparison ✅Original model has 59 species. oxidation Passed Edge Comparison ✅Original model has 230 species.
Observables 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:01:19 sulfur Passed Core Comparison ✅Original model has 27 species. sulfur Failed Edge Comparison ❌Original model has 89 species.
Observables 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:48 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:03:48 RMS_constantVIdealGasReactor_superminimal Passed Core Comparison ✅Original model has 13 species. RMS_constantVIdealGasReactor_superminimal Passed Edge Comparison ✅Original model has 13 species.
Observables 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:09:17 RMS_CSTR_liquid_oxidation Failed Core Comparison ❌Original model has 37 species. RMS_CSTR_liquid_oxidation Failed Edge Comparison ❌Original model has 206 species.
Observables 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 🤖 |
Regression Testing Results
Detailed regression test results.Regression test aromatics:Reference: Execution time (DD:HH:MM:SS): 00:00:01:37 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)(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 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:
Observables 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:03:14 liquid_oxidation Failed Core Comparison ❌Original model has 37 species. liquid_oxidation Failed Edge Comparison ❌Original model has 202 species. Non-identical kinetics! ❌
kinetics:
Observables 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:01:56 nitrogen Failed Core Comparison ❌Original model has 41 species. nitrogen Failed Edge Comparison ❌Original model has 132 species. Non-identical thermo! ❌
thermo: Thermo group additivity estimation: group(O2s-CdN3d) + group(N3d-OCd) + group(Cd-HN3dO) + ring(Cyclopropene) + radical(CdJ-NdO) Non-identical kinetics! ❌
kinetics:
Observables 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:03:08 oxidation Passed Core Comparison ✅Original model has 59 species. oxidation Passed Edge Comparison ✅Original model has 230 species.
Observables 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:01:19 sulfur Passed Core Comparison ✅Original model has 27 species. sulfur Failed Edge Comparison ❌Original model has 89 species.
Observables 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:48 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:03:48 RMS_constantVIdealGasReactor_superminimal Passed Core Comparison ✅Original model has 13 species. RMS_constantVIdealGasReactor_superminimal Passed Edge Comparison ✅Original model has 13 species.
Observables 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:09:17 RMS_CSTR_liquid_oxidation Failed Core Comparison ❌Original model has 37 species. RMS_CSTR_liquid_oxidation Failed Edge Comparison ❌Original model has 206 species. Non-identical kinetics! ❌
kinetics:
Observables 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 🤖 |
Regression Testing Results
Detailed regression test results.Regression test aromatics:Reference: Execution time (DD:HH:MM:SS): 00:00:01:37 aromatics Passed Core Comparison ✅Original model has 15 species. aromatics Failed Edge Comparison ❌Original model has 106 species. 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 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:
Observables 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:03:14 liquid_oxidation Failed Core Comparison ❌Original model has 37 species. Non-identical kinetics! ❌
kinetics: liquid_oxidation Failed Edge Comparison ❌Original model has 202 species. Non-identical kinetics! ❌
kinetics: Non-identical kinetics! ❌
kinetics:
Observables 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:01:56 nitrogen Failed Core Comparison ❌Original model has 41 species. nitrogen Failed Edge Comparison ❌Original model has 132 species.
Observables 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:03:08 oxidation Passed Core Comparison ✅Original model has 59 species. oxidation Passed Edge Comparison ✅Original model has 230 species.
Observables 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:01:19 sulfur Passed Core Comparison ✅Original model has 27 species. sulfur Failed Edge Comparison ❌Original model has 89 species.
Observables Test Case: SO2 Comparison
The following observables did not match: ❌ Observable species O=S=O varied by more than 0.100 on average between old model SO2(15) and new model SO2(15) in condition 1.
sulfur Failed Observable Testing ❌Regression test superminimal:Reference: Execution time (DD:HH:MM:SS): 00:00:00:48 superminimal Passed Core Comparison ✅Original model has 13 species. superminimal Failed Edge Comparison ❌Original model has 18 species. Non-identical kinetics! ❌
kinetics: Regression test RMS_constantVIdealGasReactor_superminimal:Reference: Execution time (DD:HH:MM:SS): 00:00:03:48 RMS_constantVIdealGasReactor_superminimal Passed Core Comparison ✅Original model has 13 species. RMS_constantVIdealGasReactor_superminimal Passed Edge Comparison ✅Original model has 13 species.
Observables 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:09:17 RMS_CSTR_liquid_oxidation Passed Core Comparison ✅Original model has 37 species. RMS_CSTR_liquid_oxidation Failed Edge Comparison ❌Original model has 206 species. Non-identical kinetics! ❌
kinetics:
Observables 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 🤖 |
|
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. |
|
Just making a comment so that it is no longer a stale PR. I'll pick up the work on resonance again soon. |
|
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
added
stale
Motivation or Problem
Adsorbate molecules can exhibit resonance similar to gas-phase molecules. For example, the bidentate adsorbate XCXC (where X is a surface site) has the following three configurations:
[X]C#C[X] <-> [X]#CC#[X] <-> [X]=C=C=[X]
Currently, RMG cannot identify resonance structures for adsorbates.
In this pull request, I made the first step and added functions for the generation of resonance structures for adsorbates.
Description of Changes
Resonance occurs only for adsorbates that have at least two binding sites, also known as bidentate adsorbates.
Therefore, a function was added to molecule.py to check if an adsorbate has two binding sites.
The pathfinder.py was updated with functions to find the structures of adsorbates which can eventually have resonance structures. The structures that are searched for in the adsorbates are X-C-C-X and X-C-C-C-X.
Five types of resonance for adsorbates were added to resonance.py. The identified and implemented resonance types are as follows:
Apologies for the funky names of the resonance types. Resonance has not really been discussed in catalysis yet.
This pull request is work in progress and requires more changes to the source code. This is meant to start the discussion on resonance for adsorbates.
Testing
I have tested the functionalities of the added resonance functions by providing various adsorbates with and without resonance and confirmed that the functions work properly.
An RMG run for an catalytic system completed also without any errors.
Reviewer Tips
Use the functions in a standalone Jupyter notebook and see if RMG can identify the resonance structures correctly. A selection of bidentate adsorbates with resonance is: "XCXC, XCHXCH, XCXCH, XCXCCH2, XCHXCCH2, XCHXCXC, XCHXCXCH, XCHCXC, XCHCHXC, XCHCHXCH" where X is a surface atom.
Open Tasks
A function is required to generate the resonance structures for bidentate format (XOC(H)XO) and bicarbonate (XOC(OH)XO).