Skip to content

Simulation state is process-wide, not per-simulation #175

Description

@HugoFara

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.
FireNode::nmlScheme, smoothing, relax, minSpeed, minFrontDepth FireNode.cpp:22-30 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:

  1. 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.
  2. Atomic id counter and a safe singleton. instanceNRCountstd::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.
  3. A per-instance output buffer for StringRepresentation.
  4. 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:

  1. 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.
  2. Declare py::mod_gil_not_used() and resume cp314t wheels, only once 5 is done. Deliberately reverses part of Stop shipping free-threaded (cp314t) wheels #155.

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.

EDIT: rewrote for human readability.

Metadata

Metadata

Assignees

No one assigned

    Labels

    Type

    No type

    Projects

    No projects

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions