fix(keypoints): preserve identities across repeated horizontal flips by INo-xious · Pull Request #1569 · roboflow/rf-detr · GitHub
Skip to content

fix(keypoints): preserve identities across repeated horizontal flips - #1569

Merged
Borda merged 5 commits into
roboflow:developfrom
INo-xious:fix/keypoint-flip-parity
Sep 30, 2026
Merged

Borda merged 5 commits into
roboflow:developfrom
INo-xious:fix/keypoint-flip-parity

Conversation

@INo-xious

Copy link
Copy Markdown
Contributor

Summary

Two horizontal flips restore image geometry but currently leave paired keypoint identities exchanged. Count applied flip parity in supported logging-container trees, including SomeOf(replace=True) executing one transform repeatedly, so identities and visibility remain aligned with the final coordinates. Programmatic containers without complete execution records retain replay handling.

Changes

  • Enable Albumentations' public occurrence recording for audited container trees while retaining the wrapper, ReplayCompose and existing swap mechanism.
  • Compute odd/even parity through the existing alias-aware classifier, retaining replay fallback.
  • Add deterministic image/box/keypoint regression coverage and an Unreleased changelog entry.

Verification

  • New regressions against the base: 22 failed, 56 passed; with the fix: 78 passed (Albumentations 2.0.8).
  • uv run --no-sync pytest tests/datasets/test_augmentations.py tests/datasets/test__torchvision.py -n 1 --timeout=120 --color=no: 289 passed.
  • Albumentations 1.4.24: 77 regression/classifier cases passed; real SquareSymmetry execution is tested on 2.0.8 where that alias is available.
  • Full macOS CPU suite (-n 2, CI marker selection): 6608 passed, 114 skipped in 310.95s.
  • pre-commit run --all-files: passed, including mypy.
  • mkdocs build --strict: passed; existing document-conversion warnings remain.
  • Known limitation: SomeOf replacement reuse in programmatic trees containing OneOrOther remains limited by incomplete upstream execution metadata; direct, nested and mixed non-repeating OneOrOther cases are protected by regression tests.
  • No GPU or training-accuracy claim.

Related issues

Follow-up correctness coverage for replay handling in #1164 and flip aliases in #1386 (both merged). No closed issue is claimed as fixed; #1523's coordinate conventions remain outside scope.

@Borda
Borda requested a balanced review from Copilot September 30, 2026 10:43
@Borda Borda added the bug Something isn't working label Sep 30, 2026

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 review overview

🟢 Approval recommended

The targeted implementation preserves fallback behavior and is comprehensively covered by deterministic tests.

Review effort: Balanced
Findings: None

What changed in this PR

Fixes keypoint identity swapping by tracking horizontal-flip parity across Albumentations containers.

Changes:

  • Records repeated transform executions and computes odd/even flip parity.
  • Adds regression coverage for aliases, nested containers, and repeated selections.
  • Documents the fix in the changelog.
File Description
src/​rfdetr/​datasets/​transforms.py Adds occurrence-aware flip parity handling.
tests/​datasets/​test_augmentations.py Adds comprehensive parity regressions.
CHANGELOG.md Records the keypoint fix.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Borda and others added 4 commits September 30, 2026 19:49
…,P1)

Narrow PR-body H1 claim per maintainer decision (draft, no GitHub write);
warn once when a container falls outside execution-log allowlisting and
SomeOf(replace=True) repeats a draw (A1); tighten CHANGELOG wording for
the same limitation; extract _resolve_keypoint_swap to bring
_apply_geometric_transform back under the pre-PR complexity baseline
(A2); rename _replay_contains_horizontal_flip -> _replay_horizontal_flip_parity
to match its parity return value (A3); track the execution-log decision in
self._uses_execution_log instead of re-reading a third-party attribute (A4);
skip ReplayCompose deep-copy when the execution log already decides the
swap (P1).

[resolve group] PR roboflow#1569 — items 1 2 4 5 6 10

---
Co-authored-by: claude[bot] <209825114+claude[bot]@users.noreply.github.com>
Cover nested Compose/RandomOrder as the execution-log allowlist's
outer container for a repeating SomeOf(replace=True) (T1); verify
from_config-built SomeOf(replace=True) matches the direct-constructor
wrapper end-to-end (T3); unit-test _first_unlogged_container directly
for empty, deeply-nested-allowlisted, and deeply-nested-offending
container trees (T4).

[resolve group] PR roboflow#1569 — items 3 8 9

---
Co-authored-by: claude[bot] <209825114+claude[bot]@users.noreply.github.com>
Append PR link to the keypoint-flip Fixed bullet to match the
sibling-bullet convention in the same section.

[resolve group] PR roboflow#1569 — items 11

---
Co-authored-by: claude[bot] <209825114+claude[bot]@users.noreply.github.com>
@codecov

codecov Bot commented Sep 30, 2026 •

Copy link
Copy Markdown

@Borda
Borda merged commit 4504d65 into roboflow:develop Sep 30, 2026
52 checks passed
Borda added a commit that referenced this pull request Oct 3, 2026
…1569)

* fix: preserve keypoint identities across repeated flips
* sw-engineer: fix keypoint-flip parity review findings (H1,A1,A2,A3,A4,P1)
* qa-specialist: add flip-parity test coverage (T1,T3,T4)
* linting-expert: link keypoint-flip CHANGELOG bullet to PR (C1)

---------

Co-authored-by: jirka <6035284+borda@users.noreply.github.com>
Co-authored-by: claude[bot] <209825114+claude[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants