Closed BradyPlanden closed 6 months ago
One possible fix for this can be found by wrapping the top-level script in a main()
function; however, it's not the cleanest solution for end-users. This can completed as,
import pybamm
import numpy as np
def main():
# create the model
model = pybamm.lithium_ion.DFN()
# set the default model parameters
param = model.default_parameter_values
# change the current function to be an input parameter
param["Current function [A]"] = "[input]"
simulation = pybamm.Simulation(model, parameter_values=param)
# solve the model at the given time points, passing the current as an input
t_eval = np.linspace(0, 600, 300)
current_dict = [{"Current function [A]": x} for x in range(1,3)]
sol = simulation.solve(t_eval, inputs=current_dict)
return sol
if __name__ == "__main__":
sol = main()
If we are happy to move away from multiprocessing
, ray has a drop in replacement to mp.Pool
that appears to solve this issue. This would require adding it as a dependancy though.
Adding this drop-in allows the above MWE to complete without error.
from ray.util.multiprocessing import Pool
with Pool(processes=nproc) as p:
@martinjrobins @jsbrittain @tinosulzer I'll start implementing this if it seems reasonable. Open to other suggestions!
Hi, @BradyPlanden – thanks for investigating a solution and while I'm not sure whether it addresses everything at hand, I did want to highlight that ray
has faced a lot of segfaults and compatibility concerns on many new and old systems alike because their wheels do not conform to the PEP 599 manylinux2014
policy (https://github.com/ray-project/ray/issues/18506) since many years now, so if there is a way around this or if there exists a different library that can handle this, it would be great to use that, or we can hope that these issues are fixed with the upcoming manylinux_2_28
standard. However, if it can be an optional dependency, that should be okay too – another concern with adding it as a required dependency is that the size of their wheels is quite big, which will directly impact PyBaMM's download size (we've been having some discussions about download sizes in #3913, which has some additional info). Out of the rest of our dependencies, just JAX has wheels as large, but users can opt out of installing it since it is an optional dependency.
Sounds good to me, this should definitely be an optional dependency
Thanks for the replies @tinosulzer @agriyakhetarpal.
Turns out fork
is just being depreciated as the default on linux machines starting in 3.14. So an easy fix to this one is to just construct the pool with fork
as mentioned in the original comment. No need to add more dependancies 😄
Yeah I like sticking with multiprocessing over adding more dependencies
PyBaMM Version
24.1
Python Version
3.11
Describe the bug
Multiprocessing inputs within BaseSolver.solve() is broken on operating systems that default to
spawn
(Windows, MacOS) instead offork
(Linux). A minimal working example is shown below; however, when run inside a Jupyter notebook, the code completes without problem, which seems to be how it didn't get caught in tests. I suspect this is due to the difference in call structure, i.e. jupyter might wrap the cells inmain()
. The code in question is: https://github.com/pybamm-team/PyBaMM/blob/c204bdd891d886c955b01bc14049ace5167ed2c3/pybamm/solvers/base_solver.py#L915Modifying this to
with mp.get_context("fork").Pool(processes=nproc) as p:
appears to solve the issue; however, as of python 3.14,fork
is being depreciated forspawn
orforkserver
.Linking https://github.com/pybop-team/PyBOP/pull/283
Steps to Reproduce
MWE:
Relevant log output