-
Notifications
You must be signed in to change notification settings - Fork 245
Add environment validation and error handling to RMS installation script #2828
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
base: main
Are you sure you want to change the base?
Conversation
Can you edit the docker file so that it will still build with this new script? I think you will just need to set the environment variable you added inside the dockerfile |
Also, the reason we didn't set Current is that Julia would break the environment when allowed to install its own packages into rmg_env. The exact reason is buried somewhere in the comments of the Python 3.9 upgrade, but @rwest might recall. My first thought is that it broke sundials, but I can run through that PR tomorrow and check. |
Reading
We could just set the path right and install the required packages? |
This is what we currently do, though we let |
The main problem is when installing RMS, the correct Julia environment was not activated, even though we set the |
I'm not against adding something to RMG to make co-developing RMG and RMS easier, but I'm not sure it needs to be an automated script like this. I believe a better fit would be some documentation for developers pointing out how to follow the same 'spirit' as the IMO an automated script to handle a complex developer install doesn't make a lot of sense. Someone looking to do something this complicated should be able to run the commands on their own, especially with some help from a nice docs page. |
Some of these additions I'm definitely a fan of. I like checking things more thoroughly too. As a middle ground the "dev" install could perhaps be triggered just by setting environment variables before running the install_rms script rather than it asking everybody what type of install they want? And yes we need better documentation. 👍 |
I have made "standard" mode default so it won't be asking the user which mode to use. Now the "developer" mode will only be used if the user use the command |
efb971c
to
3cc8470
Compare
Regression Testing Results
Detailed regression test results.Regression test aromatics:Reference: Execution time (DD:HH:MM:SS): 00:00:00:50 aromatics Passed Core Comparison ✅Original model has 15 species. aromatics Passed Edge Comparison ✅Original model has 106 species.
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:01 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:01:04 nitrogen Passed Core Comparison ✅Original model has 41 species. nitrogen Passed Edge Comparison ✅Original model has 133 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:01: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 ✅Errors occurred during observable testing
WARNING:root:Initial mole fractions do not sum to one; normalizing.
|
Added developer install option for install_rms.sh
Initialize Julia environment before installing RMS Add extra debug info
Developer mode will only be enabled if the user specify "developer" option for install_rms.sh
Added developer install option for install_rms.sh
Motivation or Problem
Some of our group members have noticed that they have a Python compatibility problem when installing RMS. It seems that their default system Python version is 3.12 or newer. I've identified the problem to be the
JULIA_CONDAPKG_BACKEND
environment variable, which is set to "Null", meaning it's using whichever Python is currently installed and in the user's PATH, and the user must have already installed any Python packages that they need.The intended behavior is for
CondaPkg
to use the currently activated Conda environment (say, rmg_env) and use the Python binaries within that environment, so it ensures compatibility. For that, the correct environment variable value should be "Current". Furthermore, theJULIA_CONDAPKG_EXE
variable needs to be specified (to $which conda).I'm not sure why our CI didn't catch this problem (seems to be that only Python 3.9 is installed in any of our test systems and conda environment), but at least a fix is found and some testing has been done on my local computer. I'll need some more testing results from other users later.
Description of Changes
CondaPkg
configurations to use the correct backend.install_rms.sh
to use a developer build of RMS.