refactor(precision)!: a System no longer carries a precision at all - #399
Open
ofloveandhate wants to merge 2 commits into
Open
refactor(precision)!: a System no longer carries a precision at all#399ofloveandhate wants to merge 2 commits into
ofloveandhate wants to merge 2 commits into
Conversation
…using them
A System was WEDGED after anything moved the ambient default precision, with no way
out. An SLP's Memory takes its precision from DefaultPrecision() when the program is
lazily compiled, while the owning System keeps whatever it was told, so the two diverge
the moment an AMP tracker or endgame moves the ambient default -- which they do as a
matter of course. Both directions then threw:
eval at the system's OWN precision -> "variable_values and SLP must be of same
precision. respective precisions: 30 16"
eval at the ambient precision -> "precision of input point in SetVariables
(16) must match the precision of the
system (30)."
and neither setter could repair it, because System::precision(n) and
StraightLineProgram::precision(n) both short-circuit when handed the value they already
hold. Measured: the endgame -> Jacobian-spectrum path hits this every single time,
since the endgame is exactly what moved the default.
The Memory's precision is an ARTIFACT OF THE CURRENT EVALUATION, never an invariant to
defend: the compiled Program is a precision-independent tape of operations, and only the
Memory holding values carries digits. So evaluating at whatever precision the caller
brings is always meaningful, and the right response to a mismatch is to re-tag, not to
refuse. Likewise a System, whose precision is the fan-out point to its blocks' working
coefficients and its patch -- each of which already keeps an exact master
(constant_recipes_, coefficients_highest_precision_) beside its working copy.
- SetVariableValues re-tags the SLP's memory to the incoming precision, reusing
precision(), which REFILLS CONSTANTS FROM THEIR EXACT RECIPES rather than padding
them with zeros -- so accuracy is rebuilt, not faked.
- SetPathVariable only ever RAISES: the variables are already in memory by then, so
re-tagging downward would truncate them. Memory ends an evaluation at the max of
its arguments' precisions, and the time slot is re-tagged after assignment because
assigning an mp value adopts the source's precision.
- System::SetVariables aligns the System to the incoming point, pushing the precision
down to every block and the patch. Deliberately NOT behind
BERTINI_DISABLE_PRECISION_CHECKS: alignment is required behaviour, not an assertion.
Threading is unaffected: ZeroDimSolver already gives every worker its own deep-cloned
System, precisely because "residual evaluation mutates System precision state".
Tests in both languages. Beyond the wedge reproduction for Eval and Jacobian, one
system evaluated up and down a ladder of precisions, and the max-of-arguments rule for
the path variable, the mean one asserts that a constant INEXACT in decimal -- 1/3 --
compiled at 16 digits returns residual 0 when evaluated at 100 digits. That only holds
if re-tagging REBUILDS constants from their recipes; padding a 16-digit 1/3 would leave
the tail zero and the residual at ~1e-17.
Part of #377. Does not yet address its asks 2 and 3 (removing System precision as
load-bearing caller-managed state, and one documented ownership rule) -- with evaluation
now aligning on its own, System::precision() has nothing left that callers need, so
removing it is a follow-up sweep over ~161 call sites plus an ADR.
Verified: ctest 10/10 suites pass, pytest 933 passed 1 skipped.
Removes the setter, the getter, AND the precision_ member, in C++ and in Python. Closes the rest of #377 (asks 2 and 3); ADR-0057 records the ownership rule. Once evaluation aligns itself to the point it is handed, a System's precision had no job left. Callers never needed it -- the boilerplate it forced auto target = max(Precision(point), system.precision(), DefaultPrecision()); system.precision(target); was the complaint in #377, and refusing to evaluate on a mismatch could wedge a System with no escape. Two independent places had even grown the same workaround after deserializing, `sys.precision(sys.precision())`, to force a re-materialize; both are gone. The state that genuinely must be carried belongs to the objects that HOLD multiprecision values -- each block's working coefficients, the patch, the SLP's memory -- and each already kept its own "materialized at" tag. The System's copy was a duplicate. Only the patch short-circuited on it, so the five blocks gained the same early return; that is what makes it affordable for SetVariables to fan out unconditionally on every evaluation, where the no-change path is now a handful of integer compares. Result precision is derived from the staged point rather than read from a field. There is also NOTHING TO PREPARE and nothing to fan out. Every evaluable type self-aligns to the precision of the point it is handed, under one name and one body -- SyncPrecision, called first thing in each EvalInPlace / JacobianInPlace / TimeDerivInPlace. Six types have it: the four blocks, the patch, and the SLP. That is not a new invention: blend_block and randomization_block already did exactly this independently, and this makes the pattern AND the name uniform. The patch tells the same story from the other side -- it carried a commented-out assert demanding that callers match its precision, disabled because it could not be honoured. It now aligns itself instead. An earlier draft of this change kept a "prepare"/"materialize at" entry point on System, on the theory that evaluations consuming STAGED values (EvalInPlace/JacobianInPlace with no input point, as Newton uses) need the system prepared beforehand. That theory is false: every such evaluation is preceded by an Eval(point) that stages the point and aligns as a side effect. Removing all 22 of its callers -- 13 in the trackers/endgames/zero_dim_solve, 5 pushing into sub-Systems from blocks, 4 in start systems -- changed no test result. It was pure ceremony, and a long self-documenting name made it look deliberate. Coefficient generation is a construct-time concern. The start systems (mhom, total_degree_linear_product) build their blocks' exact masters under DefaultPrecision(MaxPrecisionAllowed()), which is all that matters -- the ambient default governs the precision new mp values are BORN at. They no longer touch any working precision: setting the precision of working coefficients is an evaluation concern only. Invariants that ceased to exist were removed rather than worked around: FixedPrecisionTracker::PrecisionSanityCheck no longer asks the System what precision it is at, and euler_test's `assert(sys.precision()==50)` is gone because there is nothing left to report. The tracker observers' log formatting now takes its digit count from the VALUES being printed -- not from the ambient default, which need not be the precision anything was computed at. ARCHIVE FORMAT CHANGED: precision_ is no longer serialized. It was transient evaluation state, which the same serialize() already excludes elsewhere (current_variable_values_), but a System archived by an older build will not load into a newer one. Boost archives carry Systems between MPI ranks within one run; durable storage is the records/JSON path (ADR-0042) and is unaffected. Most of the diff at call sites is DELETION -- the three-line incantation DefaultPrecision(50); sys.precision(50); expanded.precision(50); collapses to its first line, because the points carry 50 digits and the systems follow. One test needed rethinking rather than converting: system_identity_test proved that two Python handles are one C++ System by setting a precision through one and reading it back through the other. With no precision to observe, it now stages a point through one handle and evaluates with NO ARGUMENTS through the other -- which only works if they share their staged values. That is a better proof, since it exercises the real shared transient state instead of a bookkeeping field. Considered and rejected, recorded in the ADR so it is not re-proposed: moving the staged values into a separate evaluation handle (`sys.At(point).Values()`). It would tax the common case -- sys.Eval(point), one call, no ceremony -- to buy thread-safety that per-thread cloning already provides. Direct evaluation is the interface worth protecting. Verified: ctest 10/10 suites, pytest 934 passed 1 skipped, both doclints clean -- including the AMP-heavy tracking, endgame and nag_algorithms suites, which are the ones that would notice.
ofloveandhate
force-pushed
the
refactor/remove-system-precision
branch
from
August 27, 2026 21:45
4d2329f to
451e9b9
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes the rest of #377 (asks 2 and 3). Stacked on #394 — review that first; this branch
contains its commit, and the removal only makes sense once evaluation aligns itself.
A System no longer carries a precision
The setter, the getter, and the
precision_member are all gone, in C++ and in Python.Once evaluation aligns to the point it is handed (#394), a System's precision had no job
left. What it did have was a cost — the boilerplate it forced on every caller:
auto target = max(Precision(point), system.precision(), DefaultPrecision()); system.precision(target);That is the complaint #377 opens with, and refusing to evaluate on a mismatch could wedge a
System outright with no escape through either setter. Two independent places had even grown
the same post-deserialize workaround,
sys.precision(sys.precision()), to force are-materialize —
witness_set.hppandsystem_export.cpp. Both are gone.Where the state actually belongs
The objects that hold multiprecision values — each block's working coefficients, the
patch, the SLP's memory — already kept their own "materialized at" tag. The System's copy was
a duplicate of theirs.
Only the patch elided on it. The five blocks gained the same early return, and that is
what makes it affordable for
SetVariablesto fan out on every evaluation: the unchangedpath is now a handful of integer compares across seven holders.
The elision is deliberately per holder and not at the System, which is a soundness point
rather than a style one. A System-level early-out would need a System precision to compare
against, which no longer exists — and inventing one would skip work that is genuinely needed,
because holders can legitimately disagree. An SLP compiled at the ambient default while the
rest sat elsewhere is the desync of #377; a System-level cache would say "already at 30,
nothing to do" and skip the very repair. Each holder knows its own truth.
Result precision is likewise derived:
CoerceBlockOutputPrecisiontakes its target fromthe staged point instead of a stored field.
Nothing remains to prepare
There is no "prepare"/"materialize at" entry point either. Every evaluable type
self-aligns to the precision of the point it is handed, under one name and one body:
called first thing in each
EvalInPlace/JacobianInPlace/TimeDerivInPlace. Six typeshave it: the four blocks, the patch, and the SLP.
This is not a new invention.
blend_blockandrandomization_blockalready did exactlythis independently — the change makes the pattern and the name uniform. The patch tells the
same story from the other side: it carried a commented-out assert demanding that callers
match its precision, disabled because it could not be honoured. It now aligns itself instead.
An earlier draft of this branch kept a prepare-style method, on the theory that evaluations
consuming staged values (
EvalInPlace/JacobianInPlacewith no input point, as Newtonuses) need the system prepared beforehand. That theory is false: every such evaluation is
preceded by an
Eval(point)that stages the point and aligns as a side effect. Removing all22 of its callers — 13 in the trackers/endgames/
zero_dim_solve, 5 pushing intosub-Systems from blocks, 4 in start systems — changed no test result. It was pure ceremony,
and a long self-documenting name made it look deliberate.
Coefficient generation is a construct-time concern
The start systems (
mhom,total_degree_linear_product) build their blocks' exact mastersunder
DefaultPrecision(MaxPrecisionAllowed())— which is all that matters, since the ambientdefault governs the precision new mp values are born at. They no longer touch any working
precision. Setting the precision of working coefficients is an evaluation concern only.
Invariants that ceased to exist were removed, not worked around
FixedPrecisionTracker::PrecisionSanityCheckno longer asks the System what precision itis at.
euler_test'sassert(sys.precision()==50)is gone — there is nothing left to report.printed, not from the ambient default (which need not be the precision anything was
computed at) and not from the System.
Breaking
System.precisionno longer exists, in either form.precision_is no longer serialized. It was transient evaluation state— the same
serializealready excludescurrent_variable_values_for exactly that reason —but a System archived by an older build will not load into a newer one. Boost archives carry
Systems between MPI ranks within one run; durable storage is the records/JSON path
(ADR-0042) and is unaffected.
Notes for review
Most of the call-site diff is deletion. The three-line incantation
collapses to its first line, because the points carry 50 digits and the systems follow.
One test needed rethinking rather than converting.
system_identity_testproved that twoPython handles are one C++ System by setting a precision through one and reading it back
through the other. With no precision to observe, it now stages a point through one handle and
evaluates with no arguments through the other — which only works if they share their
staged values. That is a better proof: it exercises the real shared transient state instead of
a bookkeeping field.
Considered and rejected
Moving the staged values into a separate evaluation handle (
sys.At(point).Values()).Recorded in ADR-0057 so it is not re-proposed: it would tax the common case —
sys.Eval(point), one call, no ceremony — to buy thread-safety that per-thread cloningalready provides. Direct evaluation is the interface worth protecting.
current_variable_values_stays: unlike
precision_it is invisible to callers and does real work, backing theno-argument
Eval()/Jacobian()so the SLP's tape runs once and the Jacobian can be readlater without re-running it.
ADR
ADR-0057 — the ownership rule #377 asks for, with a table of who holds precision truth
(the point is the authority; every
precision_is a derived cache tag; the exact masters arethe source of truth for values, so everything is re-materialized, never re-drawn), the
archive-format consequence, and the rejected handle design.
Verified:
ctest10/10 suites,pytest934 passed / 1 skipped, both doclints clean —including the AMP-heavy tracking, endgame and nag_algorithms suites, which are the ones that
would notice.