Repository navigation
Split property_engine.h into separately tested units (no behaviour change) #232
Description
Part of #237. First phase: every later phase touches the engine.
Why
core/property_engine.h grows from 609 lines on main to 1359 lines with #229. One class now holds six responsibilities:
| Responsibility | Today (line numbers on feature/issue-218) |
|---|---|
| Graph building | compute_execution_order (~317), edges and kinds, find_cycle/describe_cycle (~487-528), upstream_of (~1023), graph() |
| Scheduling | constant folding (find_constants ~570, refresh_constants, retire, verify), track_reads, route_to_writers, distribute |
| Iteration groups and stages | form_groups (~897), stage running |
| Derivatives | index_partials (~1100), derivative<R>, differentiate, accumulate (~1258), fold (~1270), plans and slots |
| Residual systems | plan_systems (~1118), solve, step, jacobian, jacobian_by_differences, start, scatter, gather (~753-890) |
| History state | commit, revert, checkpoint/restore, collect_history_properties |
Each feature added since #173 has grown the same class. The units share members freely: m_edges, m_reverse_edges, m_upstream, the stage lists and the plans. None of them can be tested without a full material context. The review of #226 and #229 found two bugs whose cause was this sharing:
- group membership read the non-local edges only;
update_property()did not know about system stages.
Design
- One header per unit under
core/engine/, each a class with an explicit narrow interface:graph.h: the immutable result ofbuild(), holding nodes, typed edges, topological order,upstream_ofand the cycle report;schedule.h: the active/constant callback lists and stages, built from a graph;groups.hsystems.hderivatives.h(the engine side; packing stays incore/derivatives.h)history_state.h
property_enginebecomes a facade that owns the units and forwards the public API unchanged.material_contextand every host stay untouched.- Data flows one way: graph → schedule → (groups, systems, derivatives). A unit receives the graph by const reference, and no unit reaches into another's members.
- Each unit gets its own test file working on a hand-built graph of fake properties, with no materials.
Tests
- All existing tests pass unchanged; this refactor has no behaviour change.
- Report CSVs are byte-identical to before.
- New per-unit tests, for example:
graph: cycle report and upstream sets on a hand-built graph;schedule: constant folding on a graph with a guarded node;systems: Newton on a 2×2 residual through fake properties;derivatives: accumulation order on a diamond graph.
- A size check in CI: no header under
core/engine/over about 400 lines (a script, not a hard rule).
Depends on
#229 merged (it is the last PR adding to the monolith).
Out of scope
Any API change, and new features. Fixes found during the split go in separate commits with their own red test.
Activity