Draw reversible arrow if reaction is reversible #2693

sevyharris · 2024-07-18T16:57:15Z

Motivation or Problem

RMG reactions are typically reversible, but they are always drawn with a forward arrow → instead of the double harpoons. ⇌

Description of Changes

This PR makes it the default to draw the arrow as described by the reaction.reversible property (→ for False and ⇌ for True). If someone wants to revert to the way things were and always draw the arrow as →, they can set the ReactionDrawer option 'drawReversibleArrow' to False.

Testing

Here's some code to test out the results in a Jupyter notebook (note the last example writes to a file)

import rmgpy.reaction

# make an example reaction
my_reaction = rmgpy.reaction.Reaction()
my_reaction.reactants = [rmgpy.species.Species(smiles='CCCC'), rmgpy.species.Species(smiles='[OH]')]
my_reaction.products = [rmgpy.species.Species(smiles='O'), rmgpy.species.Species(smiles='[CH2]CCC')]

# show it with reversible arrows
display(my_reaction)

# show it with forward arrow
my_reaction.reversible = False
display(my_reaction)

# save an image of the reaction using the old method of always drawing just a forward arrow
my_reaction.reversible = True
rmgpy.molecule.draw.ReactionDrawer(options={'drawReversibleArrow': False}).draw(my_reaction, 'png', 'temp_reaction.png', )

Reviewer Tips

Any other thoughts on changing the default from →? Possible consequences downstream?

sevyharris · 2024-07-19T16:14:18Z

(top arrow was pointing backward and bottom pointing forward, so I fixed that and force pushed)

sevyharris · 2024-07-19T16:18:13Z

Hmm... for tall reactions the arrows are too far apart.

Maybe I'll set a maximum space between the harpoons.

sevyharris · 2024-07-19T16:28:01Z

Made the spacing a fixed number of pixels. Seems to have fixed it.

rwest

Looks good to me

sevyharris · 2024-07-22T14:18:40Z

Found an example that looks like it's not perfectly centered in the y-direction so I might look into that and make sure bigger molecules render okay here, but this example isn't too bad. It still appears to be in the approximate center of mass or so.

github-actions · 2024-07-29T23:45:14Z

Regression Testing Results

WARNING:root:Initial mole fractions do not sum to one; normalizing.
WARNING:root:Initial mole fractions do not sum to one; normalizing.
WARNING:root:Initial mole fractions do not sum to one; normalizing.
⚠️ One or more regression tests failed.
Please download the failed results and run the tests locally or check the log to see why.

Detailed regression test results.

Regression test aromatics:

Reference: Execution time (DD:HH:MM:SS): 00:00:01:08
Current: Execution time (DD:HH:MM:SS): 00:00:01:07
Reference: Memory used: 2773.66 MB
Current: Memory used: 2784.35 MB

aromatics Passed Core Comparison ✅

Original model has 15 species.
Test model has 15 species. ✅
Original model has 11 reactions.
Test model has 11 reactions. ✅

aromatics Failed Edge Comparison ❌

Original model has 106 species.
Test model has 106 species. ✅
Original model has 358 reactions.
Test model has 358 reactions. ✅

Non-identical thermo! ❌
original: C=CC1C=CC2=CC1C=C2
tested: C=CC1C=CC2=CC1C=C2

Hf(300K)	S(300K)	Cp(300K)	Cp(400K)	Cp(500K)	Cp(600K)	Cp(800K)	Cp(1000K)	Cp(1500K)
83.22	84.16	35.48	45.14	53.78	61.40	73.58	82.20	95.08
83.22	82.78	35.48	45.14	53.78	61.40	73.58	82.20	95.08

Identical thermo comments:
thermo: Thermo group additivity estimation: group(Cs-(Cds-Cds)(Cds-Cds)CsH) + group(Cs-(Cds-Cds)(Cds-Cds)CsH) + group(Cds-Cds(Cds-Cds)(Cds-Cds)) + group(Cds- CdsCsH) + group(Cds-CdsCsH) + group(Cds-CdsCsH) + group(Cds-CdsCsH) + group(Cds-Cds(Cds-Cds)H) + group(Cds-Cds(Cds-Cds)H) + group(Cds-CdsHH) + Estimated bicyclic component: polycyclic(s3_5_6_ane) - ring(Cyclohexane) - ring(Cyclopentane) + ring(1,3-Cyclohexadiene) + ring(Cyclopentadiene)

Details

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:02:12
Current: Execution time (DD:HH:MM:SS): 00:00:02:10
Reference: Memory used: 2925.42 MB
Current: Memory used: 2917.74 MB

liquid_oxidation Failed Core Comparison ❌

Original model has 37 species.
Test model has 37 species. ✅
Original model has 216 reactions.
Test model has 215 reactions. ❌
The original model has 1 reactions that the tested model does not have. ❌
rxn: CCO[O](29) <=> [OH](22) + CC=O(69) origin: intra_H_migration

liquid_oxidation Failed Edge Comparison ❌

Original model has 202 species.
Test model has 202 species. ✅
Original model has 1610 reactions.
Test model has 1610 reactions. ✅
The original model has 1 reactions that the tested model does not have. ❌
rxn: CCO[O](29) <=> [OH](22) + CC=O(69) origin: intra_H_migration
The tested model has 1 reactions that the original model does not have. ❌
rxn: CCO[O](31) <=> C[CH]OO(70) origin: intra_H_migration

Non-identical kinetics! ❌
original:
rxn: CCCCCO[O](104) + CC(CC(C)OO)O[O](103) <=> oxygen(1) + CCCCC[O](128) + CC([O])CC(C)OO(127) origin: Peroxyl_Disproportionation
tested:
rxn: CCCCCO[O](104) + CC(CC(C)OO)O[O](103) <=> oxygen(1) + CCCCC[O](127) + CC([O])CC(C)OO(129) origin: Peroxyl_Disproportionation

k(1bar)	300K	400K	500K	600K	800K	1000K	1500K	2000K
k(T):	3.52	4.27	4.71	5.01	5.39	5.61	5.91	6.06
k(T):	7.79	7.46	7.21	7.00	6.67	6.41	5.94	5.60

kinetics: Arrhenius(A=(3.2e+12,'cm^3/(mol*s)'), n=0, Ea=(4.096,'kcal/mol'), T0=(1,'K'), comment="""Estimated from node Root_Ext-5R-R_7R!H->C_N-7C-inRing_Ext-5R-R""")
kinetics: Arrhenius(A=(3.18266e+20,'cm^3/(mol*s)'), n=-2.694, Ea=(0.053,'kcal/mol'), T0=(1,'K'), comment="""Estimated from node Root_Ext-5R-R_7R!H->C_N-7C-inRing Ea raised from 0.0 to 0.2 kJ/mol to match endothermicity of reaction.""")
kinetics: Estimated from node Root_Ext-5R-R_7R!H->C_N-7C-inRing_Ext-5R-R
kinetics: Estimated from node Root_Ext-5R-R_7R!H->C_N-7C-inRing
Ea raised from 0.0 to 0.2 kJ/mol to match endothermicity of reaction.

Details

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:25
Current: Execution time (DD:HH:MM:SS): 00:00:01:27
Reference: Memory used: 2922.64 MB
Current: Memory used: 2908.66 MB

nitrogen Failed Core Comparison ❌

Original model has 41 species.
Test model has 41 species. ✅
Original model has 360 reactions.
Test model has 359 reactions. ❌
The original model has 1 reactions that the tested model does not have. ❌
rxn: HNO(48) + HCO(13) <=> NO(38) + CH2O(18) origin: H_Abstraction

nitrogen Failed Edge Comparison ❌

Original model has 132 species.
Test model has 132 species. ✅
Original model has 997 reactions.
Test model has 995 reactions. ❌
The original model has 2 reactions that the tested model does not have. ❌
rxn: HNO(48) + HCO(13) <=> NO(38) + CH2O(18) origin: H_Abstraction
rxn: HON(T)(83) + HCO(13) <=> NO(38) + CH2O(18) origin: Disproportionation

Details

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:02:25
Current: Execution time (DD:HH:MM:SS): 00:00:02:23
Reference: Memory used: 2787.54 MB
Current: Memory used: 2780.34 MB

oxidation Passed Core Comparison ✅

Original model has 59 species.
Test model has 59 species. ✅
Original model has 694 reactions.
Test model has 694 reactions. ✅

oxidation Passed Edge Comparison ✅

Original model has 230 species.
Test model has 230 species. ✅
Original model has 1526 reactions.
Test model has 1526 reactions. ✅

Details

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:00:57
Current: Execution time (DD:HH:MM:SS): 00:00:00:56
Reference: Memory used: 2889.38 MB
Current: Memory used: 2880.38 MB

