RMG-Py release v3.2.0 #2311
RMG-Py release v3.2.0 #2311
Conversation
Codecov Report
@@ Coverage Diff @@
## stable #2311 +/- ##
==========================================
+ Coverage 47.18% 48.53% +1.34%
==========================================
Files 88 110 +22
Lines 23141 30811 +7670
Branches 6014 8054 +2040
==========================================
+ Hits 10920 14955 +4035
- Misses 11070 14330 +3260
- Partials 1151 1526 +375
... and 32 files with indirect coverage changes 📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
@kspieks thanks for your comments! I pushed the changes, so let me know if I got any of your suggestions wrong. Also, feel free to check out the RMG-Py release notes when I post them (hopefully later today). Any comments will be appreciated :) |
sevyharris
force-pushed
the
v3.2.0
branch
3 times, most recently
from
07c3c60
to
bddbafd
Compare
|
This pull request introduces 46 alerts and fixes 5 when merging 0a70c79 into 067aace - view on LGTM.com new alerts:
fixed alerts:
|
|
This pull request introduces 46 alerts and fixes 5 when merging 0d9c91a into 067aace - view on LGTM.com new alerts:
fixed alerts:
|
sevyharris
added
Status: WIP
|
This pull request introduces 46 alerts and fixes 5 when merging 63d10a6 into 067aace - view on LGTM.com new alerts:
fixed alerts:
|
|
This pull request introduces 46 alerts and fixes 5 when merging b813f45 into 067aace - view on LGTM.com new alerts:
fixed alerts:
|
There was a problem hiding this comment.
How well does b813f45 work and what are the consequences. I like (and in fact advocated for) the motivation - not using rmg channel when we can get away with it - but I now recall why we put some of these pins in: to massively accelerate the conda solving process. Once we realized that the only feasible solution to a consistent set was to get a package from the rmg channel, we put the pin in so that conda found that solution more quickly. Without it, the conda solves were timing out.
|
This pull request introduces 46 alerts and fixes 5 when merging 338504f into 067aace - view on LGTM.com new alerts:
fixed alerts:
|
|
I can't say for sure whether or not it will be compatible. The main concern would be changes to muq since 2019. The version in the RMG channel is 0.2.0 while conda-forge has builds of 0.3.1-0.4.2. Although it's probably equally likely that global uncertainty has been broken by changes to RMG, since there aren't any automated tests for the uncertainty module to my knowledge (sorry!). The best options for testing manually would probably be either the example jupyter notebook or the example for the uncertainty paper. If the uncertainty modules work with the old muq package in the RMG channel but not with the newer conda-forge versions, it might be best to reach out to Matthew Parno (who seems to be at Dartmouth now) to see if he can provide advice about what has changed in muq. |
Forgot to define the pull request number in the environment
The workflow is triggered by things other than pull requests, but only works for pull requests. This should make it not fail on other events. Also reports the github.event.workflow_run in case there are clues in there that would let us filter it differently.
The continue-on-error:true makes it think that step succeeded (though it didn't) so the "do on failure" doesn't run, and then the next step fails. Now we let it fail, and at least report in json the information that may let us try something else. One idea is to cange the javascript around line 32 that gets the matchArtifact and if there isn't one then return there and behave differently.
Hoping this works. Also fix a problem with escaping ' inside JSON when logging the github.event.workflow_run context.
Rather than run a pointless "the workflow failed" job, we just skip it.
The Continuous Integration workflow runs on - pushes - pull requests - a schedule - workflow calls from database testing With this change, we create the regression test summary and cat it into the workflow summary for all job types, though it is still only uploaded as an artifact for pull requests.
Library reactions marked with an ``elementary_high_p`` flag set to ``True`` can either have PDep kinetics or Arrhenius kinetics. The high-pressure-limit kinetics is used in PDep networks to improve them instead of using RMG's estimations. However, if the original library reaction already has PDep kinetics, then this library rate should end up in the model rather than a PDep estimation. Of course, if the library reaction only has Arrhenius (non-PDep) kinetics, and the reaction is pressure-dependent, the kinetics must be replaced with the PDep estimation.
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.
Regression Testing Results
Detailed regression test results.Regression test aromatics:Reference: Execution time (DD:HH:MM:SS): 00:00:01:23 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:02:48 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:
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:44 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(oxirene) + 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:02:48 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:08 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:43 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:21 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:08:14 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 🤖 |
I still question the utility of a changelog when we have the git history, but I've updated it anyway. And removed the description of things that were removed a few days ago.
I had a reaction library folder with another python file in it that had been used to renumber the reactions at some point. RMG was trying to load this as if it were a reactions.py, and failing the database tests. Since named reaction libraries are forced to have the file be called "reactions.py", I think it is OK that the "load all libraries" method makes the same assumption.
It wasn't showing up right in the actions log.
We want the compiled PDFs to end up in the documentation/ folder.
To replace this commit, if the documentation changes yet again, run `make latexpdf` in the documentation folder.
Regression Testing Results
Detailed regression test results.Regression test aromatics:Reference: Execution time (DD:HH:MM:SS): 00:00:01:24 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(124cyclohexatriene) 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:02:50 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:44 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:02:51 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:09 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:44 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:24 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:08:11 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 🤖 |
|
v3.2.0 was successfully merged, so closing this |
Motivation or Problem
Branch to eventually create new binaries
Description of Changes
Joint PR with RMG-database ReactionMechanismGenerator/RMG-database#590