-
Notifications
You must be signed in to change notification settings - Fork 244
RMG-Py Version 3.3.0 stable
Update
#2827
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
Conversation
remove deprecated check for nose test
specify electronchange in RMS yaml construction
Now it's done in RMS Added solvent correction for SurfaceChargeTransfer
PCET families have to be skipped since the SurfaceChargeTransfer and SurfaceChargeTransferBEP classes have no attribute "coverage_dependence"
kineticsSurfaceTest used atol for rate coefficient tests previously. The atol was too strict for double precision floating point. All other rate coefficient tests used rtol.
The `Fragment` class is now a subclass of `Molecule`. As such, some if statements for checking atom charges no longer work for Fragments since CuttingLabels don't have defined charges. These if statements were modified such that charge checks for `Fragments` ignore CuttingLabels.
For: >> Continuous Integration >> Documentations for compiling from source >> Building the Docker image *The marcus_development branch includes all the commits necessary to make electrochemistry work but doesn't include changes for the RMG-RMS interface overhaul. Will be reverted once that gets merged into RMS main.
This is needed for testing. Once lithium_rebase is merged to main on the database, we can go back to using main.
I presume this method was reintroduced by mistake during a rebase. It was removed in f695b05
I couldn't help myself, while doing code review.
- conda recipe itself is overhauled, updating the dependency lists and build steps - github action for building the conda recipe is also overhauled, building a binary on all platforms with future flexibility for other Python and numpy versions - some internal code changes to facilitate the above points, namely moving the driver code into `rmgpy/__main__.py` and leaving a stripped-down version of `rmg.py` in the root directory for backwards compatibility (same for Arkane.py)
as reminded here: #2820 (comment)
Make RMG-Py `conda` Work Again 🎉
Thanks for coordinating this, Jackson! #2815 is done except for the few RMS notebooks. If someone familiar with the new Julia/RMS syntax could please help take a look, that'd be of tremendous help. #2824 is similarly basically done except for some Julia-based/simulation errors. Here again, if someone familiar with the Julia code could try and troubleshoot, I'd be very grateful. |
Regression Testing Results
Detailed regression test results.Regression test aromatics:Reference: Execution time (DD:HH:MM:SS): 00:00:00:51 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(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:01:58 liquid_oxidation Passed Core Comparison ✅Original model has 37 species. liquid_oxidation Failed Edge Comparison ❌Original model has 214 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:00:59 nitrogen Passed Core Comparison ✅Original model has 41 species. nitrogen Failed Edge Comparison ❌Original model has 133 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:01:32 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 ✅Errors occurred during observable testing
WARNING:root:Initial mole fractions do not sum to one; normalizing.
|
@jonwzheng and I discussed offline about the exaples and notebook errors. I think it is best to go ahead and release 3.3.0 now while we have the momentum. The core codebase of RMG-Py works, we just need some small updates to the notebooks (which will soon be done and reflected in |
RMG-Py Version 3.3.0
@rwest I believe this is ready. Same deal as the rmg data set 3.3.0 PR - all of these changes have been reviewed, this is just an approval to release the new version and push conda binaries. |
Hi @JacksonBurns , some of our group members have noticed a compatibility problem when installing RMS using the provided script. A fix has been proposed in #2828. We shall have some test results by tomorrow. |
Regression Testing Results
Detailed regression test results.Regression test aromatics:Reference: Execution time (DD:HH:MM:SS): 00:00:00:51 aromatics Passed Core Comparison ✅Original model has 15 species. aromatics Failed Edge Comparison ❌Original model has 106 species. Non-identical thermo! ❌
Identical thermo comments:
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:02 liquid_oxidation Passed Core Comparison ✅Original model has 37 species. liquid_oxidation Failed Edge Comparison ❌Original model has 214 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:00:59 nitrogen Passed Core Comparison ✅Original model has 41 species. nitrogen Failed Edge Comparison ❌Original model has 133 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:01:35 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 ✅Errors occurred during observable testing
WARNING:root:Initial mole fractions do not sum to one; normalizing.
|
@ssun30 I see the goal of the linked PR. I think for our general users the |
My view: If this only affects developer installation (eg. building from source) then it can go in later, because they'll be building from current main. I think there may be improvements possible to |
@rwest sounds good - if you leave an approval I'll go ahead and merge 🚀 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
After merging main to stable we should also merge stable to main and tag it.
Yes, I'm okay with that. |
This PR updates the stable branch, to be merged once #2826 is merged into
main
. Doing so constitutes the official release of RMG-Py Version 3.3.0 🎉Merging this PR will trigger the
conda
binaries for Version 3.3.0 to be built and uploaded. The corresponding 3.3.0 docker image will then be uploaded by Me (@JacksonBurns).