test(geometry): add real corpus models as Geometry_IT fixtures - #2029
Merged
Conversation
Generated on the golden base branch, against the pre-change implementation (f35bead), per the workflow this suite documents. Cherry-picked onto master separately so the goldens provably predate the code they check. Six stored models from the VCML test corpus, spanning 0.47-3.96 MP, 2D and 3D, cubic through to a 2151x504 slab. They earn their place by being irregular in ways the synthetic fixtures are not -- thin features, awkward aspect ratios, and disconnected regions sharing a pixel value. corpus_95707047_208x153x83 alone yields SIX regions from TWO pixel classes, five separate cytosol bodies, which nothing in the synthetic set produces. They deliberately REBUILD surfaces rather than pinning what the document already carries. A stored model has a <SurfaceDescription>, XmlReader applies it, and precomputeAll therefore skips updateAll() on parse -- so the geometry arrives with regions restored from the file rather than computed. Pinning those would test the XML reader instead of surface generation, which is the opposite of the point. fromCorpus() calls updateAll() to force a fresh RegionImage and SurfaceCollection. Kept in their own group and class so the fast lane stays fast. Worth being straight about the naming though: at 2.87 s for all six they are not actually slow, and could sit in Fast if running on every push is preferred over running only in the regression lane. The separation is mainly headroom -- they depend on corpus resources and the obvious way to extend them is to add more models. Also splits golden regeneration so iterating on the synthetic fixtures does not silently leave the corpus goldens stale or spend minutes rebuilding them: GeometrySurfaceGolden.main with no argument does both, with 'fast' does only the synthetic set. The comparison logic is now shared rather than duplicated between the two test classes, including the everyFixtureHasAGolden guard -- a @testfactory whose resources went missing would otherwise just produce fewer tests and stay green. Pre-merge verification: GeometrySurfaceRegressionTest 7/7 (0.42 s), GeometrySurfaceCorpusRegressionTest 7/7 (2.87 s). Refs #2021, #2025, #2028 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018kr8SbzXtwW3gMVUgMfDDt
Without this the corpus goldens compile and never execute in CI -- regression.yml drives its matrix from an explicit group table, so a tag that is not listed there is simply never selected. One shard; the whole group is ~3 s of tests on top of the ~4 min compile floor every shard already pays. Verified by executing the workflow's embedded matrix script directly rather than trusting the edit: 16 entries across 7 groups, with Geometry_IT present as a single shard scoped to vcell-core. The YAML parses. Refs #2021, #2028 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.
Extends the geometry golden suite (#2028) with real stored models, and wires the group into
regression CI.
Built the documented way: fixtures and goldens were generated on
test/geometry-goldens-baseagainst the pre-change implementation (
f35beaddcd), then cherry-picked here — so the goldensprovably predate the code they check.
Why real models, when synthetic fixtures already pass
The synthetic set is spheres, shells and stripes. Real segmentations are not shaped like that.
corpus_209284198_600x300x22corpus_26454463_564x160x31corpus_95707047_208x153x83corpus_65311813_256x256x34corpus_12522025_2151x504_2dcorpus_201022999_211x201x11corpus_95707047alone yields six regions from two pixel classes — five separate cytosol bodies.Nothing in the synthetic set produces that, and multi-region-per-pixel-class is exactly the case that
region linking and merging has to get right.
The one design decision worth reviewing
These rebuild surfaces rather than pinning what the document already carries.
A stored model has a
<SurfaceDescription>;XmlReaderapplies it, soprecomputeAllskipsupdateAll()on parse and the geometry arrives with regions restored from the file rather thancomputed (this is the correction recorded in #2025 §4.1). Pinning those would test the XML reader
instead of surface generation — the opposite of the point.
fromCorpus()therefore callsupdateAll()to force a freshRegionImageandSurfaceCollection.Naming: they are not actually slow
You asked for these as slower fixtures, and I want to be straight that the label overstates it —
2.87 s for all six. They could sit in
Fastand run on every push instead of only in theregression lane, which would be strictly better protection. I kept them separate for headroom: they
depend on the corpus resources, and the obvious way to extend them is to add more models. Easy to
promote if you'd rather.
CI wiring — the part that would have silently done nothing
regression.ymldrives its matrix from an explicit group table, so a tag that isn't listed is neverselected: the tests would have compiled and never run. Added
Geometry_ITas a single shard.Verified by executing the workflow's embedded matrix script directly rather than trusting the
edit — 16 entries across 7 groups with
Geometry_ITpresent, and the YAML parses.Verification
-Dgroups=Geometry_IT→ 7/7 (3.87 s)vcell-coreFastThe middle row is the experiment repeated for real models: goldens generated by the old
implementation, matched exactly by current master.
Also in here
Golden regeneration is now split, so iterating on the synthetic fixtures doesn't silently leave the
corpus goldens stale or spend minutes rebuilding them —
GeometrySurfaceGolden.mainwith noargument does both, with
fastdoes only the synthetic set. And the comparison logic is sharedbetween the two test classes rather than duplicated, including the
everyFixtureHasAGoldenguard,since a
@TestFactorywhose resources went missing would otherwise just produce fewer tests andstay green.
Refs #2021, #2025, #2028
🤖 Generated with Claude Code
https://claude.ai/code/session_018kr8SbzXtwW3gMVUgMfDDt