You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
ForeFire keeps simulation state in process-wide storage — statics, class-wide members, and one singleton — rather than in the objects that own a simulation. A FireDomain is not a self-contained simulation; it is one of several users of a shared pool.
Under a normal CPython this is invisible: the binding never releases the GIL, so every entry point is serialised. Take the GIL away and eight concurrent simulations crash or return wrong answers.
What is shared
Line numbers against dev at e1df887.
State
Where
Scope today
SimulationParameters::instance
SimulationParameters.cpp:29
One parameter set for the whole process.
ForeFireAtom::instanceNRCount
ForeFireAtom.h:106
Every atom's id from one counter, incremented with a plain ++.
FireDomain::propModelsTable, fluxModelsTable
FireDomain.cpp:58-59
Fixed arrays of 50 and 500 slots, scanned at 859, written at 697. Two domains compete for the same slots.
Numerical settings held per class, so every node in the process shares one value.
StringRepresentation::outputstr
StringRepresentation.cpp:29
One ostringstream shared by every representation object.
There are no concurrency primitives anywhere in src/ except the HTTP server's running flag: ~33,000 lines searched for std::mutex, std::atomic, lock_guard, pthread_mutex and OpenMP directives, one match.
Measured
Eight threads, each building a 1000×1000 domain with Iso, igniting at the centre and stepping five times; each must reproduce the fire node count it produces alone. CPython 3.14.6 free-threading, manylinux_2_28, wheel built from source.
Build
GIL on
GIL off, 5 runs
dev @ e1df887
3/3 pass
4 × segfault, 1 × wrong results
dev + the four small fixes below
—
4 × segfault, 1 × abort
full branch (all 8 commits)
pass
5/5 pass
The GIL-on column is the control: identical workload and code, passing every time. The only variable is serialisation.
The one GIL-off run on dev that did not crash returned:
single-threaded baseline: 62 fire nodes
FAILED with 7 problem(s):
- thread 1 produced 49 fire nodes, expected 62
- thread 2 produced 46 fire nodes, expected 62
- thread 3 produced 209 fire nodes, expected 62
- thread 4 produced 27 fire nodes, expected 62
Silently wrong output is worse than the segfaults. The single-threaded baseline is 62 on both dev and the fixed branch, so nothing below changes results.
The four small fixes do not make threaded use safe on their own, as the middle row shows. Each is a correctness fix; safety arrives only with the state refactor in step 5.
Why it matters
More than one simulation per process.Command.cpp:187 already carries a special case for a second FireDomain[...], keyed on the domain ID being 1 — the shape a shared model table forces. #172 wants many simulations, and today each must be its own process. (Read from the code, not measured; the crashes above are the threading half.)
Free-threaded Python.#155 stopped shipping cp314t wheels because of the table above and closed with "worth its own issue if anyone wants it". This is that issue, and the measurements say #155 was right: the GIL is what currently makes ForeFire safe.
Proposed order
Drafted on a branch, splitting into parts with very different risk. I would like the first four to land independently:
The concurrency stress test (tests/python/test_threading.py, new) — the harness above. Skips loudly with the GIL on, so it cannot pass vacuously. No core changes.
Atomic id counter and a safe singleton.instanceNRCount → std::atomic<long> with relaxed fetch_add; GetInstance → function-local static, whose once-only initialisation C++11 already guarantees. Eight lines, no single-threaded behaviour change.
A per-instance output buffer for StringRepresentation.
Serialise entry into NetCDF and HDF5. The NetCDF C library is not thread-safe unless built for it; the lock is additive and uncontended single-threaded.
Then the substantial part, which is a real API change and should not proceed without maintainer agreement:
Give each simulation its own state — model tables, FireNode settings and parameters onto the domain; Command and the Python binding instance-based. Touches the signatures of FireDomain, DataBroker, Command and CLibForeFire, so the coupling ABI needs deciding first.
Steps 1–4 rebase cleanly onto dev today. Step 5 conflicts with DataBroker.cpp and Command.cpp and is still rough.
What would help
Whether multiple simulations in one process is something the project wants at all. If not, steps 1–4 are still worth having as plain correctness fixes and 5–6 should be closed. If yes, the coupling ABI is the thing to settle first, since CLibForeFire's extern "C" surface is what Meso-NH binds to.
Drafted by Claude Opus 5 from a codebase audit. Reviewed by a maintainer before filing.
ForeFire keeps simulation state in process-wide storage — statics, class-wide members, and one singleton — rather than in the objects that own a simulation. A
FireDomainis not a self-contained simulation; it is one of several users of a shared pool.Under a normal CPython this is invisible: the binding never releases the GIL, so every entry point is serialised. Take the GIL away and eight concurrent simulations crash or return wrong answers.
What is shared
Line numbers against
devate1df887.SimulationParameters::instanceSimulationParameters.cpp:29ForeFireAtom::instanceNRCountForeFireAtom.h:106++.FireDomain::propModelsTable,fluxModelsTableFireDomain.cpp:58-59859, written at697. Two domains compete for the same slots.FireNode::nmlScheme,smoothing,relax,minSpeed,minFrontDepthFireNode.cpp:22-30StringRepresentation::outputstrStringRepresentation.cpp:29ostringstreamshared by every representation object.There are no concurrency primitives anywhere in
src/except the HTTP server'srunningflag: ~33,000 lines searched forstd::mutex,std::atomic,lock_guard,pthread_mutexand OpenMP directives, one match.Measured
Eight threads, each building a 1000×1000 domain with
Iso, igniting at the centre and stepping five times; each must reproduce the fire node count it produces alone. CPython 3.14.6 free-threading, manylinux_2_28, wheel built from source.dev@e1df887dev+ the four small fixes belowThe GIL-on column is the control: identical workload and code, passing every time. The only variable is serialisation.
The one GIL-off run on
devthat did not crash returned:Silently wrong output is worse than the segfaults. The single-threaded baseline is 62 on both
devand the fixed branch, so nothing below changes results.The four small fixes do not make threaded use safe on their own, as the middle row shows. Each is a correctness fix; safety arrives only with the state refactor in step 5.
Why it matters
More than one simulation per process.
Command.cpp:187already carries a special case for a secondFireDomain[...], keyed on the domain ID being 1 — the shape a shared model table forces. #172 wants many simulations, and today each must be its own process. (Read from the code, not measured; the crashes above are the threading half.)Free-threaded Python. #155 stopped shipping
cp314twheels because of the table above and closed with "worth its own issue if anyone wants it". This is that issue, and the measurements say #155 was right: the GIL is what currently makes ForeFire safe.Proposed order
Drafted on a branch, splitting into parts with very different risk. I would like the first four to land independently:
tests/python/test_threading.py, new) — the harness above. Skips loudly with the GIL on, so it cannot pass vacuously. No core changes.instanceNRCount→std::atomic<long>with relaxedfetch_add;GetInstance→ function-local static, whose once-only initialisation C++11 already guarantees. Eight lines, no single-threaded behaviour change.StringRepresentation.Then the substantial part, which is a real API change and should not proceed without maintainer agreement:
FireNodesettings and parameters onto the domain;Commandand the Python binding instance-based. Touches the signatures ofFireDomain,DataBroker,CommandandCLibForeFire, so the coupling ABI needs deciding first.py::mod_gil_not_used()and resumecp314twheels, only once 5 is done. Deliberately reverses part of Stop shipping free-threaded (cp314t) wheels #155.Steps 1–4 rebase cleanly onto
devtoday. Step 5 conflicts withDataBroker.cppandCommand.cppand is still rough.What would help
Whether multiple simulations in one process is something the project wants at all. If not, steps 1–4 are still worth having as plain correctness fixes and 5–6 should be closed. If yes, the coupling ABI is the thing to settle first, since
CLibForeFire'sextern "C"surface is what Meso-NH binds to.Drafted by Claude Opus 5 from a codebase audit. Reviewed by a maintainer before filing.
EDIT: rewrote for human readability.