sulfur Passed Core Comparison ✅

Original model has 27 species.
Test model has 27 species. ✅
Original model has 74 reactions.
Test model has 74 reactions. ✅

sulfur Failed Edge Comparison ❌

Original model has 89 species.
Test model has 89 species. ✅
Original model has 227 reactions.
Test model has 227 reactions. ✅
The original model has 1 reactions that the tested model does not have. ❌
rxn: O(4) + SO2(15) (+N2) <=> SO3(16) (+N2) origin: primarySulfurLibrary
The tested model has 1 reactions that the original model does not have. ❌
rxn: O(4) + SO2(15) (+N2) <=> SO3(16) (+N2) origin: primarySulfurLibrary

Details

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:39
Current: Execution time (DD:HH:MM:SS): 00:00:00:39
Reference: Memory used: 2986.12 MB
Current: Memory used: 2991.05 MB

superminimal Passed Core Comparison ✅

Original model has 13 species.
Test model has 13 species. ✅
Original model has 21 reactions.
Test model has 21 reactions. ✅

superminimal Passed Edge Comparison ✅

Original model has 18 species.
Test model has 18 species. ✅
Original model has 28 reactions.
Test model has 28 reactions. ✅

Regression test RMS_constantVIdealGasReactor_superminimal:

Reference: Execution time (DD:HH:MM:SS): 00:00:02:23
Current: Execution time (DD:HH:MM:SS): 00:00:02:22
Reference: Memory used: 3445.90 MB
Current: Memory used: 3442.64 MB

RMS_constantVIdealGasReactor_superminimal Passed Core Comparison ✅

Original model has 13 species.
Test model has 13 species. ✅
Original model has 19 reactions.
Test model has 19 reactions. ✅

RMS_constantVIdealGasReactor_superminimal Passed Edge Comparison ✅

Original model has 13 species.
Test model has 13 species. ✅
Original model has 19 reactions.
Test model has 19 reactions. ✅

Details

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:05:57
Current: Execution time (DD:HH:MM:SS): 00:00:05:56
Reference: Memory used: 3365.81 MB
Current: Memory used: 3383.88 MB

RMS_CSTR_liquid_oxidation Failed Core Comparison ❌

Original model has 37 species.
Test model has 37 species. ✅
Original model has 232 reactions.
Test model has 233 reactions. ❌
The tested model has 1 reactions that the original model does not have. ❌
rxn: CCO[O](35) <=> [OH](22) + CC=O(62) origin: intra_H_migration

RMS_CSTR_liquid_oxidation Failed Edge Comparison ❌

Original model has 206 species.
Test model has 206 species. ✅
Original model has 1508 reactions.
Test model has 1508 reactions. ✅
The original model has 2 reactions that the tested model does not have. ❌
rxn: CCCO[O](34) <=> CC[CH]OO(45) origin: intra_H_migration
rxn: CCO[O](35) <=> C[CH]OO(62) origin: intra_H_migration
The tested model has 2 reactions that the original model does not have. ❌
rxn: CCO[O](35) <=> [OH](22) + CC=O(62) origin: intra_H_migration
rxn: CCCO[O](36) <=> [OH](22) + CCC=O(50) origin: intra_H_migration

Non-identical kinetics! ❌
original:
rxn: CCCO[O](34) + CCCC(C)O[O](33) <=> oxygen(1) + CCC[O](96) + CCCC(C)[O](61) origin: Peroxyl_Disproportionation
tested:
rxn: CCCO[O](36) + CCCC(C)O[O](33) <=> oxygen(1) + CCC[O](92) + CCCC(C)[O](65) origin: Peroxyl_Disproportionation

k(1bar)	300K	400K	500K	600K	800K	1000K	1500K	2000K
k(T):	7.83	7.49	7.23	7.02	6.68	6.42	5.95	5.61
k(T):	3.69	4.39	4.82	5.10	5.45	5.66	5.94	6.08

kinetics: Arrhenius(A=(3.18266e+20,'cm^3/(mol*s)'), n=-2.694, Ea=(0,'kcal/mol'), T0=(1,'K'), comment="""Estimated from node Root_Ext-5R-R_7R!H->C_N-7C-inRing""")
kinetics: Arrhenius(A=(3.2e+12,'cm^3/(mol*s)'), n=0, Ea=(3.866,'kcal/mol'), T0=(1,'K'), comment="""Estimated from node Root_Ext-5R-R_7R!H->C_N-7C-inRing_Ext-5R-R""")
kinetics: Estimated from node Root_Ext-5R-R_7R!H->C_N-7C-inRing
kinetics: Estimated from node Root_Ext-5R-R_7R!H->C_N-7C-inRing_Ext-5R-R

