-
Notifications
You must be signed in to change notification settings - Fork 242
Description
Important
Please read and comment on this issue if you are a developer or user of RMG - we need lots of input!
The purpose of this issue is to centralize discussion and hopefully reach a compromise surrounding a topic which has been discussed throughout many issues, pull requests, and offline meetings - whether ReactionMechanismSimulator.jl
and its associated Julia dependencies should be required to run RMG.
This issue is styled after the Python Enhancement Proposal (see PEP 733 for an example), thus the name 'RMG Improvement Proposal', or RIP for short. This RIP will attempt to:
- provide a brief summary of all of the below points
- introduce the perceived issue
- discuss the drawbacks of the proposed change
- discuss the benefits of the proposed change
- discuss the drawbacks of the proposed change
Summary
- RMS and its associated Julia dependencies are currently required to install and run RMG
- This requirements has created installation issues and runtime issues for both users and developers
- A pull request is in place which would make RMS optional and resolve these
- There are drawbacks to this approach and a suggested alternative which would merely avoid some Julia operations but not allow RMS to be optional
Introduction
As of writing, RMG-Py currently requires RMS and its associated dependencies to be installed in order to run anything in RMG. This is because (a) RMG can optionally interface with RMS to use its reactors and (b) Arkane optionally uses Julia's differential equations solver.
Reasons to Require RMS
I'm attempting to summarize comments from #2631 (especially here) and elsewhere. Please leave comments replying to any of these points, or add your own, if you think I missed or misrepresented anything.
The 'Batteries Included' Ideology
In general Python adheres to a "Batteries Included" thinking - everything that you could possibly need should come included without additional installation. By including RMS with RMG by requirement, we are avoiding the questions of "what features are available if I add RMS?" and the like.
Counterpoint
We can continue to ship the Docker container with 'batteries included' (all dependencies) but allow for RMS-less source or binary installs. The CI will also continue to test RMS and future PRs will require RMS tests to pass, so there is no risk of incompatible code being merged.
Promised Features in Papers are only in RMS
Some recent papers coming out of our groups describe features which are only available via RMS. It makes sense that the default installation of RMG should therefore provide these features. It's also an additional layer of 'protection' for these features to make them required by default - by making them required and continuing to interact with them, we stand a better chance of them continuing to work into the future.
Simplifying Debugging by Forcing Everyone to Install the Same Way
Offering multiple installation setups can lead to more complicated bug reports. If users install with our without Julia they may end up running a different set of unit tests, for example. This is a similar line of thinking to why we moved to Docker in the first place - unify the user experience.
Reasons to Make RMS Optional
Resolves Outstanding Issues
There are issues scattered across the @ReactionMechanismGenerator organization which are related to these dependencies that would be resolved by making them optional:
- Missing Riedel Equation Parameters for Methanol RMG-database#634 (comment) - user reported that missing reidel equation parameters would crash RMG because RMS required them even though RMS was not being used
- Arkane takes a long time to run due to loading Julia dependencies #2547 - loading Julia dependencies takes minutes to then run a few-second Arkane job that does not use Julia
- Reinstalled from source and it is very slow. #2617 - installing Julia dependencies required "scary" manual steps beyond what we want for our users and RMG was also slower than previously
Counterpoint
These issues would also be resolved by merging part of #2631 - only the "avoid unnecessary Julia operations".
Enables Conda Binary Building
Currently, our conda binaries are significantly (~3 years) out of date because we have not been able to successfully build the rmg
package since the integration of RMS. Users must either do a source install or use the Docker image.
This would be resolved by #2641 already - by making a pure-python RMG package and then having users install RMS on top of it, the conda build for RMG is massively simplified and shown to work.
The alternative w/o making RMS optional in #2636 was not successful.
Health of the RMG Software
As far back as 2021 in this discussion (#2247) we have had requests to make installing the Julia dependencies optional, citing that the features it provides, when unused, amount to bloat.
Along the same lines of thinking, extending RMG is prohibitively difficult because of the huge number of dependencies besides RMS - taking chances to make dependencies optional will help alleviate this.
This is opinionated, but the separation between RMG and RMS is clean, amounting to ~300 lines of code in https://github.com/ReactionMechanismGenerator/RMG-Py/pull/2631/files. To quote PEP 20: "Simple is better than complex", and separating these two halves is simple.
Highly Opinionated Point - Julia can be a bit of a headache
Julia is very new and consequently suffers a lot of ecosystem problems - a selection of some reported here:
- Julia restriction #2609 - we are currently limited to a specific patch of Julia because others do not work
- Revert
libstdcxx-ng
Patches in CI, Docs, and Dockerfile #2469 - a previous specific patch release of Julia which also did not work - Continuous Integration, Documentation Build, and Dockerfile Fixes #2608 - Dockerfile would not build on Windows without a special Julia environment variable being thrown in
- CI for Mac frequently timing out #2453 - we can't run our CI on MacOS because the Julia installation times out for no obvious reason (almost fixed in Run Complete CI on MacOS and Ubuntu #2648)
Developing RMG with Julia suffers from problems as well:
- typo prevents writing input files #2531 - attempting to debug a problem takes extra minutes on every run because of loading Julia dependencies
- Conversion from nose to pytest #2516 (comment) - you cannot run the RMG tests in parallel because of Julia
- There are no conda binaries for Julia for the latest Apple CPUs (M1+) requiring users to instead use a compatibility layer and run the x86 code: e0e2601#diff-155d3a2dcb81a38f68cd8cda2bcebe6368b9c0677c0513541e1140dfcebd7fd7R76-R79
Acknowledgements
Thanks to @rwest for the suggestion to formalize this discussion into this format.
cc
I'd specifically like to make sure that the following people see this issue and have an opportunity to respond: @mjohnson541 @hwpang @alongd @calvinp0 @rwest @sevyharris @oscarwumit @jonwzheng @xiaoruiDong @ChrisBNEU - please see the Summary above, consider reading the full issue, and let us know your thoughts!