Repository navigation
An accuracy estimate names its coordinates, and the bare name is retired - #487
Merged
ofloveandhate merged 1 commit intoOct 3, 2026
Conversation
A solution's metadata carries two accuracy estimates, both the distance between the endgame's last two approximations of the root. accuracy_estimate was in the solver's internal coordinates (homogenized, on the patch) and accuracy_estimate_user_coords in the user's. The plain name read as the accuracy of my solution in my coordinates, and it was not that: the internal estimate is what final_tolerance is compared with, so it behaves like a number of correct digits, and the absolute error in the user's variables is that times roughly the scale of the solution. accuracy_estimate is renamed accuracy_estimate_internal_coords. accuracy_estimate_user_coords keeps its name. The bare name is retired, not given to the other estimate: the same name returning a different number would leave every reader running on a value off by the scale of its solution, with no error anywhere. In Python, reading it raises a RuntimeError that names both successors. It is not an AttributeError, because getattr with a default swallows that and the caller goes on with the default. The records write each estimate under its new key, and the reader accepts the earlier key for the internal one, so a record written before the rename loads. The dataframe column is renamed with the field. The docstrings of both estimates and of final_tolerance now say which coordinates they are in, and that the tolerance is in effect a number of digits. Whether either estimate bounds the true error is a separate question and is not asserted here. ADR-0069. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
ofloveandhate
added this pull request to stack #488
October 3, 2026 17:12
ofloveandhate
commented
Oct 3, 2026
ofloveandhate
left a comment
Contributor
Author
There was a problem hiding this comment.
this code is correct, and closes a point of confusion i have had for a while. the aphorism i am practicing is "prefer explicit to implicit".
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.
Stacked on #486 (
fix/combining-systems-compares-the-variables), so the diff shows only this work; it retargets when that one merges.A solve reports two accuracy estimates for an endpoint, and both are the infinity-norm distance between the endgame's last two approximations. One is measured in the coordinates the tracker works in, homogenized and patched; the other after dehomogenization, in the coordinates the caller wrote the system in. The second was already named
accuracy_estimate_user_coords. The first was the bareaccuracy_estimate, and a caller reads a bare name as being about their own coordinates, which it was not. It is nowaccuracy_estimate_internal_coords, in the C++ results (zero_dim_solve.hpp,parallel/path_result.hpp,nag_algorithms/output.hpp), in the records writer, and in the Python dataframe columns. The records reader accepts the earlier key, so records written before this change still load.The bare name is retired rather than aliased. In Python it is a property that raises
RuntimeErrornaming both successors. It is deliberately not anAttributeError, becausegetattr(md, 'accuracy_estimate', 0.0)would swallow that and silently read zero, which is exactly the silent misreading the rename is meant to end.The
final_tolerancedocstring now says what the setting is: a number of digits, applied in internal coordinates, so that a solution of size 1000 with six digits correct reports an accuracy near 1e-3 in user coordinates.ADR-0069 records the naming decision. Tests:
python/test/zero_dim/accuracy_estimate_names_test.pyand a records round trip incore/test/nag_algorithms/zero_dim_records.cpp. Verified: all C++ suites, the Python suite, the tutorial doctests, and the three lints; the cellular decomposition port was moved to the new name in the same session.🤖 Generated with Claude Code