Convert torchvision ColorJitter hue to degrees for fn.color_twist

[Kayvan-Zahiri]

Category:

Bug fix (non-breaking change which fixes an issue)

Description:

ColorJitter._kernel forwards the torchvision hue factor straight into
fn.color_twist(hue=...). torchvision expresses hue as a fraction of a full turn
(|hue| <= 0.5), but the ColorTwist hue argument is a delta in degrees
(color_twist.cc: "Hue change, in degrees."; color_twist.h: h_rad = hue * M_PI / 180).
The jitter is therefore 360x too small, and the largest legal value, hue=0.5, rotates
by half a degree instead of 180.

Measured on the three db/single/jpeg/113 images the tests already use, comparing the
median HSV hue shift against torchvision.transforms.v2.functional.adjust_hue:

hue=+0.05   torchvision +16.88   before  +0.00   after +15.47
hue=+0.10   torchvision +35.16   before  +0.00   after +29.53
hue=-0.10   torchvision -36.56   before  +0.00   after -49.22

Additional information:

Affected modules and functionalities:

ColorJitter in experimental/torchvision/v2/color.py. The sampled range is converted
to degrees once in __init__; the public self.hue stays in torchvision units so
validation is unchanged.

Key points relevant for the review:

DALI rotates hue linearly in YIQ, so its drift from torchvision's HSV shift grows with the
angle rather than staying under a fixed number of degrees. The tolerance is therefore half
the requested rotation. Measured against ref_color_twist from test_color_twist.py, the
drift peaks at 23.9 degrees for a 90 degree rotation but stays under 35% of the rotation
everywhere.

Tests:

• New tests added
  • Python tests

test_colorjitter_hue_rotation compares against adjust_hue at hue of +-0.05, +-0.1,
+-0.25 and +-0.5, on cpu and gpu. It passes on all 21 image and hue cases with the
conversion and fails on all 21 without it, since the unconverted value rotates by 0 and the
error is then the full requested angle. test_colorjitter_images only ran the pipeline and
could not catch this. The other test_tv_color.py cases are unaffected: the same
ValueErrors are raised for invalid parameters and the no-jitter path still passes
hue=-0.0.

Checklist

Documentation

• Existing documentation applies

DALI team only

Requirements

• N/A

REQ IDs: N/A

JIRA TASK: N/A

🤖 Generated with Claude Code

[copy-pr-bot]

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

[greptile-apps]

Greptile Summary

This PR converts torchvision ColorJitter hue fractions to degrees before forwarding them to DALI’s color-twist operator.

• Documents the YIQ-versus-HSV behavior difference.
• Adds CPU and GPU comparisons against torchvision across positive, negative, and boundary hue values.
• Adds coverage for sampling from a converted hue range.

Confidence Score: 5/5

The PR appears safe to merge.

No blocking failure remains.

Important Files Changed

Filename	Overview
dali/python/nvidia/dali/experimental/torchvision/v2/color.py	Converts validated hue ranges from fractions of a turn to degrees before sampling and invoking color_twist.
dali/test/python/torchvision/test_tv_color.py	Adds reference-based CPU/GPU regression tests for fixed and ranged hue rotations.

_{Reviews (4): Last reviewed commit: "Address review: drop the cached hue, tig..." | Re-trigger Greptile}

[greptile-apps]

Hue edge cases remain uncovered

The new regression test covers both devices but only fixed interior hue values on the existing JPEG inputs. Add the required ±0.5 boundaries, empty input, and explicit batch_size=1 coverage so failures at the ±180-degree conversion and required input shapes do not pass undetected.

Rule Used: New operator tests must cover: empty input, single... (source)

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

[Kayvan-Zahiri]

Added the boundary cases, but the assertion needed fixing first or they would have failed.

hue=±0.5 maps to ±180 degrees, and median_hue_shift wraps its result into
(-180, 180]. Right at 180 that wrap is a discontinuity, so two implementations agreeing
to within a degree can report opposite signs. Measured on a synthetic image with the
helper from this PR:

rotate 178.0 deg -> median_hue_shift reports   178.59
rotate 179.0 deg -> median_hue_shift reports   178.59
rotate 180.0 deg -> median_hue_shift reports  -179.30
rotate 181.0 deg -> median_hue_shift reports  -178.59

So with the old abs(actual - expected), a reference at 179 degrees and a DALI result at
181 degrees gives abs(-178.59 - 178.59) = 357.19 against a 15 degree tolerance. The test
would have failed on a true error of 2 degrees.

The assertion now compares the shorter way around, and hue gains ±0.25 and ±0.5, so
seven values across cpu and gpu. The circular form does not weaken it: a genuine 90 or
180 degree disagreement still errors at 90 and 180 respectively.

On the other two suggestions, I think the rule is aimed at new operators and this PR is a
unit conversion in the torchvision compat wrapper, so I left them alone. Channel-count and
batch coverage for ColorJitter already live in this file (test_colorjitter_channels uses
cartesian_params((1, 3), ("cpu", "gpu")), and the shape tests run batch_size = 4), and
an empty input never reaches _kernel through the torchvision Compose path being tested
here. Happy to add either if a maintainer disagrees.

flake8 is clean on the file. The DALI side of the test runs in CI, since DALI needs Linux
and CUDA and I verified only the pure-numpy helper locally.

[Kayvan-Zahiri]

Correcting my last comment. I said the DALI side of the boundary cases only runs in CI, so
I went and checked them numerically instead. The 15 degree tolerance would have failed at
hue=+-0.25.

DALI rotates hue linearly in YIQ, so its drift from torchvision's HSV shift grows with the
angle. Standing the operator in with ref_color_twist from
dali/test/python/operator_1/test_color_twist.py, on the three images this test already
uses:

requested	worst error vs torchvision	share of the rotation
18 deg (hue=0.05)	4.22	23%
36 deg (hue=+-0.1)	12.66	35%
90 deg (hue=+-0.25)	23.91	27%
180 deg (hue=+-0.5)	12.66	7%

So the error is bounded as a share of the rotation, not as a fixed angle, and the tolerance
is now half the requested rotation. Over all 21 image and hue cases it passes with the
degree conversion and fails on all 21 without it, because the unconverted value rotates by
0 and the error is then the full requested angle. Tightest passing margin is 4.78 degrees.

flake8 and black are clean on both changed files.

[JanuszL]

!build

[JanuszL]

@Kayvan-Zahiri thank you for your contribution.

[dali-automaton]

CI MESSAGE: [65990723]: BUILD STARTED

[dali-automaton]

CI MESSAGE: [65990723]: BUILD PASSED

[jantonguirao]

[Minor] The fix itself is right (color_twist documents hue as degrees, h_rad = hue * M_PI / 180), but caching the converted value in a second attribute duplicates state that can silently go stale: anything that reassigns self.hue after construction (subclass, test helper, user code poking at the public attribute) will keep using the __init__-time _hue_degrees. Since _kernel is the only consumer, consider dropping the extra attribute and converting at the point of use:

hue_degrees = tuple(h * 360.0 for h in self.hue)
brightness, contrast, saturation, hue = _get_BrightnessContrastSaturationHue(
    self.brightness, self.contrast, self.saturation, hue_degrees, fn.random.uniform
)

Same comment, one attribute, and self.hue stays the single source of truth.

[jantonguirao]

[Minor] Now that hue actually rotates, the class docstring above (the hue parameter description) is worth a sentence noting that DALI's color_twist rotates hue linearly in YIQ, so results are close to but not identical to torchvision.transforms.v2.functional.adjust_hue's HSV rotation — your own measurements show up to ~24 degrees of drift at a 90 degree rotation. Users migrating from torchvision and diffing outputs will hit this, and the docstring is the place they'll look.

[jantonguirao]

