Skip to content

Commit 6d2fbfc

Browse files
committed
fix(augment): reject CLAHE clip_limit sequences albumentations rejects
A one-element sequence is not a scalar. Albumentations validates clip_limit as a float or an exact 2-tuple and raises on [4.0], while this helper read it through _as_range and turned it into (1.0, 4.0). That accepted a config the CPU backend refuses, which is the divergence the helper exists to remove. Also annotates the parametrized arguments and splits the cross-backend comparison into independent cases, so one failure no longer hides the rest.
1 parent 605ea6d commit 6d2fbfc

2 files changed

Lines changed: 46 additions & 14 deletions

File tree

src/rfdetr/datasets/kornia_transforms.py

Lines changed: 16 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -602,22 +602,33 @@ def _as_clahe_clip_limit(value: Any) -> tuple[float, float]:
602602
distribution rather than the units.
603603
604604
Args:
605-
value: A scalar, a 1-element sequence, or a 2-element ``(min, max)`` pair.
605+
value: A scalar, or a 2-element ``(min, max)`` pair.
606606
607607
Returns:
608608
The ``(min, max)`` pair Albumentations would sample from for the same config.
609609
610+
Raises:
611+
ValueError: If *value* is a sequence of any length other than two. Albumentations validates ``clip_limit`` as
612+
either a float or an exact 2-tuple and rejects ``[4.0]``, so accepting it here (as a scalar, via
613+
:func:`_as_range`) would make the same config train on the GPU backend and fail on the CPU one. The point
614+
of this helper is that the two agree.
615+
610616
Examples:
611617
>>> _as_clahe_clip_limit(4.0)
612618
(1.0, 4.0)
613619
>>> _as_clahe_clip_limit((2.0, 6.0))
614620
(2.0, 6.0)
615621
"""
616-
if isinstance(value, (list, tuple)) and len(value) == 2:
622+
if isinstance(value, (list, tuple)):
623+
if len(value) != 2:
624+
raise ValueError(
625+
"CLAHE clip_limit must be a scalar or a 2-element (min, max) pair; "
626+
f"got a {len(value)}-element sequence: {value!r}. Albumentations rejects this too, so the CPU "
627+
"(albumentations) backend would fail on the same config."
628+
)
617629
return (float(value[0]), float(value[1]))
618-
low, high = _as_range(value)
619-
# A scalar (or 1-element sequence) arrives here as the degenerate (v, v).
620-
return (1.0, high) if low == high else (low, high)
630+
# A scalar means the range (1, v), which is what `to_tuple(v, low=1)` produces.
631+
return (1.0, float(value))
621632

622633

623634
def _make_clahe(params: dict[str, Any]) -> Any:

tests/datasets/test_kornia_transforms.py

Lines changed: 30 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -426,7 +426,9 @@ def test_clahe_maps_both_parameters(self):
426426
],
427427
ids=["default", "scalar-default-value", "scalar", "pair", "pair-non-default"],
428428
)
429-
def test_clahe_scalar_clip_limit_is_a_range_not_a_fixed_value(self, configured, expected) -> None:
429+
def test_clahe_scalar_clip_limit_is_a_range_not_a_fixed_value(
430+
self, configured: float | tuple[float, float] | None, expected: tuple[float, float]
431+
) -> None:
430432
"""Albumentations reads a scalar clip_limit as (1, v), so the GPU path must too.
431433
432434
Passing it through `_as_range` produced the degenerate (v, v), which pins every sample to maximum contrast
@@ -441,20 +443,39 @@ def test_clahe_scalar_clip_limit_is_a_range_not_a_fixed_value(self, configured,
441443

442444
assert tuple(transform.clip_limit) == pytest.approx(expected)
443445

444-
def test_clahe_clip_limit_matches_albumentations(self) -> None:
446+
@pytest.mark.parametrize(
447+
"configured",
448+
[4.0, 2.0, (2.0, 6.0)],
449+
ids=["scalar-default-value", "scalar", "pair"],
450+
)
451+
def test_clahe_clip_limit_matches_albumentations(self, configured: float | tuple[float, float]) -> None:
445452
"""The contract stated directly: same config, same range on both backends."""
446453
albumentations = pytest.importorskip("albumentations")
447454

448455
from rfdetr.datasets.kornia_transforms import build_kornia_pipeline
449456

450-
for configured in (4.0, 2.0, (2.0, 6.0)):
451-
pipeline = build_kornia_pipeline({"CLAHE": {"clip_limit": configured}}, 560)
452-
transform = next(iter(pipeline.children()))
453-
cpu = albumentations.CLAHE(clip_limit=configured)
457+
pipeline = build_kornia_pipeline({"CLAHE": {"clip_limit": configured}}, 560)
458+
transform = next(iter(pipeline.children()))
459+
cpu = albumentations.CLAHE(clip_limit=configured)
454460

455-
assert tuple(transform.clip_limit) == pytest.approx(tuple(cpu.clip_limit)), (
456-
f"backends disagree for clip_limit={configured!r}"
457-
)
461+
assert tuple(transform.clip_limit) == pytest.approx(tuple(cpu.clip_limit)), (
462+
f"backends disagree for clip_limit={configured!r}"
463+
)
464+
465+
@pytest.mark.parametrize("configured", [[4.0], (4.0,), (1.0, 2.0, 3.0)], ids=["one-list", "one-tuple", "three"])
466+
def test_clahe_rejects_sequences_that_albumentations_rejects(
467+
self, configured: tuple[float, ...] | list[float]
468+
) -> None:
469+
"""A one-element sequence is not a scalar.
470+
471+
Albumentations validates `clip_limit` as a float or an exact 2-tuple and raises on `[4.0]`. Reading it as a
472+
scalar here would accept a config the CPU backend refuses, which is the divergence this helper exists to
473+
remove.
474+
"""
475+
from rfdetr.datasets.kornia_transforms import build_kornia_pipeline
476+
477+
with pytest.raises(ValueError, match="2-element"):
478+
build_kornia_pipeline({"CLAHE": {"clip_limit": configured}}, 560)
458479

459480
def test_hue_saturation_value_still_unsupported(self):
460481
"""Deliberately out of scope: albumentations shifts additively, Kornia scales multiplicatively."""

0 commit comments

Comments
 (0)