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

remove reusable workflow logic, make RMG_DATABSE_BRANCH file-scoped #2512

Merged
merged 1 commit into from
Aug 1, 2023

Conversation

JacksonBurns
Copy link
Contributor

@JacksonBurns JacksonBurns commented Jul 31, 2023

See #2510 (comment)

Re-using the RMG-Py workflow removes the ability to (easily) run the tests from forks or to target forks, so we will instead copy the testing action and test files to RMG-database. The workflow will be similar but (as mentioned below) can skip some steps.

@rwest
Copy link
Member

rwest commented Jul 31, 2023

What's the idea here? (perhaps you can updated the summary to answer)

We stop trying to make a re-usable workflow, and just have a similar-ish workflow action in the RMG-database repository? That sounds reasonable. Downside is we'd have to remember to maintain it independently when we make tweaks and improvements to this workflow. Upsides are: it's simpler to get running, and it could be tailored to just database testing (eg. skip the unit test - just do database tests and regression tests). Am I understanding correctly?

@github-actions
Copy link

Regression Testing Results

⚠️ 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:47
Current: Execution time (DD:HH:MM:SS): 00:00:01:37
Reference: Memory used: 2085.00 MB
Current: Memory used: 2095.98 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:03:41
Current: Execution time (DD:HH:MM:SS): 00:00:03:15
Reference: Memory used: 2191.95 MB
Current: Memory used: 2223.74 MB

liquid_oxidation Failed Core Comparison ❌

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

Non-identical kinetics! ❌
original:
rxn: CCCC(C)O[O](20) + CCCCCO[O](104) <=> oxygen(1) + CCCC(C)[O](64) + CCCCC[O](128) origin: Peroxyl_Disproportionation
tested:
rxn: CCCC(C)O[O](20) + CCCCCO[O](103) <=> oxygen(1) + CCCC(C)[O](64) + CCCCC[O](128) 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.77 4.45 4.86 5.14 5.48 5.68 5.96 6.09

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.756,'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

liquid_oxidation Failed Edge Comparison ❌

Original model has 202 species.
Test model has 202 species. ✅
Original model has 1618 reactions.
Test model has 1610 reactions. ❌
The original model has 10 reactions that the tested model does not have. ❌
rxn: CCO[O](29) <=> C[CH]OO(70) origin: intra_H_migration
rxn: [CH2]CCOO(77) + CCCCCOO(105) <=> CCCOO(35) + CC[CH]CCOO(114) origin: H_Abstraction
rxn: [CH2]CCOO(77) + CCCCCOO(105) <=> CCCOO(35) + CCC[CH]COO(113) origin: H_Abstraction
rxn: [CH2]CCOO(77) + CCCCCOO(105) <=> CCCOO(35) + C[CH]CCCOO(115) origin: H_Abstraction
rxn: [CH2]CCOO(77) + CCCCCOO(105) <=> CCCOO(35) + CCCC[CH]OO(135) origin: H_Abstraction
rxn: CCCOO(35) + [CH2]CCCCOO(116) <=> [CH2]CCOO(77) + CCCCCOO(105) origin: H_Abstraction
rxn: C[CH]CCCO(157) + CCCCCO[O](104) <=> CC=CCCO(192) + CCCCCOO(105) origin: Disproportionation
rxn: C[CH]CCCO(157) + CCCCCO[O](104) <=> C=CCCCO(193) + CCCCCOO(105) origin: Disproportionation
rxn: C[CH]CCCO(157) + C[CH]CCCO(157) <=> CC=CCCO(192) + CCCCCO(130) origin: Disproportionation
rxn: C[CH]CCCO(157) + C[CH]CCCO(157) <=> C=CCCCO(193) + CCCCCO(130) origin: Disproportionation
The tested model has 2 reactions that the original model does not have. ❌
rxn: CCO[O](29) <=> [OH](22) + CC=O(72) origin: intra_H_migration
rxn: CCCCCO[O](103) + CCCCCO[O](103) <=> oxygen(1) + CCCCC=O(120) + CCCCCO(130) origin: Peroxyl_Termination

Non-identical kinetics! ❌
original:
rxn: CCCC(C)O[O](20) + CCCCCO[O](104) <=> oxygen(1) + CCCC(C)[O](64) + CCCCC[O](128) origin: Peroxyl_Disproportionation
tested:
rxn: CCCC(C)O[O](20) + CCCCCO[O](103) <=> oxygen(1) + CCCC(C)[O](64) + CCCCC[O](128) 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.77 4.45 4.86 5.14 5.48 5.68 5.96 6.09

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.756,'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: 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:02:20
Current: Execution time (DD:HH:MM:SS): 00:00:01:59
Reference: Memory used: 2205.09 MB
Current: Memory used: 2225.68 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 132 species.
Test model has 132 species. ✅
Original model has 997 reactions.
Test model has 997 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:03:47
Current: Execution time (DD:HH:MM:SS): 00:00:03:20
Reference: Memory used: 2072.38 MB
Current: Memory used: 2066.75 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:01:31
Current: Execution time (DD:HH:MM:SS): 00:00:01:19
Reference: Memory used: 2180.78 MB
Current: Memory used: 2169.87 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

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.

