Repository navigation
Simulation state is process-wide, not per-simulation #175
Description
Activity
One clarification on the
StringRepresentation::outputstrrow, after chasing it further: the shared buffer is safe under sequential use, and nothing in the table above should be read as a live single-threaded bug.dumpStringRepresentation()resets the buffer withoutputstr.str("")atStringRepresentation.cpp:341before writing, and every write to it sits inside avisit()method reachable only through thedomain->accept(this)on the next line. There is no other entry point.That matters for the coupled path specifically.
Command.cpp:205and232create two representations in one session —outStrReppandoutStrRep— and both are inserted into the timetable as events, so both calloutput()during a run. Because each dump resets the buffer first, their outputs cannot mix. Only overlapping dumps can corrupt each other, which is the threading case already measured above.The same applies to
currentLevel(StringRepresentation.cpp:28), reset in the same place.So step 3 in the plan is a design fix — a static buffer on a class that is instantiated more than once — rather than a repair of anything that misbehaves today. Recording it here so nobody goes looking for a coupled-run reproduction that does not exist.
- added a commit that references this issue
on Aug 12, 2026 - added a commit that references this issue
on Aug 13, 2026 hi @HugoFara and @antonio-leblanc Thanks so much for all the hard work.
I was heavily involved into wildfire nowcasting this summer, and now have loads of reports to hand-out.. sorry not to be reactive. I'll have more time end of october hopefully.
Netcdf/HDF should be definitely optional and isolated at some point. will leverage webassembly and many other possibilities.
Nevertheless, I will have trouble reviewing these massive PRs, and the potential implication most notably for the coupled simulations. I really need to understand what is going on, especially if it is AI generated, so to foresee what implications it has. This would be my first goal and thanks again for all the work.most notably Singleton ensures i can access the static link memory part of the process in Fortran. Launching concurent processes would not be a problem even with a static context as long as it is completely forked. But indeed, since ForeFire is a simulation kernel itself, many simulation means many kernels. Another possibility could maybe be to launch a separated process when
sim = new ForeFire() is called in Python instead of making it all dynamically referenced with counters.
I think a cleaner architecture may be to keep the current process-local singleton design and move the isolation boundary to the Python binding.
Instead of making the whole C++ kernel fully re-entrant, each Python
ForeFire()instance could own a separate worker process:Python | +-- ForeFire() ---> worker process A ---> singleton/context A | +-- ForeFire() ---> worker process B ---> singleton/context BThis gives memory isolation for free, preserves the existing Fortran/MPI coupling model, and avoids propagating instance/context pointers throughout the codebase.
It would also make simulation branching natural:
base simulation | +-- fork() ---> runner A ---> step/parameters A +-- fork() ---> runner B ---> step/parameters B +-- fork() ---> runner C ---> step/parameters CWith copy-on-write, forks could initially share the initialized simulation memory and diverge only when modified.
So keeping the current kernel design may actually be a good option; the main change would be in the Python constructor/binding layer rather than invasive changes across the C++ code.
More interestingly, the simulation can then naturally become a tree. At any point, a running simulation could be forked into several new runners:
simulation | ------+------ | | runner A runner B | | ----+---- +---- runner D | | runner C runner EEach branch starts from the exact simulation state at the fork point and can then receive different parameters, forcings or simulation steps. With copy-on-write, the branches initially share the same physical memory and only diverge when state is modified.
This would make dynamic ensembles particularly natural: ensemble members can be created at any time during the simulation, not only at initialization, while remaining fully memory-isolated.
So keeping the current kernel design may actually be an advantage. The main architectural change would live in the Python binding/process management layer rather than requiring invasive context changes throughout the C++ codebase.
I beleive it is the problem only in Python... because otherwise, i've been launching ensembles for long time simply by calling the CommandShell process in batch from forefire interpreter and with multiple ff scripts that provides the parameter coverage
I would put
ForeFireRunnerentirely inside the Python binding layer, because it is an execution/isolation concern of the Python API, not part of the ForeFire simulation kernel itself.A clean structure would be:
bindings/python/src/pyforefire/ │ ├── _pyforefire.cpp ├── _pyforefire.h │ ├── ForeFireRunner.cpp ├── ForeFireRunner.h │ ├── ForeFireWorker.cpp # optional, can initially be merged into Runner.cpp ├── ForeFireWorker.h │ └── ...The responsibilities would be:
_pyforefire.cpp | | pybind11 API only v PLibForeFire | | owns v ForeFireRunner | | IPC / process management v worker process | v existing ForeFire kernel Command / Session / FireDomain / SimulationParametersI would make
_pyforefire.cppvery thin.For example:
class PLibForeFire { public: PLibForeFire(); std::string execute(const std::string&); void setDouble(const std::string&, double); double getDouble(const std::string&); PLibForeFire fork(); private: std::shared_ptr<ForeFireRunner> runner; };
And then:
PLibForeFire::PLibForeFire() : runner(std::make_shared<ForeFireRunner>()) { runner->spawn(); }
ForeFireRunner.hcould look roughly like:class ForeFireRunner { public: ForeFireRunner(); ~ForeFireRunner(); void spawn(); std::string execute(const std::string& command); void setDouble(const std::string& name, double value); double getDouble(const std::string& name); std::shared_ptr<ForeFireRunner> forkRunner(); private: pid_t pid; int commandFd; int responseFd; void workerLoop(); };
Then the actual ForeFire objects exist only in the worker:
void ForeFireRunner::workerLoop() { Command pyxecutor; Command::Session* session = &pyxecutor.currentSession; SimulationParameters* params = session->params; while (true) { Request req = readRequest(); switch (req.type) { case EXECUTE: // pyxecutor.ExecuteCommand(...) break; case SET_DOUBLE: // params->setDouble(...) break; case FORK: // fork current worker break; case QUIT: return; } } }
So the key separation is:
ForeFire core DOES NOT KNOW about: Python pybind11 processes pipes fork() bindings/python/ knows about: pybind11 ForeFireRunner process creation IPC fork treesI would actually start with only these four files:
bindings/python/src/pyforefire/ ├── _pyforefire.cpp ├── _pyforefire.h ├── ForeFireRunner.cpp └── ForeFireRunner.hNo need for
ForeFireWorkerinitially.workerLoop()can just be a private/static function inForeFireRunner.cpp.One detail: I would probably avoid calling the class simply
Runnerbecause later you may have MPI runners, local runners, remote runners, etc.ForeFireRunneror evenProcessForeFireRunnermakes the execution model explicit.The eventual architecture could then evolve naturally into:
PLibForeFire | v ForeFireRunner interface | +-- LocalForeFireRunner | +-- ProcessForeFireRunner | +-- RemoteForeFireRunner # someday, cluster/serviceBut I would not introduce that abstraction yet. Start with one
ForeFireRunnerin the Python binding folder and keep the kernel untouched.Thanks @filippi for the detailed answer, that makes sense. I'll drop steps 5 and 6 (I held off on PRs for those to get your input first) and move the isolation into the Python binding, as you describe.
Two things from what I tested:
- Give the propagation models and layers an owner #182: on rank 0,
Init.ffbuildsfdp(Command.cpp:195), which clears the static model tables, sofd's Rothermel is leaked andfdusesfdp's. On therunffcase the fronts are identical, becauseDataBroker's layers are static too, so coupled runs only leak one model. Where Give the propagation models and layers an owner #182 matters is building many domains in one process (+91.5 MB for 500), which still happens inside a Python worker. - NetCDF: I asked the netcdf-cxx4 maintainers about a release (New release? Unidata/netcdf-cxx4#165). The maintainer has little time, and there has been no release since 2019. One more reason to make NetCDF optional.
A few points on the runner:
- Your sketch covers
executeand get/set.addScalarLayer/getDoubleArrayalso need an array format over the pipe. - The Python interpreter may have threads, so the first worker should start with
fork+exec. Forking from the worker is fine. - Each branch needs its own output path, and open NetCDF/HDF5 files must be closed before forking.
If okqy with you, I can do a small PR: one
ForeFireRunnerwithexecute, get/set and arrays, and noFORKyet. I also reduced the verbosity of the PR/issues description this morning as it was still too long, I don't know which version you read, but it should be easier to review now.Thanks again, and let me know if anything!
Hugo- Give the propagation models and layers an owner #182: on rank 0,
- Great ! Thanks Indeed, all files closed... Yes, great for worker, as long as it is isolated in the pytjon bindings that's great ******************************************** Jean-Baptiste Filippi Chargé de Recherches SPE - UMR 6134 CNRS Université de Corse Quartier grossetti BP 52, 20250 Corte Tel. +33 495 450 158 ****************************************** Le sam. 26 sept. 2026, 18:32, Hugo ***@***.***> a écrit :…*HugoFara* left a comment (forefireAPI/forefire#175) <#175 (comment)> Thanks @filippi <https://github.com/filippi> for the detailed answer, that makes sense. I'll drop steps 5 and 6 (I held off on PRs for those to get your input first) and move the isolation into the Python binding, as you describe. Two things from what I tested: - *#182 <#182>:* on rank 0, Init.ff builds fdp (Command.cpp:195), which clears the static model tables, so fd's Rothermel is leaked and fd uses fdp's. On the runff case the fronts are identical, because DataBroker's layers are static too, so coupled runs only leak one model. Where #182 <#182> matters is building many domains in one process (+91.5 MB for 500), which still happens inside a Python worker. - *NetCDF:* I asked the netcdf-cxx4 maintainers about a release ( Unidata/netcdf-cxx4#165 <Unidata/netcdf-cxx4#165>). The maintainer has little time, and there has been no release since 2019. One more reason to make NetCDF optional. A few points on the runner: - Your sketch covers execute and get/set. addScalarLayer / getDoubleArray also need an array format over the pipe. - The Python interpreter may have threads, so the first worker should start with fork+exec. Forking from the worker is fine. - Each branch needs its own output path, and open NetCDF/HDF5 files must be closed before forking. If okqy with you, I can do a small PR: one ForeFireRunner with execute, get/set and arrays, and no FORK yet. I also reduced the verbosity of the PR/issues description this morning as it was still too long, I don't know which version you read, but it should be easier to review now. Thanks again, and let me know if anything! Hugo — Reply to this email directly, view it on GitHub <#175?email_source=notifications&email_token=AAUZ7CSJPS6YBJJWNDGB7XD5Q7VTHA5CNFSNUABFM5UWIORPF5TWS5BNNB2WEL2JONZXKZKDN5WW2ZLOOQXTKOBUG44TENRQGIZ2M4TFMFZW63VHNVSW45DJN5XKKZLWMVXHJLDGN5XXIZLSL5RWY2LDNM#issuecomment-5847926023>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAUZ7CVBE5EZATACG7PHZAD5Q7VTHAVCNFSNUABEKJSXA33TNF2G64TZHMYTOMRUHE3TKMJ3JFZXG5LFHM2TCMZTHEYTQOJQGGQXMAQ> . You are receiving this because you were mentioned.Message ID: ***@***.***>
Question for @filippi / @antonio-leblanc: should one process be able to run several simulations (for ensembles, #172, or free-threaded Python)? If not, we merge the small fixes below and stop there.
ForeFire keeps simulation state in statics and a singleton:
SimulationParameters::instance,FireDomain::propModelsTable/fluxModelsTable, theFireNodenumerical settings,ForeFireAtom::instanceNRCountandStringRepresentation::outputstr. The GIL hides this today. Without it, 8 concurrent simulations segfault in 4 out of 5 runs, and the fifth gives wrong node counts (27–209 instead of 62).Plan:
Stress test(Add the concurrency stress test that reproduces #175 #176, merged)Atomic id counter and thread-safe singleton(Make the atom id counter atomic and the parameters singleton safe #177, merged)One output buffer per(Give each string representation its own buffer #178, merged)StringRepresentationSteps 1–4 are correctness fixes. They are not enough for thread safety on their own.
FireDomain,DataBroker,CommandandCLibForeFire, so the coupling ABI that Meso-NH uses must be agreed first.cp314twheels again (reverses part of Stop shipping free-threaded (cp314t) wheels #155), only after step 5.Drafted with Claude Opus 5, reviewed by a maintainer.