Details

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 ✅

Regression test fragment:

Reference: Execution time (DD:HH:MM:SS): 00:00:00:43
Current: Execution time (DD:HH:MM:SS): 00:00:00:43
Reference: Memory used: 2717.11 MB
Current: Memory used: 2700.79 MB

fragment Passed Core Comparison ✅

Original model has 10 species.
Test model has 10 species. ✅
Original model has 2 reactions.
Test model has 2 reactions. ✅

fragment Passed Edge Comparison ✅

Original model has 33 species.
Test model has 33 species. ✅
Original model has 47 reactions.
Test model has 47 reactions. ✅

Details

Observables Test Case: fragment Comparison

✅ All Observables varied by less than 0.100 on average between old model and new model in all conditions!

fragment Passed Observable Testing ✅

Regression test RMS_constantVIdealGasReactor_fragment:

Reference: Execution time (DD:HH:MM:SS): 00:00:03:03
Current: Execution time (DD:HH:MM:SS): 00:00:03:08
Reference: Memory used: 3626.20 MB
Current: Memory used: 3602.57 MB

RMS_constantVIdealGasReactor_fragment Passed Core Comparison ✅

Original model has 10 species.
Test model has 10 species. ✅
Original model has 2 reactions.
Test model has 2 reactions. ✅

RMS_constantVIdealGasReactor_fragment Passed Edge Comparison ✅

Original model has 27 species.
Test model has 27 species. ✅
Original model has 24 reactions.
Test model has 24 reactions. ✅

Details

Observables Test Case: RMS_constantVIdealGasReactor_fragment Comparison

✅ All Observables varied by less than 0.100 on average between old model and new model in all conditions!

RMS_constantVIdealGasReactor_fragment Passed Observable Testing ✅

beep boop this comment was written by a bot 🤖

rwest · 2024-07-30T03:27:24Z

but this example isn't too bad. It still appears to be in the approximate center of mass or so.

I agree that looks fine to me - near the center of mass. Unless you have examples of it looking much weirder (perhaps one big one small?) and being the fault of this PR, I would call this good.

sevyharris · 2024-07-30T12:42:30Z

Yeah, here are some larger images that look fine. Probably good to merge.

sevyharris added the Complexity: Low label Jul 18, 2024

sevyharris force-pushed the reversible_arrow branch 2 times, most recently from 1f6da28 to 8519fe8 Compare July 19, 2024 16:12

sevyharris force-pushed the reversible_arrow branch from 8519fe8 to e4d4c0e Compare July 19, 2024 16:27

rwest approved these changes Jul 19, 2024

View reviewed changes

add option of drawing reactions with reversible arrow

e42f89b

rwest force-pushed the reversible_arrow branch from e4d4c0e to e42f89b Compare July 29, 2024 21:20

rwest merged commit 8eb302e into ReactionMechanismGenerator:main Jul 30, 2024

JacksonBurns mentioned this pull request Jul 10, 2025

RMG-Py Version 3.3.0 #2826

Merged

Draw reversible arrow if reaction is reversible #2693

Draw reversible arrow if reaction is reversible #2693

Uh oh!

Conversation

sevyharris commented Jul 18, 2024

Motivation or Problem

Description of Changes

Testing

Reviewer Tips

Uh oh!

sevyharris commented Jul 19, 2024

Uh oh!

sevyharris commented Jul 19, 2024

Uh oh!

sevyharris commented Jul 19, 2024

Uh oh!

rwest left a comment

Choose a reason for hiding this comment

Uh oh!

sevyharris commented Jul 22, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions bot commented Jul 29, 2024

Regression Testing Results

Regression test aromatics:

Regression test liquid_oxidation:

Regression test nitrogen:

Regression test oxidation:

Regression test sulfur:

Regression test superminimal:

Regression test RMS_constantVIdealGasReactor_superminimal:

Regression test RMS_CSTR_liquid_oxidation:

Regression test fragment:

Regression test RMS_constantVIdealGasReactor_fragment:

Uh oh!

rwest commented Jul 30, 2024

Uh oh!

sevyharris commented Jul 30, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

sevyharris commented Jul 22, 2024 •

edited

Loading