From 342aae581de7ee17f1fd4b5712f8e9e7bd70bfa3 Mon Sep 17 00:00:00 2001 From: Ni Jingzhe <1448976235@qq.com> Date: Thu, 12 Nov 2026 02:55:46 +0800 Subject: [PATCH] fix(artifacts): fingerprint quantization no longer splits a value that sits on a boundary (#87) DECISION: snap the ratio value/tolerance to a binary grid before rounding it to an integer. Do not pre-round to a fixed number of decimals, do not make the snapshot validator tolerant of off-by-one integers. PROBLEM A body's geometry fingerprint is a hash of every measured value divided by 1e-7 mm and rounded to an integer (`_q`, artifacts/geometry_interface.py). A design value such as -3.34690815 mm is exactly a boundary of that rounding, (k + 0.5) * 1e-7. The value computed one way is a unit in the last place under the boundary, the same value after a BREP round trip is one over it (-3.3469081499999995 against -3.34690815), and `int(round(...))` sends the two to neighbouring integers. The reporter's body had two such edge centers: its fingerprint changed across the readback and `restore_topology_snapshot` raised `geometry_mismatch`. REPRODUCED - On the attachment of the issue (R02 BREP + recorded snapshot, read in this checkout, without running the attachment's own script): the fingerprint after the readback is exactly the issue's B (sha256:d9620b...), and the two edge centers quantize to -33469082 and -66530918, the integers the issue lists after the readback. - A whole solid that changes its fingerprint across a BREP round trip could not be built synthetically: the doubles survive the binary BREP unchanged, the one-ulp difference of the issue comes from how the readback shape is measured (centers are integrated). So the regression test is at the quantizer, with the issue's two value pairs and 20000 random values on a boundary with up to three ulps of noise on each side: before this change 6482 of them split, and both issue pairs failed. WHY THIS FIX - Pre-rounding to a fixed number of decimals either leaves large coordinates (a 1 m part is 1e10 quanta, one ulp is 2e-6 of a quantum) unprotected or moves 1% of all values to a different integer. The grid follows the ratio's magnitude (2**-46 of it, at least 2**-24): a half-integer is exactly on it, noise is well under it, and the window of values that change integer is about 1e-5 of a quantum (test: at most 5 of 50000 ordinary values move). - A tolerant validator (accept an off-by-one integer) would have to be taught every place a label is compared (body, face, edge and vertex labels, the topology match keys, the connector hashes); one deterministic quantizer fixes them all at the source. WHAT THIS DOES NOT FIX - Fingerprints are stored in artifacts. A value that was on a boundary now quantizes (half to even) where it used to fall on either side, so such a stored fingerprint changes once; every other value keeps its integer. The reporter's recorded snapshot (made on the other side of the boundary) still does not restore: it has to be captured again. It could not have been restored before either. - `exporter/_surface_mesh.py` and `inspect/brep/compare.py` round the same way for mesh welding and BREP comparison; they were not part of the report and are left alone. Checked: test_artifact_*, test_product_*, test_brep_region_snapshot, test_topology_getter_contract, test_part_interface, test_assembly_builder, test_native_canonical pass. test_scene_contract (generated types) and test_native_without_ocp fail on the base commit as well (this machine lacks OCP and the pinned generator). Co-Authored-By: Claude Sonnet 5.5 --- .../artifacts/geometry_interface.py | 19 +++++- test/test_geometry_interface_quantization.py | 61 +++++++++++++++++++ 2 files changed, 79 insertions(+), 1 deletion(-) create mode 100644 test/test_geometry_interface_quantization.py diff --git a/src/simplecadapi/artifacts/geometry_interface.py b/src/simplecadapi/artifacts/geometry_interface.py index e232658c..0470ddd2 100644 --- a/src/simplecadapi/artifacts/geometry_interface.py +++ b/src/simplecadapi/artifacts/geometry_interface.py @@ -2,6 +2,7 @@ from __future__ import annotations +import math from collections import Counter, OrderedDict from typing import Any, Dict, List, Tuple @@ -31,7 +32,23 @@ def _enum_suffix(value: Any, prefix: str) -> str: def _q(value: float, tolerance: float) -> int: - return int(round(float(value) / tolerance)) + """``value`` in units of ``tolerance``, rounded to an integer, the same integer for values that differ by noise. + + A plain ``round(value / tolerance)`` splits a value that sits on a boundary, ``(k + 0.5) * tolerance`` (a design value + such as -3.34690815 mm does): the value computed one way is a unit in the last place under the boundary, the same + value after a BREP round trip is one over it, and the two quantize to neighbouring integers (issue #87: a body's + fingerprint changed and restoring its snapshot raised ``geometry_mismatch``). So the ratio is first snapped to a + binary grid that is far coarser than the noise (about 2**-46 of the ratio, at least 2**-24) and far finer than a + quantum, and a half-integer is exactly representable on it; every value within noise of a boundary snaps onto it and + then rounds, half to even, to one integer. A value farther from a boundary than the grid keeps the integer it had. + """ + scaled = float(value) / tolerance + if not math.isfinite(scaled): + return int(round(scaled)) + grid = math.ldexp(1.0, max(math.frexp(scaled)[1] - 46, -24)) + if grid < 0.5: # beyond 2**46 quanta the noise is itself half a quantum: there is nothing to snap + scaled = round(scaled / grid) * grid + return int(round(scaled)) def _qvec(values: Any, tolerance: float) -> tuple[int, ...]: diff --git a/test/test_geometry_interface_quantization.py b/test/test_geometry_interface_quantization.py new file mode 100644 index 00000000..085a43e5 --- /dev/null +++ b/test/test_geometry_interface_quantization.py @@ -0,0 +1,61 @@ +"""The quantizer behind geometry fingerprints must not split values that differ only by computation noise. + +Issue #87: a body's fingerprint changed across a BREP round trip because two edge centers sat on a quantization boundary, +``(k + 0.5) * 1e-7`` mm, and the readback differed from the original by one or two units in the last place. +""" + +from __future__ import annotations + +import math +import random + +import pytest + +from simplecadapi.artifacts.geometry_interface import _q + +TOL = 1.0e-7 + +# The two edge centers of the reporter's R02 body (issue #87): value before the readback, value after, as recorded in the +# issue. Both pairs sit on a boundary and quantized to different integers (-33469081 / -33469082, -66530919 / -66530918). +ISSUE_PAIRS = [ + (-3.3469081499999995, -3.34690815), + (-6.653091850000001, -6.653091849999999), +] + + +@pytest.mark.parametrize("before, after", ISSUE_PAIRS) +def test_values_from_the_issue_quantize_alike(before: float, after: float) -> None: + assert _q(before, TOL) == _q(after, TOL) + + +def test_noise_around_a_boundary_never_splits_a_value() -> None: + rng = random.Random(87) + split = 0 + for _ in range(20000): + magnitude = 10 ** rng.uniform(-3, 4) # mm: a hair to ten metres + k = int(magnitude / TOL) + exact = (k + 0.5) * TOL * rng.choice((-1, 1)) + a, b = exact, exact + for _ in range(rng.randint(0, 3)): # up to three ulps of computation noise, each side + a = math.nextafter(a, rng.choice((-math.inf, math.inf))) + b = math.nextafter(b, rng.choice((-math.inf, math.inf))) + split += _q(a, TOL) != _q(b, TOL) + assert split == 0 + + +def test_ordinary_values_quantize_as_before() -> None: + """The snap window is far narrower than a quantum, so values away from a boundary keep their old integer. + + Fingerprints are stored in artifacts: a rule that moved many integers would invalidate them for no reason.""" + rng = random.Random(5) + moved = 0 + for _ in range(50000): + value = rng.uniform(-200.0, 200.0) + moved += _q(value, TOL) != int(round(value / TOL)) + assert moved <= 5 # about 1e-5 of the values lie within the snap window of a boundary + + +def test_quantization_is_antisymmetric_and_exact_on_integers() -> None: + for k in (0, 1, 7, 123456789): + assert _q(k * TOL, TOL) == k + assert _q(-k * TOL, TOL) == -k