test(geometry): pin surface generation against the deployed implementation - #2028
Merged
Conversation
…ation Authored ON PRE-MERGE MASTER, deliberately. This commit's parent is f35bead, the tip before the #2026 / #2027 memory work, so the goldens are produced by the implementation that was already deployed rather than by the code they are meant to check. Merging master in comes next, as a separate commit, and that merge is the actual experiment. The gap this fills: VCell's regression suites are math-generation centric and mostly non-spatial. Nothing pins the output of RegionImage or SurfaceCollection, so a change to region finding, surface tessellation, Taubin smoothing or membrane adjacency could alter every spatial model without one test noticing. Each golden records both halves of the picture. What VCML's own <SurfaceDescription> carries -- sample size, cutoff, volume and membrane regions with their sizes -- and what it does NOT: the mesh, the per-pixel region-label map, the surface classes. The second half is where the algorithms live. Six fixtures, chosen so each exercises something the others do not: a 2D image with one flat membrane, 2D stripes with several disconnected regions of the same pixel value, nested 3D spheres (curved, nested regions), the same with smoothing on, four concentric shells (several distinct interfaces), and an analytic subvolume, which reaches RegionImage through the sampling path rather than from an image. The numbers are physically checkable, which is why the goldens are readable text rather than an opaque hash. Subvolume sizes for the nested spheres sum to exactly 1.0, the unit cube. Unsmoothed sphere area is 3.272 against a true 4*pi*r^2 of 2.011 -- the ~1.6x is the staircase area of a voxelated sphere. With smoothing the same surface measures 2.100, recovering the analytic value. A golden that drifts can therefore be argued about rather than merely re-blessed. Determinism was established before anything was committed: two separate JVM runs produce byte-identical output. Node coordinates are quantised to 1e-9 before hashing, so a last-ulp difference cannot flip a golden while a real change still will -- verified, see below. Verified the goldens detect a change rather than merely existing. Perturbing the surface node origin in RegionImage by ~3e-9 (0.5 -> 0.5000001 of a voxel) fails all six fixtures with a readable report naming the line, the golden value and the actual one. That control also found a bad fixture. image2d_two_subvolumes was the one fixture that did NOT notice the perturbation, because GeometryTest.getImageExample2D() never calls updateAll(): its golden read "regionImage none / surfaceCollection none" and pinned essentially nothing. Fixed, and it now pins 202 nodes and 100 polygons over a membrane of area exactly 10.0. A README in the golden directory documents the workflow this commit establishes: branch from HERE to add fixtures, generate goldens with the old implementation, then merge master and let that merge be the experiment. The commit is deliberately reachable as a long-lived base for that. Region images and the region graph are both covered, which took a second pass to get right. The first version pinned region sizes and a digest of the per-pixel region-index image, but not the adjacency, so two things could have changed invisibly: which two volume elements each membrane quad separates (Quadrilateral.getVolIndexNeighbor1/2 -- not implied by the node indices, so a re-assignment with an identical mesh would have passed) and the membrane connectivity graph from RegionImage.calculateNeighbors, which solvers use for membrane diffusion. Both are digested now. The goldens also assert one thing rather than only pinning it: every pixel must belong to exactly one region, so regionImage.pixelPartition prints the sum of region sizes against the total and whether they agree. All six read complete=true. The membrane counts corroborate it independently -- the nested spheres report 16,320 edge neighbours against 4,080 polygons, exactly four per quad, i.e. a closed membrane graph with no dangling edge. everyFixtureHasAGolden guards the guard: a @testfactory with a missing resource would otherwise just produce fewer tests and stay green. Refs #2021, #2025 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018kr8SbzXtwW3gMVUgMfDDt
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.
Fills a real gap: VCell's regression suites are math-generation centric and mostly non-spatial,
so nothing pins
RegionImageorSurfaceCollection. A change to region finding, surfacetessellation, Taubin smoothing or membrane adjacency could alter every spatial model without one
test noticing.
Where the goldens came from
They were generated on
f35beaddcd— master before the #2026 / #2027 memory work — on thebranch
test/geometry-goldens-base, and cherry-picked here. A golden produced by the same codeit is meant to check proves nothing: it photographs the current answer, including any regression
already present.
The experiment ran on that branch first: suite committed against the old implementation, then master
merged in, then the suite re-run. It passed — with the bytecode checked
(
private transient SoftReference<byte[]> softPixels) to confirm the new code was actually in play.So #2026 / #2027 altered no geometry output — not region images, region adjacency, membrane
connectivity, mesh, or surface areas. That was claimed at review time but could not be demonstrated,
because nothing pinned surface generation.
test/geometry-goldens-baseis meant to stayIt is pinned at
f35beaddcdwith the harness on it — a standing checkout of the old implementation.To add fixtures later: branch from it, generate goldens there, then cherry-pick onto master.
Cherry-picking rather than merging is the point; merging master into that branch would drag the new
implementation in and destroy the one property that makes it worth keeping. Documented in
surface-golden/README.md.What each golden captures
Region images and the region graph
Both are covered, and getting there took a second pass. The first version pinned region sizes and a
digest of the per-pixel region-index image but not the adjacency, so two things could have
changed invisibly:
Quadrilateral.getVolIndexNeighbor1/2)— not implied by the node indices, so a re-assignment over an identical mesh would have passed;
RegionImage.calculateNeighbors, which solvers usefor membrane diffusion.
Both are digested now.
The goldens also assert something rather than only pinning it: every pixel must belong to exactly
one region, so
regionImage.pixelPartitionprints the sum of region sizes against the total andwhether they agree. All six read
complete=true. The membrane counts corroborate that independently— the nested spheres report 16,320 edge neighbours against 4,080 polygons, exactly four per quad,
i.e. a closed membrane graph with no dangling edge.
(For the record:
RegionImagedoes build acbit.util.graphspanning forest, but only incalculateRegions, whose sole call site is commented out atRegionImage:407. The live path iscalculateRegions_New, and its adjacency is what is captured above.)Everything else
Both halves — what VCML's own
<SurfaceDescription>carries (sample size, cutoff, volume andmembrane regions with sizes and adjacency), and what it does not: the region-label map, the mesh
(node count, quantised coordinate digest, polygon node-index digest, per-surface polygon counts and
areas), and the surface classes. The second half is where the algorithms live.
Six fixtures, each exercising something the others don't: a 2D image with one flat membrane; 2D
stripes with several disconnected regions of the same pixel value; nested 3D spheres; the same with
smoothing on; four concentric shells; and an analytic subvolume, which reaches
RegionImagethrough the sampling path rather than from an image.
The goldens are arguable, not just diffable
They're readable text with physically checkable numbers, so a drift can be reasoned about instead of
re-blessed:
of a voxelated sphere
A change that moves a surface area away from its analytic value is a bug report, not a golden
update.
Verification
Determinism established before committing anything — two separate JVM runs produce byte-identical
output. Node coordinates are quantised to 1e-9 before hashing so a last-ulp difference can't flip a
golden.
The goldens were verified to detect a change, not merely to exist. Perturbing the surface node
origin in
RegionImageby ~3e-9 (0.5 → 0.5000001 of a voxel) fails all six fixtures with a readablereport naming the line, the golden value and the actual one.
That control also found a bad fixture.
image2d_two_subvolumeswas the one fixture that didnot notice the perturbation:
GeometryTest.getImageExample2D()never callsupdateAll(), so itsgolden read
regionImage none / surfaceCollection noneand pinned essentially nothing. Fixed — itnow pins 202 nodes and 100 polygons over a membrane of area exactly 10.0. Without the control I'd
have shipped a fixture that verified nothing.
everyFixtureHasAGoldenguards the guard: a@TestFactorywith a missing resource would otherwisejust produce fewer tests and stay green.
GeometrySurfaceRegressionTest: 7/7 in 0.38 s — cheap enough for theFastgroup.vcell-coreFast: 594 run, 0 failures, 8 errors — theMathOverrideRoundTripTest/VCellDataTesterrorsdocs/BUILDING.mddocuments for a worktree without the Poetry environments.Scope
Synthetic fixtures only. Real corpus models (
biomodel_27192717and friends carry 2–4 MP images)would broaden coverage but cost seconds per fixture; if wanted, they belong in a slower group than
Fast.Refs #2021, #2025, #2026, #2027
🤖 Generated with Claude Code
https://claude.ai/code/session_018kr8SbzXtwW3gMVUgMfDDt