Skip to content

Convert torchvision ColorJitter hue to degrees for fn.color_twist #6471

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
Kayvan-Zahiri wants to merge 4 commits into
base: main
Choose a base branch
from
+69 −1
Open

Convert torchvision ColorJitter hue to degrees for fn.color_twist #6471

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
c9971ca
Convert torchvision ColorJitter hue to degrees for fn.color_twist
Kayvan-Zahiri Sep 2, 2026
f61ac18
Cover the hue boundary and compare rotations circularly
Kayvan-Zahiri Sep 2, 2026
aedaeb4
Scale the hue parity tolerance with the requested rotation
Kayvan-Zahiri Sep 2, 2026
0f78dcd
Address review: drop the cached hue, tighten the bound, cover the ran…
Kayvan-Zahiri Sep 3, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 5 additions & 1 deletion dali/python/nvidia/dali/experimental/torchvision/v2/color.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -208,12 +208,16 @@ def __init__(
if isinstance(hue, (int, float)):
self.hue = (-float(hue), float(hue))

# torchvision expresses hue as a fraction of a full turn (|hue| <= 0.5), while
# fn.color_twist takes the hue delta in degrees.
self._hue_degrees = tuple(float(h) * 360.0 for h in self.hue)

@jantonguirao jantonguirao Sep 3, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

[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.


def _kernel(self, data_input):
"""
Performs the color jitter using the ``fn.color_twist`` operator.
"""
brightness, contrast, saturation, hue = _get_BrightnessContrastSaturationHue(
self.brightness, self.contrast, self.saturation, self.hue, fn.random.uniform
self.brightness, self.contrast, self.saturation, self._hue_degrees, fn.random.uniform

@jantonguirao jantonguirao Sep 3, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

[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.

)

data_input = fn.color_twist(
Expand Down
30 changes: 30 additions & 0 deletions dali/test/python/torchvision/test_tv_color.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@

import os

import numpy as np
from nose2.tools import params, cartesian_params
from nose_utils import assert_raises
from PIL import Image
Expand Down Expand Up @@ -149,6 +150,35 @@ def test_colorjitter_images(cj_params, device):
_ = cj(img)


def median_hue_shift(before: Image.Image, after: Image.Image) -> float:
"""Median hue rotation from `before` to `after`, in degrees, over the colorful pixels."""
hue_before, saturation, _ = before.convert("HSV").split()
hue_after, _, _ = after.convert("HSV").split()
to_degrees = 360.0 / 256.0
hue_before = np.asarray(hue_before, dtype=np.float64) * to_degrees
hue_after = np.asarray(hue_after, dtype=np.float64) * to_degrees
# hue is meaningless for near-gray pixels
colorful = np.asarray(saturation) > 32

@jantonguirao jantonguirao Sep 3, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

[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.

shift = (hue_after - hue_before + 180.0) % 360.0 - 180.0
return float(np.median(shift[colorful]))


@cartesian_params((0.05, 0.1, -0.1), ("cpu", "gpu"))

@greptile-apps greptile-apps Bot Sep 2, 2026

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.

P2 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!

def test_colorjitter_hue_rotation(hue, device):
# torchvision expresses hue as a fraction of a full turn, fn.color_twist takes degrees.
# The tolerance covers DALI's linear YIQ approximation of the hue rotation.
cj = Compose([ColorJitter(hue=(hue, hue), device=device)])

@jantonguirao jantonguirao Sep 3, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

[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.


for fn in test_files:
img = Image.open(fn).convert("RGB")
expected = median_hue_shift(img, transforms.functional.adjust_hue(img, hue))
actual = median_hue_shift(img, cj(img))
assert abs(actual - expected) < 15.0, (
f"hue={hue} rotated by {actual:.2f} degrees, torchvision rotates by "
f"{expected:.2f} degrees: {fn}"
)


"""
TODO (https://github.com/NVIDIA/DALI/issues/DALI-4656):
DALI ColorJitter does not currently work on CHW layout
Expand Down