Skip to content

Commit 147aa93

Browse files
wrignj08claude
andcommitted
Fix one-pixel shift when smooth_edge_size is even
cv2.morphologyEx(MORPH_OPEN) applies a single anchor to both the erosion and the dilation it performs. That is only correct when the anchor lands on the structuring element's centre of symmetry: true for odd kernel sizes, false for even ones. For even smooth_edge_size the result was the correct opening translated one pixel down and to the right, and was not anti-extensive -- smoothing could add pixels to a class rather than only removing them. It was most visible at smooth_edge_size=2, where the opening is otherwise a no-op on a solid blob so the shift was the only effect. Run the erosion and dilation separately, anchoring the dilation at the reflection of the erosion's anchor. For odd sizes the two anchors coincide and output is bit-identical to before, including border handling; the Landsat regression fixture (smooth_edge_size=3) is unchanged. The dilation writes back into the erosion's buffer, as morphologyEx does internally. Allocating a second full-size buffer instead costs more than the morphology itself at small kernels -- 2.6x on a 4000x4000 mask at smooth_edge_size=2, where allocation dominates the trivial compute. As written, runtime and peak memory match the previous implementation. This changes output for even smooth_edge_size, including the default of 2. Tests: pin the opening against an independent implementation of the definition, exact at borders as well as in the interior, and cover the even sizes the parameter sweeps previously skipped (SWEEP_SMOOTH was [0, 1, 3]). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 parent ae8922a commit 147aa93

3 files changed

Lines changed: 283 additions & 4 deletions

File tree

CHANGELOG.md

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,21 @@ All notable changes to MultiClean are documented here.
44

55
## [Unreleased]
66

7+
### Fixed
8+
- Edge smoothing no longer translates the output by one pixel down and to the
9+
right when `smooth_edge_size` is even. `cv2.morphologyEx(MORPH_OPEN)` applies
10+
a single anchor to both the erosion and the dilation, which is only correct
11+
when that anchor coincides with the structuring element's centre of symmetry
12+
— true for odd kernel sizes, false for even ones. The result was a shifted
13+
opening that could also add pixels to a class rather than only removing them.
14+
The erosion and dilation are now run separately with the dilation anchored at
15+
the reflection of the erosion's anchor, giving a true opening at every size.
16+
17+
**This changes output for even `smooth_edge_size`, including the default of
18+
`2`.** Output for odd values is bit-identical to previous releases. If you
19+
have cached results produced with an even `smooth_edge_size`, regenerate
20+
them or expect a one-pixel offset against new output.
21+
722
## [0.4.0] - 2026-07-28
823

924
### Changed

multiclean/utils.py

