Collapse redundant TilingConfig.tissue_threshold into min_coverage (#157)#158
Merged
Conversation
) tissue_threshold and min_coverage.tissue were the same knob, round-tripped through a derived TilingConfig field (min_coverage -> scalar -> back into the sampling spec / min_tissue_fraction). Remove the stored scalar: TilingConfig now carries the resolved min_coverage map (the single source of truth) and read-sites derive the tissue fraction via min_coverage.get('tissue') or 0.0. - models.py: tissue_threshold field -> min_coverage: Mapping[str, float] (field(hash=False) keeps the frozen dataclass hashable) - resolvers.py: carry min_coverage verbatim; build_default_sampling_spec reads it - orchestration.py (x3), artifacts.py: read the threshold from min_coverage - tests migrated off the scalar; benchmarking YAML moves tissue_threshold from tiling.params to tiling.masks.min_coverage (the canonical masks-only surface) io.py caches key on min_tissue_fraction (not the scalar), so existing on-disk caches still validate. Released bottom-up: hs2p -> slide2vec (#182) -> soma (#97). Closes #157
Binary tissue tiling read `min_coverage.get("tissue") or 0.0`, silently
disabling tissue filtering (keep == fractions >= 0.0 keeps every tile,
background included) when the threshold was unset. Replace the fabricated
0.0 with a `require_tissue_fraction` guard that errors on a missing tissue
entry while honouring an explicit 0.0 as a deliberate opt-out.
The guard lives in the config layer and is applied at the three binary-path
read-sites in orchestration plus build_default_sampling_spec. Annotation
sampling is unaffected: it gates on per-class coverage and never reads the
tissue default, and resolve_sampling_request now detects the untouched
default via constants rather than building the tissue-requiring default spec.
Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
clemsgrs
added a commit
to clemsgrs/slide2vec
that referenced
this pull request
Jun 24, 2026
#183) hs2p 4.2.0 (clemsgrs/hs2p#158) collapses the derived TilingConfig.tissue_threshold scalar into the resolved min_coverage map, the single source of truth (min_coverage["tissue"] is the tissue threshold). slide2vec's runtime is already masks-only and never reads the scalar; only the test suite and a stale comment referenced it. - pyproject.toml: bump hs2p pin >=4.1.2 -> >=4.2.0 - tests: assert tiling_cfg.min_coverage["tissue"] instead of the removed tiling_cfg.tissue_threshold - runtime/tiling.py: reword the resolver comment for the field-removed world Full suite green against hs2p 4.2.0 (359 passed, 15 skipped).
4 tasks
clemsgrs
added a commit
to clemsgrs/soma
that referenced
this pull request
Jun 24, 2026
…_threshold (#97) (#103) hs2p 4.2.0 (clemsgrs/hs2p#158) collapsed the derived TilingConfig.tissue_threshold scalar into the resolved min_coverage map (min_coverage["tissue"] is the tissue threshold, the single source of truth). soma's slide-manifest dense path constructed hs2p.TilingConfig(tissue_threshold=...) directly, which now raises TypeError. - dense_slide_extraction._build_tiling_config: feed min_coverage={"tissue": ...} instead of the removed scalar. The per-class sampling map still flows separately through _resolve_sampling_spec_from_masks(masks, ...); this tiling-level entry only feeds the result's binary min_tissue_fraction provenance, so ROI coordinates and the dense cache key (which never referenced TilingConfig) are unchanged. - pyproject: bump hs2p >=4.1.1 -> >=4.2.0 and slide2vec[fm] >=4.8.0 -> >=5.0.0. The bumps are coupled: slide2vec must be >=5.0.0 to run on hs2p 4.2.0 (4.8.0 still reads the removed field), and the existing >=4.8.0 floor already resolved to 5.0.0 -> hs2p>=4.2.0, so soma was already broken on a fresh install. Full suite green against hs2p 4.2.0 + slide2vec 5.0.0 (1106 passed, 3 skipped, 1 xfailed; test_sphinx_docs_build deselected for a missing docutils env dep). Closes #97
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
1 participant
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 #157.
tissue_thresholdandmin_coverage.tissuewere the same knob, round-tripped through a derivedTilingConfigfield:masks.min_coverage.tissue→ flattened intoTilingConfig.tissue_threshold→ read back into the sampling spec (tissue_percentage) andmin_tissue_fraction. That round-trip was the redundancy.Change
models.py—TilingConfig.tissue_threshold: float→min_coverage: Mapping[str, float](the resolved coverage map, the single source of truth).field(hash=False)keeps the otherwise-frozen dataclass hashable despite the mapping field.resolvers.py—resolve_tiling_configcarriesmin_coverageverbatim (no flatten);build_default_sampling_specreadsmin_coverage.get("tissue") or 0.0.orchestration.py(×3) +artifacts.pyconsistency check — derive the tissue fraction frommin_coverageat each read-site.tissue_thresholdfromtiling.paramstotiling.masks.min_coverage(the canonical masks-only surface).Notes
io.pykeys artifacts onmin_tissue_fraction, not the scalar — existing on-disk caches still validate.