⚠️ The following reaction conditions had some discrepancies:
Condition 1:
Reactor Type: IdealGasReactor
Reaction Time: 0.01 s
T0: 900 K
P0: 30 bar
Initial Mole Fractions: {'S': 0.000756, '[O][O]': 0.00129, 'N#N': 0.997954}

sulfur Failed Observable Testing ❌

Regression test superminimal:

Reference: Execution time (DD:HH:MM:SS): 00:00:00:59
Current: Execution time (DD:HH:MM:SS): 00:00:00:49
Reference: Memory used: 2295.14 MB
Current: Memory used: 2333.63 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 Failed Edge Comparison ❌

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

Non-identical kinetics! ❌
original:
rxn: O2(2) + [O]O(4) => [O]OO[O](17) + [H](3) origin: PDepNetwork #5
tested:
rxn: O2(2) + [O]O(4) => [O]OO[O](17) + [H](3) origin: PDepNetwork #5

k(1bar) 300K 400K 500K 600K 800K 1000K 1500K 2000K
k(T): -82.04 -60.38 -47.27 -38.46 -27.30 -20.50 -11.24 -6.53
k(T): -83.19 -61.32 -48.07 -39.17 -27.88 -20.98 -11.59 -6.79

kinetics: Chebyshev(coeffs=[[-38.7,-3.696e-05,-2.573e-05,-1.429e-05],[37.66,-3.807e-05,-2.65e-05,-1.471e-05],[0.396,-1.752e-05,-1.219e-05,-6.77e-06],[0.09623,-7.467e-06,-5.197e-06,-2.885e-06],[0.01887,-2.976e-06,-2.072e-06,-1.15e-06],[-0.003474,-1.187e-06,-8.261e-07,-4.586e-07]], kunits='cm^3/(mol*s)', Tmin=(300,'K'), Tmax=(2000,'K'), Pmin=(0.01,'atm'), Pmax=(98.692,'atm'))
kinetics: Chebyshev(coeffs=[[-39.45,-4.002e-05,-2.786e-05,-1.547e-05],[38.1,-4.003e-05,-2.787e-05,-1.547e-05],[0.4317,-1.799e-05,-1.252e-05,-6.95e-06],[0.1078,-7.549e-06,-5.254e-06,-2.917e-06],[0.02697,-3.017e-06,-2.1e-06,-1.166e-06],[-0.006763,-1.18e-06,-8.21e-07,-4.557e-07]], kunits='cm^3/(mol*s)', Tmin=(300,'K'), Tmax=(2000,'K'), Pmin=(0.01,'atm'), Pmax=(98.692,'atm'))
Identical kinetics comments:
kinetics:

Regression test RMS_constantVIdealGasReactor_superminimal:

Reference: Execution time (DD:HH:MM:SS): 00:00:04:14
Current: Execution time (DD:HH:MM:SS): 00:00:03:53
Reference: Memory used: 2713.54 MB
Current: Memory used: 2737.12 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:10:18
Current: Execution time (DD:HH:MM:SS): 00:00:09:32
Reference: Memory used: 2720.19 MB
Current: Memory used: 2713.73 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](36) <=> [OH](22) + CC=O(61) 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 1 reactions that the tested model does not have. ❌
rxn: CCO[O](36) <=> C[CH]OO(62) origin: intra_H_migration
The tested model has 1 reactions that the original model does not have. ❌
rxn: CCO[O](36) <=> [OH](22) + CC=O(61) origin: intra_H_migration

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 🤖

@codecov
Copy link

codecov bot commented Jul 31, 2023

Codecov Report

Merging #2512 (10a1c33) into main (65dfeca) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##             main    #2512   +/-   ##
=======================================
  Coverage   48.54%   48.54%           
=======================================
  Files         110      110           
  Lines       30812    30812           
  Branches     8054     8054           
=======================================
  Hits        14959    14959           
- Misses      14329    14330    +1     
+ Partials     1524     1523    -1     

see 1 file with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

Copy link
Member

@rwest rwest left a comment

Choose a reason for hiding this comment

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

Looks like a reasonable plan.

@rwest rwest merged commit 5be8b0e into ReactionMechanismGenerator:main Aug 1, 2023
6 checks passed
rwest added a commit to rwest/RMG-Py that referenced this pull request Aug 2, 2023
This is what we did in the (abandoned) PR ReactionMechanismGenerator#2510 and should
have been done in PR ReactionMechanismGenerator#2512.

It means that when the CI runs on pushes
to a forked repository, it checks out that
forked repository.
rwest added a commit that referenced this pull request Aug 2, 2023
This is what we did in the (abandoned) PR #2510 and should have been done in PR #2512.
It means that when the CI runs on pushes to a forked repository, 
it checks out that forked repository, instead of the official one.
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.

None yet

2 participants