Skip to content
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

Fix Python 3.9 julia Overhaul #2749

Closed
wants to merge 51 commits into from

Conversation

JacksonBurns
Copy link
Contributor

The Python 3.9 Upgrade is complete, but the new juliacall setup does not work. Current theory is that julia cannot 'see' the Python environment and/or its dependencies. Will address here.

@JacksonBurns
Copy link
Contributor Author

The pure-python tests fail on MacOS because between Python 3.7 and 3.9 the way multiprocessing works was changed (see https://stackoverflow.com/a/72830812) - we violate the requirements for using this new method (see https://docs.python.org/3/library/multiprocessing.html#the-spawn-and-forkserver-start-methods) so we may just need to set the method back to the old way.

@JacksonBurns
Copy link
Contributor Author

Using PythonCall with an existing conda environment: https://discourse.julialang.org/t/how-to-use-pythoncall-with-a-previous-conda-environment/87419

the PYTHONCALL suggestion is here: https://discourse.julialang.org/t/how-to-use-pythoncall-with-a-previous-conda-environment/87419/8

changing to current will hopefully prevent condapkg from making a new environment
@JacksonBurns
Copy link
Contributor Author

Setting JULIA_CONDAPKG_BACKEND to System caused RMS to install new conda packages during its build, since it could not 'see' the rmg_env conda environment. This resulted in the newly installed Python not being on the path, and then failing to find its encodings module during import (and thus killing the CI run).

Now attempting to set JULIA_CONDAPKG_BACKEND to Current, which should make RMS 'see' the rmg_env.

@JacksonBurns
Copy link
Contributor Author

Julia sees the correct environment (I think), so next is solving the error that Julia can't see the libraries. I think this may fix it: https://stackoverflow.com/a/77206459

@JacksonBurns
Copy link
Contributor Author

PYTHONPATH is likely set incorrectly, see: pythonnet/pythonnet#2147

@JacksonBurns
Copy link
Contributor Author

@JacksonBurns
Copy link
Contributor Author

Another person with the same issue; no solution - https://discourse.julialang.org/t/pycall-installation-on-macos-m1/87980

@JacksonBurns
Copy link
Contributor Author

More related: JuliaPy/PyCall.jl#410 (comment)

…se; and...

check that PYTHONHOME and PYTHONPATH are unset, both of which have a high likelihood of messing up the install process
@JacksonBurns
Copy link
Contributor Author

Next debugging step is to set JULIA_PYTHONCALL_EXE to the python executable rather than setting PYTHON: https://juliapy.github.io/PythonCall.jl/stable/pythoncall/#If-you-already-have-Python-and-required-Python-packages-installed

@JacksonBurns
Copy link
Contributor Author

ARM-mac can't see encodings, Ubuntu and Intel Mac can't see rmgpy...

@JacksonBurns
Copy link
Contributor Author

Seems that for Ubuntu the install works, but then during tests Python can't find it's libs - likely candidate is Python home being set incorrectly (i.e. at all)

@JacksonBurns
Copy link
Contributor Author

The RMG modules are built in place, which is why we need to set PYTHONPATH: https://stackoverflow.com/a/7851585

Maybe we also need to set the system path to point toward the libs in the conda environment?

@JacksonBurns
Copy link
Contributor Author

Intel Mac now fails only the tests that are due to the change in fork - why can't ubuntu see the libs? Going to set the library path for ld

@JacksonBurns
Copy link
Contributor Author

well, at least the CI is passing on all platforms in pure-python mode now...

@JacksonBurns
Copy link
Contributor Author

@jonwzheng where does this stand now?

@jonwzheng
Copy link
Contributor

I've tried updating LD_LIBRARY_PATH in the environment but nothing seemed to change. May need to check/confirm that the path was the correct one

@JacksonBurns
Copy link
Contributor Author

Ok, thanks for giving that a shot. I''ve got a new idea - rather than trying to get Julia to 'see' the packages we want and to not change the packages we already have, we can just disallow it from doing the latter entirely: https://juliapy.github.io/PythonCall.jl/stable/pythoncall/#If-you-already-have-Python-and-required-Python-packages-installed

In theory, if we install all of the packages that RMS would need manually, then Julia won't attempt to change them, and thus hopefully won't break the libstdc++ lib

@JacksonBurns
Copy link
Contributor Author

Possibly need to change RMS, getting this error: ERROR: LoadError: Can not get envdir when backend is Null.

JacksonBurns added a commit to hwpang/ReactionMechanismSimulator.jl that referenced this pull request Jan 22, 2025
this causes an error when the conda backend is set to Null (i.e. julia is disallowed from installing conda packages), see: ReactionMechanismGenerator/RMG-Py#2749 (comment)
@JacksonBurns
Copy link
Contributor Author

a84641c introduces changes to RMS that will stop the current issue, at least for now. If RMS attempts to install things still, it might go awry.

Cancelling and re-starting CI now

@JacksonBurns
Copy link
Contributor Author

Miracle of miracles, that actually worked...

@JacksonBurns
Copy link
Contributor Author

Now addressing the still-failing Apple Silicon in this PR: #2751

@JacksonBurns
Copy link
Contributor Author

JacksonBurns commented Jan 22, 2025

All of the RMS-calling regression tests are failing with this error:

2025-01-22T18:45:24.9932449Z Traceback (most recent call last):
2025-01-22T18:45:24.9933256Z   File "/home/runner/work/RMG-Py/RMG-Py/rmg.py", line 118, in <module>
2025-01-22T18:45:24.9933990Z     main()
2025-01-22T18:45:24.9934567Z   File "/home/runner/work/RMG-Py/RMG-Py/rmg.py", line 112, in main
2025-01-22T18:45:24.9935297Z     rmg.execute(**kwargs)
2025-01-22T18:45:24.9936075Z   File "/home/runner/work/RMG-Py/RMG-Py/rmgpy/rmg/main.py", line 799, in execute
2025-01-22T18:45:24.9936758Z     requires_rms = self.initialize(**kwargs)
2025-01-22T18:45:24.9937319Z   File "/home/runner/work/RMG-Py/RMG-Py/rmgpy/rmg/main.py", line 513, in initialize
2025-01-22T18:45:24.9937865Z     self.load_input(self.input_file)
2025-01-22T18:45:24.9938392Z   File "/home/runner/work/RMG-Py/RMG-Py/rmgpy/rmg/main.py", line 266, in load_input
2025-01-22T18:45:24.9938932Z     read_input_file(path, self)
2025-01-22T18:45:24.9939810Z   File "/home/runner/work/RMG-Py/RMG-Py/rmgpy/rmg/input.py", line 1593, in read_input_file
2025-01-22T18:45:24.9940708Z     exec(f.read(), global_context, local_context)
2025-01-22T18:45:24.9941110Z   File "<string>", line 32, in <module>
2025-01-22T18:45:24.9941732Z   File "/home/runner/work/RMG-Py/RMG-Py/rmgpy/rmg/input.py", line 912, in constant_T_V_liquid_reactor
2025-01-22T18:45:24.9942528Z     system = ConstantTVLiquidReactor(rmg.reaction_model.core.phase_system,
2025-01-22T18:45:24.9943411Z   File "/home/runner/work/RMG-Py/RMG-Py/rmgpy/rmg/reactionmechanismsimulator_reactors.py", line 585, in __init__
2025-01-22T18:45:24.9944272Z     super().__init__(core_phase_system, edge_phase_system, initial_conditions, terminations, constant_species=constant_species)
2025-01-22T18:45:24.9945089Z   File "/home/runner/work/RMG-Py/RMG-Py/rmgpy/rmg/reactionmechanismsimulator_reactors.py", line 427, in __init__
2025-01-22T18:45:24.9945697Z     self.terminations = [to_rms(term) for term in terminations]
2025-01-22T18:45:24.9946456Z   File "/home/runner/work/RMG-Py/RMG-Py/rmgpy/rmg/reactionmechanismsimulator_reactors.py", line 427, in <listcomp>
2025-01-22T18:45:24.9947077Z     self.terminations = [to_rms(term) for term in terminations]
2025-01-22T18:45:24.9947669Z   File "/home/runner/work/RMG-Py/RMG-Py/rmgpy/rmg/reactionmechanismsimulator_reactors.py", line 845, in to_rms
2025-01-22T18:45:24.9948300Z     return Main.TerminationTime(obj.time.value_si)
2025-01-22T18:45:24.9948803Z   File "/home/runner/.julia/packages/PythonCall/Nr75f/src/JlWrap/any.jl", line 245, in __getattr__
2025-01-22T18:45:24.9949481Z     return self._jl_callmethod($(pyjl_methodnum(pyjlany_getattr)), k)
2025-01-22T18:45:24.9949970Z AttributeError: Julia: UndefVarError: `TerminationTime` not defined in `Main`

The Julia imports are failing, have now added more detailed failure reporting.

@JacksonBurns
Copy link
Contributor Author

Ah, right at the start of the tests this happens:

2025-01-22T19:32:05.1095495Z collecting ... [juliapkg] Found dependencies: /home/runner/miniconda3/envs/rmg_env/lib/python3.9/site-packages/juliacall/juliapkg.json
2025-01-22T19:32:05.1096946Z [juliapkg] Found dependencies: /home/runner/miniconda3/envs/rmg_env/lib/python3.9/site-packages/juliapkg/juliapkg.json
2025-01-22T19:32:05.1097902Z [juliapkg] Locating Julia ~1.6.1, ~1.7, ~1.8, ~1.9, =1.10.0, ^1.10.3
2025-01-22T19:32:05.1098824Z [juliapkg] Installing Julia 1.11.3 using JuliaUp
2025-01-22T19:32:05.1099756Z [juliapkg] Using Julia 1.11.3 at /home/runner/.julia/juliaup/julia-1.11.3+0.x64.linux.gnu/bin/julia
2025-01-22T19:32:05.1100538Z [juliapkg] Using Julia project at /home/runner/miniconda3/envs/rmg_env/julia_env
2025-01-22T19:32:05.1101074Z [juliapkg] Installing packages:
2025-01-22T19:32:05.1101417Z            julia> import Pkg
2025-01-22T19:32:05.1101749Z            julia> Pkg.Registry.update()
2025-01-22T19:32:05.1102577Z            julia> Pkg.add([Pkg.PackageSpec(name="PythonCall", uuid="6099a3de-0909-46bc-b1f4-468b9a2dfc0d")])
2025-01-22T19:32:05.1103188Z            julia> Pkg.resolve()
2025-01-22T19:32:05.1103512Z            julia> Pkg.precompile()
2025-01-22T19:32:05.1104234Z Detected IPython. Loading juliacall extension. See https://juliapy.github.io/PythonCall.jl/stable/compat/#IPython

juliacall can't see the version of Julia we went to all of the trouble of preparing

@JacksonBurns
Copy link
Contributor Author

How to configure the above to hopefully not happen: https://github.com/JuliaPy/PyJuliaPkg?tab=readme-ov-file#details

@JacksonBurns
Copy link
Contributor Author

When attempting to run this locally I have been running into segfaults during the RMS install. Turns out they were actually just on the exit of the install command. Some reading suggests that this is expected or at least OK: https://discourse.julialang.org/t/my-julia-code-has-segfault-only-on-exit-is-there-a-way-to-simply-stop-julia-without-calling-into-this-finalization-stuff/106538

Adding a step to the install script that will continue the install even if the command actually raises an error...

@JacksonBurns
Copy link
Contributor Author

Regression tests now run locally - fail with some errors related to the changes made to make RMS optional (phases is None would it should be an RMS object). Just need to debug that now, going to let CI list out all the errors.

@JacksonBurns
Copy link
Contributor Author

How to configure the above to hopefully not happen: https://github.com/JuliaPy/PyJuliaPkg?tab=readme-ov-file#details

May need to also set PYTHON_JULIAPKG_PROJECT to ensure we see the installed RMS package

@JacksonBurns
Copy link
Contributor Author

... or just let juliacall install everything?

@JacksonBurns
Copy link
Contributor Author

May need to also set PYTHON_JULIAPKG_PROJECT to ensure we see the installed RMS package

cc2c9ba tries this

Note that we will want to, at some point, run the regression tests on only one platform (right now, if this code were all working, we would be running them on all three platforms).

@JacksonBurns
Copy link
Contributor Author

The environment variables needed to get the RMS install to work are not persisting outside of the install_rms.sh script, so I have added setting them to the actual CI step. They might be best moved to outside the script in general for all users, and just go back to the old way of installing where they would add these vars to the bashrc or something.

@JacksonBurns
Copy link
Contributor Author

Fixed the last couple issues I believe! I am moving this work to another PR that is cleaner.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants