Pr 2698 #2733

bjkreitz · 2024-11-19T18:50:03Z

Motivation or Problem

A clear and concise description of what what you're trying to fix or improve. Please reference any issues that this addresses.

Description of Changes

A clear and concise description of what what you've changed or added.

Testing

A clear and concise description of testing that you've done or plan to do.

Reviewer Tips

Suggestions for verifying that this PR works or other notes for the reviewer.

For now it leaves behind a bunch of .png files so you can see if they look ok.

By temporarily connecting the surface sites to each other, the algorithm for drawing cycles kicks in and puts the sites near each other, instead of having one site floating away far from another.

Surface sites are connected to each other in a better order, based on how close they are in the molecule.

Don't make a list if the any() can break early from a generator.

This is an attempt to address complications with drawing fused rings, when you join together two surface sites that are adsorbing things in a ring.

…lly below.

The whole procedure of checking if an adatom is part of a ring was not really helping the graphics, and adding complication, so I removed it.

If the non-adatom part of the molecule pushed below the level of the sites, it looked bad. Now the sites are moved down a bit.

Things just look bad when you try drawing 5 or more sites all in a row at the bottom. You're better off imagining you've got a birds'-eye view not a side view.

An adatom is an atom that is adsorbed to the surface.

github-actions · 2024-11-19T21:13:45Z

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:05
Current: Execution time (DD:HH:MM:SS): 00:00:01:04
Reference: Memory used: 2759.25 MB
Current: Memory used: 2760.99 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 Passed Edge Comparison ✅

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

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:08
Current: Execution time (DD:HH:MM:SS): 00:00:02:08
Reference: Memory used: 2890.78 MB
Current: Memory used: 2886.80 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](31) <=> [OH](22) + CC=O(72) 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](31) <=> [OH](22) + CC=O(72) origin: intra_H_migration
The tested model has 1 reactions that the original model does not have. ❌
rxn: CCO[O](30) <=> C[CH]OO(70) origin: intra_H_migration

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:24
Reference: Memory used: 2897.56 MB
Current: Memory used: 2888.54 MB

nitrogen Passed Core Comparison ✅

Original model has 41 species.
Test model has 41 species. ✅
Original model has 360 reactions.
Test model has 360 reactions. ✅

nitrogen Failed Edge Comparison ❌

Original model has 133 species.
Test model has 133 species. ✅
Original model has 983 reactions.
Test model has 983 reactions. ✅

Non-identical thermo! ❌
original: O1[C]=N1
tested: O1[C]=N1

Hf(300K)	S(300K)	Cp(300K)	Cp(400K)	Cp(500K)	Cp(600K)	Cp(800K)	Cp(1000K)	Cp(1500K)
141.64	58.66	12.26	12.27	12.09	11.96	12.26	12.72	12.15
116.46	53.90	11.62	12.71	13.49	13.96	14.14	13.85	13.58

thermo: Thermo group additivity estimation: group(O2s-CdN3d) + group(N3d-OCd) + group(Cd-HN3dO) + ring(oxirene) + radical(CdJ-NdO)
thermo: Thermo group additivity estimation: group(O2s-CdN3d) + group(N3d-OCd) + group(Cd-HN3dO) + ring(Cyclopropene) + radical(CdJ-NdO)

Non-identical kinetics! ❌
original:
rxn: NCO(66) <=> O1[C]=N1(126) origin: Intra_R_Add_Endocyclic
tested:
rxn: NCO(66) <=> O1[C]=N1(126) origin: Intra_R_Add_Endocyclic

k(1bar)	300K	400K	500K	600K	800K	1000K	1500K	2000K
k(T):	-66.25	-46.19	-34.19	-26.21	-16.28	-10.36	-2.54	1.31
k(T):	-49.54	-33.65	-24.16	-17.85	-10.01	-5.35	0.80	3.82

kinetics: Arrhenius(A=(6.95187e+18,'s^-1'), n=-1.628, Ea=(111.271,'kcal/mol'), T0=(1,'K'), comment="""Estimated from node Backbone0_N-2R!H-inRing_N-1R!H-inRing_Sp-2R!H-1R!H""")
kinetics: Arrhenius(A=(6.95187e+18,'s^-1'), n=-1.628, Ea=(88.327,'kcal/mol'), T0=(1,'K'), comment="""Estimated from node Backbone0_N-2R!H-inRing_N-1R!H-inRing_Sp-2R!H-1R!H""")
Identical kinetics comments:
kinetics: Estimated from node Backbone0_N-2R!H-inRing_N-1R!H-inRing_Sp-2R!H-1R!H

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:26
Current: Execution time (DD:HH:MM:SS): 00:00:02:26
Reference: Memory used: 2748.37 MB
Current: Memory used: 2755.97 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. ✅

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:53
Current: Execution time (DD:HH:MM:SS): 00:00:00:53
Reference: Memory used: 2847.88 MB
Current: Memory used: 2860.91 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

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:35
Current: Execution time (DD:HH:MM:SS): 00:00:00:35
Reference: Memory used: 2965.46 MB
Current: Memory used: 2972.91 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:24
Current: Execution time (DD:HH:MM:SS): 00:00:02:22
Reference: Memory used: 3522.57 MB
Current: Memory used: 3480.44 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. ✅

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:55
Current: Execution time (DD:HH:MM:SS): 00:00:05:53
Reference: Memory used: 3414.52 MB
Current: Memory used: 3411.66 MB