Lines changed: 29 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -93,12 +93,39 @@ def smooth_edges_to_codes(
9393
kernel = create_circle_kernel(smooth_edge_size)
9494
codes = np.zeros(array.shape, dtype=code_dtype)
9595

96+
# A true opening dilates with the *reflected* structuring element. cv2's
97+
# ``morphologyEx(MORPH_OPEN)`` reuses one anchor for both passes, which is
98+
# self-correcting only when the anchor sits on the element's centre of
99+
# symmetry -- true for odd ``smooth_edge_size``, false for even, where it
100+
# translated the result down and right by one pixel (and so was not even
101+
# anti-extensive). Anchoring the erosion at cv2's default and the dilation
102+
# at its reflection removes the shift; for odd sizes the two anchors
103+
# coincide and output is bit-identical to the previous implementation.
104+
erode_anchor = smooth_edge_size // 2
105+
dilate_anchor = smooth_edge_size - 1 - erode_anchor
106+
kernel_reflected = np.ascontiguousarray(kernel[::-1, ::-1])
107+
96108
def _opened_for_class(cv_) -> Tuple[object, np.ndarray]:
97109
# bool storage is 1 byte/element so ``.view(np.uint8)`` is a zero-
98110
# copy reinterpretation -- avoids the bool→uint8 astype copy.
99111
class_mask_u8 = (array == cv_).view(np.uint8)
100-
opened_u8 = cv2.morphologyEx(
101-
class_mask_u8, cv2.MORPH_OPEN, kernel, iterations=1
112+
eroded_u8 = cv2.erode(
113+
class_mask_u8,
114+
kernel,
115+
anchor=(erode_anchor, erode_anchor),
116+
iterations=1,
117+
)
118+
# Dilate back into the erosion's buffer, which is what
119+
# ``morphologyEx(MORPH_OPEN)`` does internally. Allocating a second
120+
# full-size buffer instead costs more than the morphology itself at
121+
# small kernel sizes. ``eroded_u8`` is local to this call, so this
122+
# stays safe under the thread pool below.
123+
opened_u8 = cv2.dilate(
124+
eroded_u8,
125+
kernel_reflected,
126+
dst=eroded_u8,
127+
anchor=(dilate_anchor, dilate_anchor),
128+
iterations=1,
102129
)
103130
return cv_, opened_u8.view(bool)
104131

tests/test_multiclean.py

Lines changed: 239 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
import pytest
55

66
from multiclean import clean_array
7+
from multiclean.utils import create_circle_kernel, smooth_edges_to_codes
78

89
TEST_DATA_DIR = Path(__file__).resolve().parent / "data"
910
LANDSAT_INPUT = TEST_DATA_DIR / "Landsat cloud and cloud shadow.tif"
@@ -100,6 +101,239 @@ def test_smoothing_removes_single_pixel_when_enabled():
100101
assert out[2, 2] == 0
101102

102103

104+
def _centred_blob(size: int = 24, half_width: int = 6) -> np.ndarray:
105+
"""Solid square of class 1 centred in a background-0 array.
106+
107+
The array is symmetric under a 180-degree rotation, which the smoothing
108+
kernels are too, so any correct opening must preserve that symmetry.
109+
"""
110+
arr = np.zeros((size, size), dtype=np.uint8)
111+
centre = (size - 1) / 2
112+
lo, hi = int(centre - half_width + 1), int(centre + half_width + 1)
113+
arr[lo:hi, lo:hi] = 1
114+
return arr
115+
116+
117+
@pytest.mark.parametrize("smooth_edge_size", [1, 2, 3, 4, 5, 6, 7, 8])
118+
def test_smoothing_does_not_translate_blobs(smooth_edge_size):
119+
# Regression: cv2's MORPH_OPEN applies one anchor to both the erosion and
120+
# the dilation, which is only correct when the structuring element's centre
121+
# of symmetry lands on that anchor. For even kernel sizes it does not, and
122+
# the whole blob came back shifted one pixel down and right -- most obvious
123+
# at smooth_edge_size=2, where the opening should otherwise be a no-op on a
124+
# solid blob.
125+
arr = _centred_blob()
126+
out = clean_array(
127+
arr,
128+
class_values=[0, 1],
129+
smooth_edge_size=smooth_edge_size,
130+
min_island_size=0,
131+
connectivity=4,
132+
max_workers=1,
133+
)
134+
135+
# The blob is far larger than any kernel here, so it must survive with its
136+
# bounding box unmoved (a circular opening rounds the corners off a square
137+
# but leaves the edge midpoints, so the extent is unchanged).
138+
ys, xs = np.where(out == 1)
139+
in_ys, in_xs = np.where(arr == 1)
140+
assert (ys.min(), ys.max()) == (in_ys.min(), in_ys.max())
141+
assert (xs.min(), xs.max()) == (in_xs.min(), in_xs.max())
142+
143+
144+
@pytest.mark.parametrize("smooth_edge_size", [1, 2, 3, 4, 5, 6, 7, 8])
145+
def test_smoothing_preserves_symmetry(smooth_edge_size):
146+
# Sharper form of the same regression: the kernels are all symmetric under
147+
# a 180-degree rotation, so a symmetric input must smooth to a symmetric
148+
# result. A one-pixel translation in either axis breaks this even when the
149+
# bounding box survives.
150+
#
151+
# Asserted against the smoothing stage rather than clean_array because the
152+
# nearest-neighbour fill that follows it breaks ties between equidistant
153+
# source pixels arbitrarily, which is asymmetric by design.
154+
arr = _centred_blob()
155+
codes, _ = smooth_edges_to_codes(
156+
arr,
157+
smooth_edge_size=smooth_edge_size,
158+
target_class_values=[1],
159+
background_class_values=[0],
160+
all_class_values=[0, 1],
161+
max_workers=1,
162+
)
163+
assert np.array_equal(codes, codes[::-1, ::-1])
164+
165+
166+
@pytest.mark.parametrize("smooth_edge_size", [1, 2, 3, 4, 5, 6, 7, 8])
167+
def test_smoothing_never_grows_a_class(smooth_edge_size):
168+
# An opening is anti-extensive: it can only remove pixels from a class,
169+
# never add them. The even-size anchor bug broke this -- the shifted blob
170+
# covered pixels that were background in the input. Checked against a
171+
# ragged multiclass array so it is not just the solid-blob case.
172+
arr = np.zeros((40, 40), dtype=np.uint8)
173+
arr[5:20, 5:20] = 1
174+
arr[22:36, 22:36] = 1
175+
arr[10:14, 24:30] = 2
176+
arr[30, 3] = 1 # thin spur that smoothing is expected to erase
177+
178+
out = clean_array(
179+
arr,
180+
class_values=[1, 2],
181+
smooth_edge_size=smooth_edge_size,
182+
min_island_size=0,
183+
connectivity=4,
184+
max_workers=1,
185+
)
186+
187+
# Every pixel that came out as a smoothed class must have held that class
188+
# on input; fill may reassign a pixel to another class, but smoothing must
189+
# not extend one beyond its original footprint.
190+
for cv in (1, 2):
191+
grew = (out == cv) & (arr != cv)
192+
# Fill can only draw from surviving neighbours, so any growth here is
193+
# the smoothing step inventing coverage.
194+
assert not grew.any(), f"class {cv} grew by {int(grew.sum())} pixels"
195+
196+
197+
def _reference_opening(mask: np.ndarray, kernel: np.ndarray, anchor: int) -> np.ndarray:
198+
"""Morphological opening straight from the definition, for small arrays.
199+
200+
``A opened by B`` is the union of every translate of ``B`` that fits
201+
entirely inside ``A``. Written out as an explicit slide so it shares no
202+
machinery with the cv2 erode/dilate pair under test.
203+
204+
Two details model cv2's finite-image behaviour, so this is exact at the
205+
borders and not just in the interior:
206+
207+
* Pixels outside the image read as foreground, matching the border value
208+
cv2 uses for erosion -- content is not eaten away merely because the
209+
image ends.
210+
* Translate positions are restricted to the image domain, because that is
211+
the domain cv2's intermediate erosion is defined on.
212+
213+
The second point is why ``anchor`` has to be named: it fixes where a
214+
translate sits relative to the position that must stay in-domain. Away
215+
from the border the choice cannot matter (a translated structuring element
216+
is still the same set of pixels), and the interior test below asserts
217+
exactly that. Within one kernel width of the border it does matter, so the
218+
border test passes cv2's own anchor.
219+
"""
220+
ks = kernel.shape[0]
221+
height, width = mask.shape
222+
offsets = [(i - anchor, j - anchor) for i, j in np.argwhere(kernel > 0)]
223+
224+
# Foreground-padded view, wide enough that no translate can run off it.
225+
extended = np.ones((height + 2 * ks, width + 2 * ks), dtype=np.uint8)
226+
extended[ks : ks + height, ks : ks + width] = mask
227+
228+
out = np.zeros_like(mask)
229+
for row in range(height):
230+
for col in range(width):
231+
if all(extended[row + ks + dr, col + ks + dc] for dr, dc in offsets):
232+
for dr, dc in offsets:
233+
r, c = row + dr, col + dc
234+
if 0 <= r < height and 0 <= c < width:
235+
out[r, c] = 1
236+
return out
237+
238+
239+
def _smoothed_mask(arr: np.ndarray, smooth_edge_size: int) -> np.ndarray:
240+
"""Run the smoothing stage alone and return class 1's mask."""
241+
codes, code_to_value = smooth_edges_to_codes(
242+
arr,
243+
smooth_edge_size=smooth_edge_size,
244+
target_class_values=[1],
245+
background_class_values=[],
246+
all_class_values=[0, 1],
247+
max_workers=1,
248+
)
249+
return (code_to_value[codes] == 1).astype(np.uint8)
250+
251+
252+
@pytest.mark.parametrize("smooth_edge_size", [1, 2, 3, 4, 5, 6, 7, 8])
253+
def test_smoothing_matches_reference_opening(smooth_edge_size):
254+
# The other smoothing tests assert properties (no shift, symmetric,
255+
# anti-extensive). Properties alone cannot distinguish a correct opening
256+
# from a differently-wrong one -- silently rounding even kernel sizes up to
257+
# odd, for instance, satisfies every one of them while changing how much
258+
# smoothing the caller actually asked for. This pins the exact result
259+
# against an independent implementation of the definition instead.
260+
rng = np.random.default_rng(7)
261+
kernel = create_circle_kernel(smooth_edge_size)
262+
263+
for _ in range(10):
264+
arr = np.zeros((34, 34), dtype=np.uint8)
265+
arr[9:25, 9:25] = rng.random((16, 16)) > 0.35 # wide zero margin
266+
267+
expected = _reference_opening(arr, kernel, anchor=smooth_edge_size // 2)
268+
assert np.array_equal(_smoothed_mask(arr, smooth_edge_size), expected)
269+
270+
# Away from the border the reference must not depend on how the
271+
# structuring element is anchored. This is the no-shift property
272+
# restated at the definition level, and it keeps the assertion above
273+
# from silently inheriting the implementation's anchor convention.
274+
for alt_anchor in (0, smooth_edge_size - 1):
275+
assert np.array_equal(
276+
_reference_opening(arr, kernel, anchor=alt_anchor), expected
277+
)
278+
279+
280+
@pytest.mark.parametrize("smooth_edge_size", [1, 2, 3, 4, 5, 6, 7, 8])
281+
def test_smoothing_matches_reference_opening_at_borders(smooth_edge_size):
282+
# Same equivalence, but with content running flush to all four edges, where
283+
# cv2's border convention decides the answer. Untested until now: the
284+
# interior test deliberately keeps a margin so border handling cannot
285+
# affect it, which left the edges of every real raster unpinned.
286+
rng = np.random.default_rng(11)
287+
kernel = create_circle_kernel(smooth_edge_size)
288+
289+
for _ in range(6):
290+
arr = (rng.random((20, 20)) > 0.35).astype(np.uint8)
291+
expected = _reference_opening(arr, kernel, anchor=smooth_edge_size // 2)
292+
assert np.array_equal(_smoothed_mask(arr, smooth_edge_size), expected)
293+
294+
295+
@pytest.mark.parametrize("smooth_edge_size", [1, 2, 3, 4, 5, 6, 7, 8])
296+
def test_smoothing_does_not_erode_content_at_the_image_edge(smooth_edge_size):
297+
# The human-readable half of the border contract: a band running the full
298+
# width of the image, flush against the top, left and right edges, must
299+
# come through an opening completely intact. If the erosion treated
300+
# out-of-image pixels as background it would chew a kernel-wide bite out of
301+
# all three edges, which on tiled processing would show up as seams along
302+
# every tile boundary.
303+
#
304+
# A band rather than a square block: its only boundary is the straight
305+
# edge along the bottom, which an opening preserves exactly. A block would
306+
# additionally have an interior corner, and a circular kernel rounds those
307+
# off by design -- correct behaviour that has nothing to do with borders.
308+
arr = np.zeros((24, 24), dtype=np.uint8)
309+
arr[:12, :] = 1
310+
311+
assert np.array_equal(_smoothed_mask(arr, smooth_edge_size), arr)
312+
313+
314+
@pytest.mark.parametrize("smooth_edge_size", [0, 2, 3])
315+
def test_output_is_independent_of_max_workers(smooth_edge_size):
316+
# Smoothing runs one class per thread and the opening now dilates back into
317+
# the erosion's own buffer. That is safe because the buffer is created per
318+
# call, but hoisting it out to avoid a per-class allocation would be an easy
319+
# and plausible "optimisation" -- and would corrupt results only under
320+
# concurrency, which every other test pins to a single worker count.
321+
rng = np.random.default_rng(3)
322+
arr = rng.integers(0, 6, size=(64, 64), dtype=np.uint8)
323+
arr[10:30, 10:30] = 2 # solid regions so smoothing has real work to do
324+
arr[35:60, 35:60] = 4
325+
326+
kwargs = dict(
327+
class_values=[1, 2, 3, 4, 5],
328+
smooth_edge_size=smooth_edge_size,
329+
min_island_size=6,
330+
connectivity=4,
331+
)
332+
baseline = clean_array(arr, max_workers=1, **kwargs)
333+
for workers in (2, 4, 8):
334+
assert np.array_equal(clean_array(arr, max_workers=workers, **kwargs), baseline)
335+
336+
103337
def test_island_threshold_strictness_preserves_area_equal_to_threshold():
104338
# 2-pixel island (area = 2) should be preserved when min_island_size = 2
105339
arr = np.zeros((5, 5), dtype=np.int32)
@@ -183,7 +417,7 @@ def test_class_values_absent_from_array_are_ignored():
183417
arr = np.full((64, 64), 254, dtype=np.uint8)
184418
arr[10:50, 10:50] = 1 # only class 1 (and background 254) present
185419

186-
for smooth_edge_size in (0, 3):
420+
for smooth_edge_size in (0, 2, 3):
187421
kwargs = dict(
188422
smooth_edge_size=smooth_edge_size,
189423
min_island_size=10,
@@ -206,7 +440,10 @@ def test_class_values_absent_from_array_are_ignored():
206440
# any single branch still fails the suite.
207441

208442
SWEEP_DTYPES = [np.uint8, np.int16, np.int32, np.float32]
209-
SWEEP_SMOOTH = [0, 1, 3]
443+
# Even kernel sizes are included deliberately: the anchor bug that translated
444+
# smoothed blobs by one pixel only ever fired for even ``smooth_edge_size``,
445+
# and the sweep's previous [0, 1, 3] never touched that half of the space.
446+
SWEEP_SMOOTH = [0, 1, 2, 3, 4]
210447
SWEEP_ISLAND = [0, 25]
211448

212449

0 commit comments

Comments
 (0)