Python 3.9 #2741
Python 3.9 #2741
Conversation
JacksonBurns
force-pushed
the
feat/py39_rebase
branch
from
76fde71 to
d81a2c8
Compare
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
JacksonBurns
force-pushed
the
feat/py39_rebase
branch
from
9d1377d to
0a13902
Compare
This comment was marked as resolved.
This comment was marked as resolved.
JacksonBurns
force-pushed
the
feat/py39_rebase
branch
from
4381db7 to
f89c483
Compare
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
JacksonBurns
force-pushed
the
feat/py39_rebase
branch
from
74932d8 to
73deea4
Compare
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as outdated.
This comment was marked as outdated.
Currently we use `RingDecomposerLib` for finding the Smallest Set of Smallest Rings and getting the Relevant Cycles. This package does not support Python 3.10+ and is thus blocking further upgrades to RMG. @KnathanM in particular is looking to get RMG to Python 3.11 so as to add support for ChemProp v2. I believe we can just use RDKit to do these operations instead. The original paper mentions that the functionality was being moved upstream to RDKit. With the help of AI I've taken just a first pass at reimplementing, with the special note that: - I opted to use the Symmetric SSSR in place of the 'true' SSSR. This is because the latter is non-unique (see [RDKit's "The SSSR Problem"](https://www.rdkit.org/docs/GettingStartedInPython.html#the-sssr-problem)). This should actually resolve #2562 - I need to read more about the "Relevant Cycles" This PR will be a draft for now, as it is predicated on Python 3.9 already being available (which it nearly is in #2741)
Currently we use `RingDecomposerLib` for finding the Smallest Set of Smallest Rings and getting the Relevant Cycles. This package does not support Python 3.10+ and is thus blocking further upgrades to RMG. @KnathanM in particular is looking to get RMG to Python 3.11 so as to add support for ChemProp v2. I believe we can just use RDKit to do these operations instead. The original paper mentions that the functionality was being moved upstream to RDKit. With the help of AI I've taken just a first pass at reimplementing, with the special note that: - I opted to use the Symmetric SSSR in place of the 'true' SSSR. This is because the latter is non-unique (see [RDKit's "The SSSR Problem"](https://www.rdkit.org/docs/GettingStartedInPython.html#the-sssr-problem)). This should actually resolve #2562 - I need to read more about the "Relevant Cycles" This PR will be a draft for now, as it is predicated on Python 3.9 already being available (which it nearly is in #2741)
Regression Testing Results
Detailed regression test results.Regression test aromatics:Reference: Execution time (DD:HH:MM:SS): 00:00:01:07 aromatics Passed Core Comparison ✅Original model has 15 species. aromatics Passed Edge Comparison ✅Original model has 106 species. aromatics Passed Observable Testing ✅Regression test liquid_oxidation:Reference: Execution time (DD:HH:MM:SS): 00:00:02:21 liquid_oxidation Passed Core Comparison ✅Original model has 37 species. liquid_oxidation Passed Edge Comparison ✅Original model has 214 species. liquid_oxidation Passed Observable Testing ✅Regression test nitrogen:Reference: Execution time (DD:HH:MM:SS): 00:00:01:26 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(oxirene) + radical(CdJ-NdO) Non-identical kinetics! ❌
kinetics: nitrogen Passed Observable Testing ✅Regression test oxidation:Reference: Execution time (DD:HH:MM:SS): 00:00:02:28 oxidation Passed Core Comparison ✅Original model has 59 species. oxidation Passed Edge Comparison ✅Original model has 230 species. oxidation Passed Observable Testing ✅Regression test sulfur:Reference: Execution time (DD:HH:MM:SS): 00:00:00:55 sulfur Passed Core Comparison ✅Original model has 27 species. sulfur Failed Edge Comparison ❌Original model has 89 species. sulfur Failed Observable Testing ❌Regression test superminimal:Reference: Execution time (DD:HH:MM:SS): 00:00:00:39 superminimal Passed Core Comparison ✅Original model has 13 species. superminimal Failed Edge Comparison ❌Original model has 18 species. Non-identical thermo! ❌
Identical thermo comments: Regression test RMS_constantVIdealGasReactor_superminimal:Reference: Execution time (DD:HH:MM:SS): 00:00:02:24 RMS_constantVIdealGasReactor_superminimal Passed Core Comparison ✅Original model has 13 species. RMS_constantVIdealGasReactor_superminimal Passed Edge Comparison ✅Original model has 13 species. RMS_constantVIdealGasReactor_superminimal Passed Observable Testing ✅Regression test RMS_CSTR_liquid_oxidation:Reference: Execution time (DD:HH:MM:SS): 00:00:06:11 RMS_CSTR_liquid_oxidation Failed Core Comparison ❌Original model has 37 species. RMS_CSTR_liquid_oxidation Failed Edge Comparison ❌Original model has 248 species. RMS_CSTR_liquid_oxidation Passed Observable Testing ✅Regression test fragment:Reference: Execution time (DD:HH:MM:SS): 00:00:00:41 fragment Passed Core Comparison ✅Original model has 10 species. fragment Passed Edge Comparison ✅Original model has 33 species. fragment Passed Observable Testing ✅Regression test RMS_constantVIdealGasReactor_fragment:Reference: Execution time (DD:HH:MM:SS): 00:00:03:04 RMS_constantVIdealGasReactor_fragment Passed Core Comparison ✅Original model has 10 species. RMS_constantVIdealGasReactor_fragment Passed Edge Comparison ✅Original model has 27 species. RMS_constantVIdealGasReactor_fragment Passed Observable Testing ✅Regression test minimal_surface:Reference: Execution time (DD:HH:MM:SS): 00:00:00:44 minimal_surface Passed Core Comparison ✅Original model has 11 species. minimal_surface Passed Edge Comparison ✅Original model has 38 species. minimal_surface Passed Observable Testing ✅Debugging info for Detected IPython. Loading juliacall extension. See https://juliapy.github.io/PythonCall.jl/stable/compat/#IPython (if any).WARNING:root:Julia import failed, RMS reactors not available. Exception: No module named 'juliacall' Stacktrace: 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! beep boop this comment was written by a bot 🤖 |
|
I think this is looking good. I made a couple of additional commits -hope they look ok to you. Is the |
Regression Testing Results
Detailed regression test results.Regression test aromatics:Reference: Execution time (DD:HH:MM:SS): 00:00:01:07 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: aromatics Passed Observable Testing ✅Regression test liquid_oxidation:Reference: Execution time (DD:HH:MM:SS): 00:00:02:23 liquid_oxidation Passed Core Comparison ✅Original model has 37 species. liquid_oxidation Failed Edge Comparison ❌Original model has 214 species. Non-identical kinetics! ❌
kinetics: liquid_oxidation Passed Observable Testing ✅Regression test nitrogen:Reference: Execution time (DD:HH:MM:SS): 00:00:01:26 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: nitrogen Passed Observable Testing ✅Regression test oxidation:Reference: Execution time (DD:HH:MM:SS): 00:00:02:29 oxidation Passed Core Comparison ✅Original model has 59 species. oxidation Passed Edge Comparison ✅Original model has 230 species. oxidation Passed Observable Testing ✅Regression test sulfur:Reference: Execution time (DD:HH:MM:SS): 00:00:00:55 sulfur Passed Core Comparison ✅Original model has 27 species. sulfur Failed Edge Comparison ❌Original model has 89 species. sulfur Failed Observable Testing ❌Regression test superminimal:Reference: Execution time (DD:HH:MM:SS): 00:00:00:40 superminimal Passed Core Comparison ✅Original model has 13 species. superminimal Failed Edge Comparison ❌Original model has 18 species. Non-identical thermo! ❌
Identical thermo comments: Regression test RMS_constantVIdealGasReactor_superminimal:Reference: Execution time (DD:HH:MM:SS): 00:00:02:24 RMS_constantVIdealGasReactor_superminimal Passed Core Comparison ✅Original model has 13 species. RMS_constantVIdealGasReactor_superminimal Passed Edge Comparison ✅Original model has 13 species. RMS_constantVIdealGasReactor_superminimal Passed Observable Testing ✅Regression test RMS_CSTR_liquid_oxidation:Reference: Execution time (DD:HH:MM:SS): 00:00:06:13 RMS_CSTR_liquid_oxidation Failed Core Comparison ❌Original model has 37 species. RMS_CSTR_liquid_oxidation Failed Edge Comparison ❌Original model has 248 species. RMS_CSTR_liquid_oxidation Passed Observable Testing ✅Regression test fragment:Reference: Execution time (DD:HH:MM:SS): 00:00:00:42 fragment Passed Core Comparison ✅Original model has 10 species. fragment Passed Edge Comparison ✅Original model has 33 species. fragment Passed Observable Testing ✅Regression test RMS_constantVIdealGasReactor_fragment:Reference: Execution time (DD:HH:MM:SS): 00:00:03:08 RMS_constantVIdealGasReactor_fragment Passed Core Comparison ✅Original model has 10 species. RMS_constantVIdealGasReactor_fragment Passed Edge Comparison ✅Original model has 27 species. RMS_constantVIdealGasReactor_fragment Passed Observable Testing ✅Regression test minimal_surface:Reference: Execution time (DD:HH:MM:SS): 00:00:00:45 minimal_surface Passed Core Comparison ✅Original model has 11 species. minimal_surface Passed Edge Comparison ✅Original model has 38 species. minimal_surface Passed Observable Testing ✅Debugging info for Detected IPython. Loading juliacall extension. See https://juliapy.github.io/PythonCall.jl/stable/compat/#IPython (if any).WARNING:root:Julia import failed, RMS reactors not available. Exception: No module named 'juliacall' Stacktrace: 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! beep boop this comment was written by a bot 🤖 |
|
@rwest those look good - let me know about the RMS PR I think the theory behind using the global julia packages dir is that one can then access RMS without being inside the RMG conda envionrment |
return 1 exits the current function or sourced script with a non-zero exit status (indicating failure), but it does NOT close the shell. Here's the key difference: exit 1: Terminates the entire shell process (would close the user's terminal) return 1: Only exits the current script/function and returns control to the calling shell When someone runs source install_rms.sh, the script is executed in the current shell context. Using return 1 means: The script stops executing at that point Control returns to the user's shell prompt The shell remains open and usable The return code 1 indicates the script failed (which the user can check with echo $?) This is exactly what you want for a sourced script - it allows the script to abort gracefully without killing the user's shell session.
This way it ends up in a julia_env subdirectory of the active conda environment.
1.10 would have matched 1.11.10, were it ever to exist
Users need to run install_rms.sh then follow its instructions then run it again. Also made a one-liner test that RMS is installed (though carefully avoid the many minutes of pre-compiling)
| # Read the version number | ||
| exec(open('rmgpy/version.py').read()) | ||
|
|
||
| # Initiate the build and/or installation | ||
| setup( | ||
| name='RMG-Py', | ||
| name='reactionmechanismgenerator', |
There was a problem hiding this comment.
Is this renaming a significant (possibly breaking) change?
what are the consequences and what did it enable?
There was a problem hiding this comment.
This actually does not impact usage of RMG - crucial detail here is the RMG-Py package versus the rmgpy module.
During the previous RMG installation process, we actually never ran pip install. This means that RMG was built 'in place', requiring us to set the PATH to points toward the repository in order to import the rmgpy module. Basically, no package was installed.
Now the build actually runs pip install which makes the rmgpy module importable from anywhere (like, in our jupyter notebooks, for example). This is why the PATH is no longer required to be set, since RMG is treated as a package.
This new process also makes reactionmechanismgenerator show as an installed package if you run pip list. This exact renaming was done to conform to Python Package Index naming conventions (no dashes, preferably) and to make the name unambiguous.
Regression Testing Results
Detailed regression test results.Regression test aromatics:Reference: Execution time (DD:HH:MM:SS): 00:00:01:07 aromatics Passed Core Comparison ✅Original model has 15 species. aromatics Failed Edge Comparison ❌Original model has 106 species. Non-identical thermo! ❌
Identical thermo comments: aromatics Passed Observable Testing ✅Regression test liquid_oxidation:Reference: Execution time (DD:HH:MM:SS): 00:00:02:23 liquid_oxidation Passed Core Comparison ✅Original model has 37 species. liquid_oxidation Failed Edge Comparison ❌Original model has 214 species. liquid_oxidation Passed Observable Testing ✅Regression test nitrogen:Reference: Execution time (DD:HH:MM:SS): 00:00:01:26 nitrogen Failed 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: nitrogen Passed Observable Testing ✅Regression test oxidation:Reference: Execution time (DD:HH:MM:SS): 00:00:02:29 oxidation Passed Core Comparison ✅Original model has 59 species. oxidation Passed Edge Comparison ✅Original model has 230 species. oxidation Passed Observable Testing ✅Regression test sulfur:Reference: Execution time (DD:HH:MM:SS): 00:00:00:55 sulfur Passed Core Comparison ✅Original model has 27 species. sulfur Failed Edge Comparison ❌Original model has 89 species. sulfur Failed Observable Testing ❌Regression test superminimal:Reference: Execution time (DD:HH:MM:SS): 00:00:00:40 superminimal Passed Core Comparison ✅Original model has 13 species. superminimal Failed Edge Comparison ❌Original model has 18 species. Non-identical thermo! ❌
Identical thermo comments: Regression test RMS_constantVIdealGasReactor_superminimal:Reference: Execution time (DD:HH:MM:SS): 00:00:02:24 RMS_constantVIdealGasReactor_superminimal Passed Core Comparison ✅Original model has 13 species. RMS_constantVIdealGasReactor_superminimal Passed Edge Comparison ✅Original model has 13 species. RMS_constantVIdealGasReactor_superminimal Passed Observable Testing ✅Regression test RMS_CSTR_liquid_oxidation:Reference: Execution time (DD:HH:MM:SS): 00:00:06:13 RMS_CSTR_liquid_oxidation Failed Core Comparison ❌Original model has 37 species. RMS_CSTR_liquid_oxidation Failed Edge Comparison ❌Original model has 248 species. RMS_CSTR_liquid_oxidation Passed Observable Testing ✅Regression test fragment:Reference: Execution time (DD:HH:MM:SS): 00:00:00:42 fragment Passed Core Comparison ✅Original model has 10 species. fragment Passed Edge Comparison ✅Original model has 33 species. fragment Passed Observable Testing ✅Regression test RMS_constantVIdealGasReactor_fragment:Reference: Execution time (DD:HH:MM:SS): 00:00:03:08 RMS_constantVIdealGasReactor_fragment Passed Core Comparison ✅Original model has 10 species. RMS_constantVIdealGasReactor_fragment Passed Edge Comparison ✅Original model has 27 species. RMS_constantVIdealGasReactor_fragment Passed Observable Testing ✅Regression test minimal_surface:Reference: Execution time (DD:HH:MM:SS): 00:00:00:45 minimal_surface Passed Core Comparison ✅Original model has 11 species. minimal_surface Passed Edge Comparison ✅Original model has 38 species. minimal_surface Passed Observable Testing ✅Debugging info for Detected IPython. Loading juliacall extension. See https://juliapy.github.io/PythonCall.jl/stable/compat/#IPython (if any).WARNING:root:Julia import failed, RMS reactors not available. Exception: No module named 'juliacall' Stacktrace: 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! beep boop this comment was written by a bot 🤖 |
Regression Testing Results
Detailed regression test results.Regression test aromatics:Reference: Execution time (DD:HH:MM:SS): 00:00:01:07 aromatics Passed Core Comparison ✅Original model has 15 species. aromatics Passed Edge Comparison ✅Original model has 106 species. aromatics Passed Observable Testing ✅Regression test liquid_oxidation:Reference: Execution time (DD:HH:MM:SS): 00:00:02:21 liquid_oxidation Passed Core Comparison ✅Original model has 37 species. liquid_oxidation Failed Edge Comparison ❌Original model has 214 species. liquid_oxidation Passed Observable Testing ✅Regression test nitrogen:Reference: Execution time (DD:HH:MM:SS): 00:00:01:27 nitrogen Passed Core Comparison ✅Original model has 41 species. nitrogen Passed Edge Comparison ✅Original model has 133 species. nitrogen Passed Observable Testing ✅Regression test oxidation:Reference: Execution time (DD:HH:MM:SS): 00:00:02:30 oxidation Passed Core Comparison ✅Original model has 59 species. oxidation Passed Edge Comparison ✅Original model has 230 species. oxidation Passed Observable Testing ✅Regression test sulfur:Reference: Execution time (DD:HH:MM:SS): 00:00:00:56 sulfur Passed Core Comparison ✅Original model has 27 species. sulfur Failed Edge Comparison ❌Original model has 89 species. sulfur Failed Observable Testing ❌Regression test superminimal:Reference: Execution time (DD:HH:MM:SS): 00:00:00:39 superminimal Passed Core Comparison ✅Original model has 13 species. superminimal Failed Edge Comparison ❌Original model has 18 species. Non-identical thermo! ❌
Identical thermo comments: Regression test RMS_constantVIdealGasReactor_superminimal:Reference: Execution time (DD:HH:MM:SS): 00:00:02:25 RMS_constantVIdealGasReactor_superminimal Passed Core Comparison ✅Original model has 13 species. RMS_constantVIdealGasReactor_superminimal Passed Edge Comparison ✅Original model has 13 species. RMS_constantVIdealGasReactor_superminimal Passed Observable Testing ✅Regression test RMS_CSTR_liquid_oxidation:Reference: Execution time (DD:HH:MM:SS): 00:00:06:14 RMS_CSTR_liquid_oxidation Failed Core Comparison ❌Original model has 37 species. RMS_CSTR_liquid_oxidation Failed Edge Comparison ❌Original model has 248 species. RMS_CSTR_liquid_oxidation Passed Observable Testing ✅Regression test fragment:Reference: Execution time (DD:HH:MM:SS): 00:00:00:42 fragment Passed Core Comparison ✅Original model has 10 species. fragment Passed Edge Comparison ✅Original model has 33 species. fragment Passed Observable Testing ✅Regression test RMS_constantVIdealGasReactor_fragment:Reference: Execution time (DD:HH:MM:SS): 00:00:03:08 RMS_constantVIdealGasReactor_fragment Passed Core Comparison ✅Original model has 10 species. RMS_constantVIdealGasReactor_fragment Passed Edge Comparison ✅Original model has 27 species. RMS_constantVIdealGasReactor_fragment Passed Observable Testing ✅Regression test minimal_surface:Reference: Execution time (DD:HH:MM:SS): 00:00:00:45 minimal_surface Passed Core Comparison ✅Original model has 11 species. minimal_surface Passed Edge Comparison ✅Original model has 38 species. minimal_surface Passed Observable Testing ✅Debugging info for Detected IPython. Loading juliacall extension. See https://juliapy.github.io/PythonCall.jl/stable/compat/#IPython (if any).WARNING:root:Julia import failed, RMS reactors not available. Exception: No module named 'juliacall' Stacktrace: 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! beep boop this comment was written by a bot 🤖 |
Upgrading RMG-Py to Python 3.9
The current version of Python used by RMG (3.7) is deprecated, so we are missing out on the latest updates to our various dependencies. This PR upgrade RMG to use Python 3.9 by doing the following:
lpsolveis removed and replaced withscipy's Linear Programming Solver (credit @xiaoruiDong and @JacksonBurns) becauselpsolvedoesn't support 3.9+ (no builds exist, and we can't get our build to work). Ported from Reimplement Clar Optimization with Scipy MILP #2623juliaoverhaul, see below.Getting this merged
One feature that remains to be implemented and will have to be figured out in a separate PR into this one is the new
juliaup+juliacallsetup implemented by @hwpang. It will be much better, but there are currently some bugs preventing it from working (at all), primarily related to the installedjuliaexecutable being able to 'see' the Python environment and its installed dependencies.After that, this should be good to merge. RMG-Py will be appreciably faster, easier to extend, and less buggy.