RMS_CSTR_liquid_oxidation Failed Core Comparison ❌

Original model has 37 species.
Test model has 37 species. ✅
Original model has 233 reactions.
Test model has 232 reactions. ❌
The original model has 1 reactions that the tested model does not have. ❌
rxn: CCO[O](34) <=> [OH](21) + 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: CCO[O](34) <=> [OH](21) + CC=O(62) origin: intra_H_migration
rxn: CCCO[O](35) <=> [OH](21) + CCC=O(44) origin: intra_H_migration
The tested model has 2 reactions that the original model does not have. ❌
rxn: CCCO[O](35) <=> CC[CH]OO(45) origin: intra_H_migration
rxn: CCO[O](36) <=> C[CH]OO(62) origin: intra_H_migration

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

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:40
Current: Execution time (DD:HH:MM:SS): 00:00:00:39
Reference: Memory used: 2680.55 MB
Current: Memory used: 2689.82 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. ✅

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:04
Current: Execution time (DD:HH:MM:SS): 00:00:03:00
Reference: Memory used: 3652.67 MB
Current: Memory used: 3644.25 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. ✅

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 ✅

Regression test minimal_surface:

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: 2852.16 MB
Current: Memory used: 2868.92 MB

minimal_surface Passed Core Comparison ✅

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

minimal_surface Passed Edge Comparison ✅

Original model has 38 species.
Test model has 38 species. ✅
Original model has 38 reactions.
Test model has 38 reactions. ✅

Observables Test Case: minimal_surface Comparison

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

minimal_surface Passed Observable Testing ✅

beep boop this comment was written by a bot 🤖

codecov · 2024-11-19T21:13:55Z

Codecov Report

Attention: Patch coverage is 98.30508% with 1 line in your changes missing coverage. Please review.

Project coverage is 54.87%. Comparing base (36bceb3) to head (94aee97).
Report is 41 commits behind head on main.

Files with missing lines	Patch %	Lines
rmgpy/molecule/draw.py	98.30%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #2733      +/-   ##
==========================================
+ Coverage   54.79%   54.87%   +0.08%     
==========================================
  Files         125      125              
  Lines       37164    37231      +67     
==========================================
+ Hits        20365    20432      +67     
  Misses      16799    16799

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚨 Try these New Features:

Flaky Tests Detection - Detect and resolve failed and flaky tests

rwest and others added 20 commits July 30, 2024 16:02

Unit test for drawing poly-dentate species.

24c65dd

For now it leaves behind a bunch of .png files so you can see if they look ok.

One more test molecule for drawing a 4-dentate adsorbate.

e317713

Draw bidentate adsorbates better.

e31b485

By temporarily connecting the surface sites to each other, the algorithm for drawing cycles kicks in and puts the sites near each other, instead of having one site floating away far from another.

Better drawing of polydentate (n>2) adsorbates.

5129892

Surface sites are connected to each other in a better order, based on how close they are in the molecule.

Rotate bidentates so the sites are horizontally aligned

7cb6c81

Clean up files after testing drawings.

ba1054c

Small optimization.

052ced9

Don't make a list if the any() can break early from a generator.

Don't bond together sites that are attached to the main ring system.

7ca2adb

This is an attempt to address complications with drawing fused rings, when you join together two surface sites that are adsorbing things in a ring.

If you have more than 3 sites, rotate molecule and draw sites vertica…

353fa1f

…lly below.

Use the new rotating method for all polydentates.

d1f9017

Simplify drawing of ring-adsorbates

777d43b

The whole procedure of checking if an adatom is part of a ring was not really helping the graphics, and adding complication, so I removed it.

Lower the sites a bit for some molecules.

b0dd0e4

If the non-adatom part of the molecule pushed below the level of the sites, it looked bad. Now the sites are moved down a bit.

Small optimization

9af46c2

Tidying up the new coordinate code a bit. Reduce redundancy.

eb488f1

Don't mess with drawing surface sites if there are more than 4.

8ae3ed0

Things just look bad when you try drawing 5 or more sites all in a row at the bottom. You're better off imagining you've got a birds'-eye view not a side view.

Test: Draw a crazy big polydentate adsorbate

4deceff

fixup! Small optimization

cb4033d

Changing terminology. adatoms

647feab

An adatom is an atom that is adsorbed to the surface.

Change terminology in _[dis]connect_surface_sites

a6c14ae

remove duplicate code

94aee97

bjkreitz closed this Nov 19, 2024

rwest deleted the pr-2698 branch November 20, 2024 04:58

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Pr 2698 #2733

Pr 2698 #2733

bjkreitz commented Nov 19, 2024

github-actions bot commented Nov 19, 2024

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:

Regression test minimal_surface:

codecov bot commented Nov 19, 2024

Pr 2698 #2733

Pr 2698 #2733

Conversation

bjkreitz commented Nov 19, 2024

Motivation or Problem

Description of Changes

Testing

Reviewer Tips

github-actions bot commented Nov 19, 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:

Regression test minimal_surface:

codecov bot commented Nov 19, 2024

Codecov Report