feat(augment): support Perspective on the Kornia backend - #1330
Conversation
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## develop #1330 +/- ##
=======================================
Coverage 85% 85%
=======================================
Files 111 111
Lines 14066 14086 +20
=======================================
+ Hits 12006 12025 +19
- Misses 2060 2061 +1 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Pull request overview
Adds Kornia GPU backend support for Perspective.
Changes:
- Registers a
RandomPerspectivefactory with scale and size handling. - Adds tests for warnings, resolution, boxes, and unsupported crops.
- Review found semantic/API compatibility and documentation issues.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 5 comments.
| File | Description |
|---|---|
src/rfdetr/datasets/kornia_transforms.py |
Adds Kornia Perspective mapping. |
tests/datasets/test_kornia_transforms.py |
Adds Perspective factory tests. |
Suppressed comments (4)
tests/datasets/test_kornia_transforms.py:1228
- Add an explicit
-> Nonereturn annotation to this new test method.
def test_scalar_scale_does_not_warn(self):
tests/datasets/test_kornia_transforms.py:1239
- Add an explicit
-> Nonereturn annotation to this new test method.
def test_keep_size_false_is_refused_not_ignored(self):
tests/datasets/test_kornia_transforms.py:1246
- Add an explicit
-> Nonereturn annotation to this new test method.
def test_output_keeps_the_input_resolution(self):
tests/datasets/test_kornia_transforms.py:1258
- Add an explicit
-> Nonereturn annotation to this new test method.
def test_boxes_follow_the_warp(self):
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
8dd1ffd to
003a41d
Compare
|
rebased onto current the mapping was described wronglyi wrote that passing the upper bound keeps the worst case roughly aligned, which is the best available, but calling it a parameter rename was wrong. the docstring now says what it actually is, and the warning fires on every a scalar
|
`Perspective` is the one geometric name from roboflow#1252 that maps onto Kornia without changing the output resolution, which is what makes it safe to add. Albumentations' `scale` and Kornia's `distortion_scale` are the same quantity (corner displacement as a fraction of the image side), so it maps directly. A `(min, max)` pair collapses to its upper bound and logs, the same asymmetry `GaussNoise` already has: Albumentations samples a fresh value per call, Kornia takes one fixed value. A scalar stays quiet, since that is an exact request rather than a collapse. `keep_size=False` is refused rather than ignored. It changes the output resolution, and the GPU path in `on_after_batch_transfer` rebuilds the batch as `NestedTensor(img_aug, samples.mask)`, reusing the pre-augmentation padding mask. Anything that resizes the image leaves that mask describing a different shape, and `NestedTensor` does not validate the pair. That is also why the three crop names from roboflow#1252 are still unsupported: they resize by construction. A test pins that they stay rejected, so this does not get "completed" later without dealing with the mask. Six tests, five of which fail on the parent commit.
…options The scale mapping was described as direct and it is not: albumentations treats scale as a sigma and samples abs(N(0, sigma)) per corner, kornia samples uniformly from [0, distortion_scale], so the GPU path distorts more on a typical sample. Say so, and warn on every config rather than only on a non-degenerate range. Also: a scalar scale is (0, v) in albumentations, not (v, v), so the reported CPU-side range was wrong; the six unmappable options now warn instead of vanishing; and aug_configs.py no longer lists Perspective as unsupported while the registry supports it.
The table row was 134 chars and tripped ruff E501, which is what failed pre-commit.ci. Also corrects the prose, which still said the divergence is logged only for a non-degenerate range; it is logged for every config.
2e51da5 to
20b36ef
Compare
|
pushed the pre-commit failure was mine: the while fixing that i noticed the prose was stale in a way the earlier push introduced: it still said the divergence is logged for "a range whose ends differ", which was true before but isn't now that the warning fires on every config. corrected.
|
Changes: - Remove the Kornia Perspective factory and registry entry so unsafe geometric augmentation cannot reach training targets. - Document Perspective as unsupported on the Kornia backend. - Replace stochastic Perspective support tests with deterministic rejection coverage for unsupported geometry. Impact: - GPU augmentation now fails fast instead of risking stale padding-mask geometry and lossy parameter mapping. - Users receive accurate backend support documentation; maintainers get reproducible regression coverage. Verification: - pre-commit run --all-files: passed. - Focused unsupported-geometry regression: 4 passed. - uv run --no-sync pytest tests/datasets/test_kornia_transforms.py -q: 96 passed. - Full CPU suite: 3958 passed, 70 skipped, 6 failed in unrelated CoreML/ONNX export paths; formal investigation recorded the missing onnxscript dependency and known CoreML/Torch parity instability. - rtk git diff --check: passed. Residual limits: - No CUDA device is available locally. - The full CPU export gate remains red for non-causal environment/test failures; current CI has not run this commit. - Strict child-review provenance telemetry is unavailable. --- Co-authored-by: Codex <codex@openai.com>
This reverts commit 036d947.
Changes: - Route the detection batch-padding channel through mask-aware Kornia geometry and return its transformed boolean mask in NestedTensor. - Carry segmentation instance masks and the padding channel together, then split them before target unpacking. - Keep Perspective fixed-size, document its mapping limits, and add mock plus real seeded regression coverage for boxes, instance masks, and unequal-size batch padding. Impact: - GPU geometric augmentation now keeps model padding coordinates aligned with transformed images and labels. - Perspective remains usable without enabling output-resizing transforms that the fixed-size batch contract cannot represent. Verification: - env UV_CACHE_DIR=/private/tmp/rfdetr-perspective-uv uv run --no-sync pytest tests/training/test_module_data.py tests/datasets/test_kornia_transforms.py -q (227 passed). - run_gates.py lint, format, types, tests, and review gates passed. - pre-commit run --all-files passed. Residual limits: - CUDA integration was unavailable locally. - The full CPU suite was not rerun because six prior export failures are unrelated to this repair. --- Co-authored-by: Codex <codex@openai.com>
|
thanks @Borda, and thanks for taking the padding-mask half on directly — my read of where that leaves #1252: the blocker was happy to pick those up as a follow-up if you want them, or leave it if you'd rather land this first and see how the mask handling behaves in practice. not touching this branch since you have commits on it — say the word if you want anything rebased or split out. one small thing on #1350, which is unrelated but overlaps the same file: it's a scalar-vs-range bug in |
What
Adds
Perspectiveto the Kornia GPU augmentation backend. Follow-up to #1277, same issue (#1252).Why
Perspectiveis one of the twelve names documented inaug_configsthat raiseValueErroron Kornia while working on Albumentations, so the sameaug_configtrains on a CPU box and hard-fails on a GPU box.It is also the only geometric name from that issue that can be added without changing the output resolution, which is what makes it safe. Measured on
8444ce3:That distinction matters because of
on_after_batch_transfer, which rebuilds the batch as:samples.maskis the pre-augmentation padding mask. Any transform that resizes the image leaves the mask describing a different shape, andNestedTensordoes not validate the pair:So the three crop names cannot be mapped by adding a registry entry alone; they need the mask cropped in step, which is a larger change with its own design question. This PR does not attempt it, and adds a test pinning that those three stay rejected so the gap is not quietly "completed" later.
Correction to my own earlier note on #1252: I said there that the crops "have Kornia equivalents with matching semantics" and offered to send all four. That was true transform-for-transform and wrong at the pipeline level, which I only found by tracing where the pipeline output goes. Perspective is the only one of the four that is actually shippable as a registry entry.
Design
scalemaps ontodistortion_scaledirectly: both express corner displacement as a fraction of the image side.A
(min, max)pair collapses to its upper bound and logs a warning, the same asymmetryGaussNoisealready documents (Albumentations samples per call, Kornia takes a single fixed value). A scalar is passed straight through with no warning, since that is an exact request rather than a collapse.keep_size=Falseraises rather than being silently dropped. Albumentations defaults it toTrueand Kornia'sRandomPerspectivealways behaves that way, so the default maps cleanly; the non-default changes the output resolution, which is exactly the case the padding mask cannot survive. Refusing names the reason and points at the CPU backend.Boxes are handled by the existing
AugmentationSequential(data_keys=["input", "bbox_xyxy", ...]), andunpack_boxesalready clamps to image bounds and drops zero-area boxes, so no target-handling change is needed here.Testing
Six new tests. Five fail on the parent commit; the sixth is the guard that the crop names stay rejected, which passes on both sides by design.
test_scale_range_collapses_to_upper_bound_and_warnstest_scalar_scale_does_not_warntest_keep_size_false_is_refused_not_ignoredtest_output_keeps_the_input_resolutiontest_boxes_follow_the_warptest_crop_names_from_1252_remain_unsupportedEnvironment: kornia 0.8.3, albumentations 2.0.8, CPU.
tests/datasets/as a whole did not finish inside my time limit, so I ran the affected module in full rather than the whole directory. Nothing outsidekornia_transforms.pyis touched, and the change is one new registry entry plus its factory.Docs
None. The name was already documented in
aug_configsas supported; this makes that true for the Kornia backend. Happy to add a note about the range-collapse behaviour if you would like it stated outside the docstring.