[Moderate] A tolerance of 0.5 * requested is very loose and, more importantly, scales with the thing being measured — so it can never detect a unit error smaller than 1.5x. At hue=0.5 it permits a 90 degree disagreement; at hue=0.05 it permits 9 degrees on an 18 degree rotation. It catches the 360x bug this PR fixes, but it would happily pass a future regression that, say, used 2*pi instead of 360 in some path, or lost the sign for one device.

Two suggestions, either is fine:

• Bound the tolerance with an absolute floor plus a smaller relative term, e.g. max(4.0, 0.35 * requested) — you already measured the worst-case YIQ-vs-HSV drift at 35%, so 0.35 with a floor is both tighter and directly justified by the data.
• Better: pin the exact conversion against DALI's own reference. ref_color_twist in test_color_twist.py models what fn.color_twist should do; comparing ColorJitter(hue=h) to ref_color_twist(..., hue=h*360) admits a tight per-pixel tolerance and locks the unit conversion precisely, leaving the torchvision comparison as a loose sanity check.

[jantonguirao]

[Moderate] Every case uses hue=(h, h), which takes the hue[0] == hue[1] short-circuit in _get_BrightnessContrastSaturationHue and passes a Python scalar to color_twist. The random branch — fn.random.uniform(range=hue_degrees) — is the one that actually consumes the converted range, and it is never exercised here. That's the path most users hit, since ColorJitter(hue=0.1) expands to (-0.1, 0.1).

Worth adding one case with a genuine range, e.g. hue=(0.1, 0.2), asserting the measured shift lands inside [36 - tol, 72 + tol] degrees. That also guards against a future change converting only one endpoint.

[jantonguirao]

[Minor] If colorful ever selects nothing (a near-gray input added to test_files later), np.median on the empty slice emits a RuntimeWarning and returns nan; hue_error(nan, ...) < tol is then False and the test fails with a message full of nan rather than saying what went wrong. A one-line guard — assert colorful.any(), f"{before} has no colorful pixels to measure hue on" — makes that failure self-explanatory.

[Kayvan-Zahiri]

All five are fair. Addressed in 0f78dcd.

Cached hue. Dropped _hue_degrees and convert in _kernel, exactly as you wrote it.
self.hue is the single source of truth again and reassigning it can no longer leave
stale state behind.

Tolerance. Took max(4.0, 0.35 * requested). You are right that a purely relative
bound can never catch a unit error smaller than 1.5x, which is the interesting class of
regression here.

I tried to check 0.35 independently rather than take it on trust. I modeled both rotations
directly, the YIQ linear rotation color_twist performs and the HSV rotation
adjust_hue performs, and measured the drift the test measures across a hue sweep,
uniform random pixels, and a natural-ish normal distribution:

  hue   req deg           image    drift   ratio
 0.05        18   uniform random     0.00    0.00
  0.1        36        hue sweep     1.41    0.04
 0.25        90   uniform random     5.62    0.06
  0.5       180        hue sweep     1.41    0.01

Worst ratio 0.06, so 0.35 has real headroom on that input. Caveat worth stating: this is my
model of the two rotations, not fn.color_twist itself, and it is not the three jpegs from
DALI_EXTRA, which I do not have locally. If the snail images drift harder than my synthetic
set, the floor is the part that will bite first at small angles.

Range branch. Added test_colorjitter_hue_range_is_converted with hue=(0.1, 0.2),
asserting the measured shift lands in [36 - tol, 72 + tol]. You were right that this was
the gap that mattered: every existing case passes hue=(h, h), which short-circuits to a
scalar, so fn.random.uniform(range=hue_degrees) was never exercised, and that is the path
ColorJitter(hue=0.1) takes.

Empty mask. Added the assert, so a near-gray input fails saying so instead of
propagating a nan.

Docstring. Noted the YIQ versus HSV difference and that the gap grows with the angle.

One process note: my first push of this had two blank-line findings, E303 and E305, from
the inserted test. Amended, and flake8 is now clean on both files where the pristine file
was not.