Skip to content

refactor: one class per augmentation, and explicit constructor parameters - #66

Open
Hendrik-code wants to merge 1 commit into
hm/demo-scriptfrom
hm/one-class-per-augmentation
Open

refactor: one class per augmentation, and explicit constructor parameters#66
Hendrik-code wants to merge 1 commit into
hm/demo-scriptfrom
hm/one-class-per-augmentation

Conversation

@Hendrik-code

@Hendrik-code Hendrik-code commented Aug 20, 2026

Copy link
Copy Markdown
Collaborator

Stacked on #65. First of the registry-phase PRs — but it stands on its own, and no config changes.

Five augmentations behind one class name, twice over

The GPU transforms dispatched on a config string:

class dispatched on
RandomConvTransformGPU kernel_type ∈ {Laplace, Scharr, GaussianBlur, UnsharpMask, RandConv}
RandomFunctionGPU an arbitrary func callable
RandomGammaGPU invert_image

They're one class each now — RandomLaplaceGPU, RandomScharrGPU, RandomGaussianBlurGPU, RandomUnsharpMaskGPU, RandomRandConvGPU, RandomLog1pGPU, RandomSqrtGPU, RandomSinGPU, RandomExpGPU, RandomSigmoidGPU, RandomGammaGPU, RandomInvGammaGPU — sharing a private base.

This is what lets a config key be a class name later in the series. Independently of that, it means each augmentation can be constructed, tested and discovered on its own: the standalone test sweep goes from 199 to 223 subtests purely because there are now classes to discover.

**kwargs is gone (from the files this PR touches), and it was hiding a bug

Correction: RandomDomainTransferGPU still has **kwargs after this PR — domain_transfer.py is not touched here, because its other pending change (a hardcoded bank path) is unrelated to the class split. It is removed in the next PR, where registering it forces the issue.

Kernel- and function-specific parameters (absolute, sigma, unsharp_amount, kernel_sizes, mix_prob) were read out of kwargs.get(). They were invisible to inspect.signature, so a typo in a config silently selected the default. Their defaults here are the historical kwargs.get() ones.

Removing **kwargs immediately surfaced a real defect — mypy found it, not me:

error: Unexpected keyword argument "crop" for "RandomLowResTransformGPU"
error: Unexpected keyword argument "crop" for "RandomAcqTransformGPU"

The two list pipelines passed crop= to both transforms. Neither has ever accepted it; **kwargs swallowed it. 24 of the shipped configs set it, and it has never done anything. The sequential pipeline never passed it, which is exactly how the two ladders drifted apart. Dropped.

Renames, and one parameter removed

  • RandomAffine3DCustomRandomAffineGPU
  • RandomPALETTEGPURandomPaletteGPU
  • one_dim moves from a parameter of RandomAcqTransformGPU into the class.

On that last one: I checked whether it was a behaviour change and it is not. All three pipelines passed one_dim=True literally, and the class docstring already read "Randomly lower acquisition along one axes only" — so the parameter only ever offered a way to contradict the class. No config sets it.

The ladders still run

The three if ladders read kernel_type and func from the config — precisely the dispatch these leaf classes exist to remove. Rather than rewrite them here and again when they're deleted, three aliases at the bottom of contrast.py bridge them, clearly marked as temporary. They and the ladders go together later in this series.

Verification

All 24 seeded config digests are unchanged — same fixed volume, same seed, same output hash as before this PR. Full suite: 105 passed, 223 subtests. ruff and mypy clean.

…ters

The GPU transforms dispatched on a config string: RandomConvTransformGPU took
kernel_type in {Laplace, Scharr, GaussianBlur, UnsharpMask, RandConv},
RandomFunctionGPU took an arbitrary func, RandomGammaGPU took invert_image. Five
augmentations behind one class name, twice over.

They are now one class each -- RandomLaplaceGPU, RandomScharrGPU,
RandomGaussianBlurGPU, RandomUnsharpMaskGPU, RandomRandConvGPU, RandomLog1pGPU,
RandomSqrtGPU, RandomSinGPU, RandomExpGPU, RandomSigmoidGPU, RandomGammaGPU and
RandomInvGammaGPU -- sharing a private base. This is what lets a config key be a
class name later in this series; until then it also means each augmentation can
be constructed, tested and discovered on its own. The standalone test sweep goes
from 199 to 223 subtests for that reason.

**kwargs is gone from every transform constructor. Kernel- and function-specific
parameters (absolute, sigma, unsharp_amount, kernel_sizes, mix_prob) were read
out of kwargs.get(), so they were invisible to inspect.signature and a typo in a
config silently selected the default. Their defaults here are the historical
kwargs.get() ones.

Removing **kwargs immediately surfaced a real defect, via mypy:

    error: Unexpected keyword argument "crop" for "RandomLowResTransformGPU"
    error: Unexpected keyword argument "crop" for "RandomAcqTransformGPU"

The two list pipelines passed crop= to both transforms; neither has ever accepted
it, and **kwargs swallowed it. 24 of the shipped configs set it. The sequential
pipeline never passed it, which is how the two drifted. Dropped.

Also RandomAffine3DCustom -> RandomAffineGPU and RandomPALETTEGPU ->
RandomPaletteGPU, and one_dim moves from a parameter of RandomAcqTransformGPU
into the class: all three pipelines passed one_dim=True literally, and the class
docstring already said "lower acquisition along one axes only", so the parameter
only ever offered a way to contradict the class.

The three `if` ladders still run, bridged by three aliases at the bottom of
contrast.py -- they read kernel_type and func from the config, which is the
dispatch these leaves exist to remove. They and the ladders go together later in
this series; the aliases are marked as temporary.

Verified byte-for-byte: the 24 seeded config digests are unchanged.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Copilot AI lite review requested due to automatic review settings August 20, 2026 13:01

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants