refine: Use active-first keypoint schema - #1160
Merged
Merged
Conversation
Default keypoint preview configs to [17] instead of the legacy background-first [0, 17] schema. Remap COCO keypoint categories onto active schema slots in keypoint mode, while keeping legacy background-first checkpoint handling documented and tested. Co-authored-by: Codex <codex@openai.com>
Contributor
There was a problem hiding this comment.
Pull request overview
This pull request switches RF-DETR keypoint models’ default schema from legacy background-first ([0, 17]) to active-first ([17]), updating dataset remapping, inference class-name resolution, docs, and tests while preserving compatibility with older checkpoints.
Changes:
- Updated the default keypoint preview config and related tests/docs to use the active-first schema (
[17]). - Adjusted COCO keypoint dataset building to remap category IDs in keypoint mode so keypoint-bearing categories align with active schema slots.
- Updated
predict()class-name mapping logic/documentation to treat only background-first schemas (leading0) as “shifted”.
Reviewed changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
src/rfdetr/config.py |
Changes keypoint preview default schema to active-first ([17]). |
src/rfdetr/datasets/coco.py |
Updates keypoint cat→label mapping docs and enables category remapping for COCO keypoint mode. |
src/rfdetr/detr.py |
Updates predict() documentation and legacy keypoint class-id handling comments/mapping. |
tests/datasets/test_coco.py |
Updates expectations for active-first label alignment and COCO keypoint-mode remapping. |
tests/models/test_builder_characterization.py |
Updates characterization assertion to the new schema default. |
tests/models/test_predict.py |
Updates test docstring context to reflect active-first vs legacy background-first behavior. |
tests/test_config_keypoints.py |
Updates config default assertions from [0, 17] → [17]. |
tests/training/test_detr_shim.py |
Renames/updates auto-detect test to assert active-first schema slot counting. |
tests/training/test_module_data.py |
Updates keypoint augmentation warning test setup to use [17]. |
docs/learn/train/training-parameters.md |
Updates documented default and clarifies legacy schema compatibility. |
docs/learn/train/dataset-formats.md |
Updates keypoint schema explanation to active-first default with legacy note. |
docs/learn/run/keypoints.md |
Updates class_id semantics for active-first vs legacy background-first checkpoints. |
Codecov Report❌ Patch coverage is ❌ Your project check has failed because the head coverage (82%) is below the target coverage (95%). You can increase the head coverage or adjust the target coverage. Additional details and impacted files@@ Coverage Diff @@
## develop #1160 +/- ##
=======================================
Coverage 82% 82%
=======================================
Files 110 110
Lines 11753 11783 +30
=======================================
+ Hits 9613 9647 +34
+ Misses 2140 2136 -4 🚀 New features to boost your workflow:
|
…rain() - refine: add UserWarning in _align_keypoint_schema_from_dataset when [0,17] checkpoint schema gets silently overridden to [17] by dataset inference; directs user to pass num_keypoints_per_class=[0,17] explicitly to keep legacy alignment - docs: add keypoint schema change note to docs/getting-started/migration.md upgrade 1.7→1.8 section with before/after code example - fix: rename _is_keypoint_model → _is_legacy_bgfirst_keypoint (was False for active-first models, name was misleading) - fix: update stale docstring in _detect_num_classes_for_training: slot-0-reserved framing now scoped to legacy bg-first schemas only - fix: improve _build_keypoint_cat2label docstring — "non-zero schema slots" → "slots with non-zero keypoint count (count > 0)"; add [17,4] multi-class example - fix: add coupling comment at both remap_category_ids=include_keypoints sites in build_coco() warning that schema and remap are semantically coupled - test: add test_standard_coco_cat_id_maps_to_active_schema_slot — COCO person (cat_id=1) maps to slot 0 under [17] - test: add TestBuildKeypointCat2Label — verify [0,17] maps person to slot 1; verify mixed detection+keypoint cat assignment --- Co-authored-by: claude[bot] <209825114+claude[bot]@users.noreply.github.com>
…oint - Add `is_bg_first_schema`, `to_active_first`, `to_bg_first`, `schemas_semantically_equal` to `utilities/keypoints.py` replacing raw `schema[0] == 0` pattern with named, tested functions - Replace raw `[0] == 0` checks in `detr.py` with `is_bg_first_schema()` - Auto-align `num_keypoints_per_class` from checkpoint `_kp_active_mask` in `load_pretrain_weights` when the user did not explicitly override the field — fixes AP≈0 when loading a bg-first `[0, 17]` pretrained model into active-first default config - Sync `num_keypoints_per_class` in derived args namespace after `load_pretrain_weights` mutates `model_config` in `_build_model_context` - Regression tests: `TestLoadPretrainWeightsKeypointSchemaAutoAlign` with bg-first auto-align, matching-schema no-op, and user-override guard cases --- Co-authored-by: claude[bot] <209825114+claude[bot]@users.noreply.github.com>
- Export 4 schema utilities from `rfdetr.utilities` public API - Add pytest coverage for `is_bg_first_schema`, `to_active_first`, `to_bg_first`, `schemas_semantically_equal` (31 new tests) plus degenerate-mask guards (2 new tests) - Add `logger.warning` in auto-align for 1-D mask and all-zero mask edge cases - Fix migration.md: `!!! note` → `!!! warning "Breaking: ..."`, move to "Upgrade 1.8 → 1.9" section, add post-construction assignment caveat - Clarify `to_active_first` and `schemas_semantically_equal` docstrings with edge-case examples - Add CHANGELOG `### Added` and `### Changed` entries for schema utilities and default change --- Co-authored-by: claude[bot] <209825114+claude[bot]@users.noreply.github.com>
Merged
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This pull request updates the default keypoint schema for RF-DETR keypoint models to use an "active-first" layout (
[17]), instead of the previous "background-first" layout ([0, 17]). The documentation, code, and tests are updated to reflect this new default, while maintaining compatibility with legacy background-first checkpoints. The changes clarify schema handling, update logic for category remapping, and ensure that keypoint-bearing categories are mapped correctly in both code and documentation.Keypoint schema and model logic updates:
Changed the default keypoint schema in
RFDETRKeypointPreviewConfigand throughout the codebase from[0, 17](background-first) to[17](active-first), and updated all related logic and documentation to reflect this new default. Legacy[0, 17]schemas are still supported for compatibility. [1] [2] [3] [4] [5] [6] [7] [8]Updated the mapping logic in
_build_keypoint_cat2labeland related dataset construction to ensure keypoint-bearing categories are mapped to the correct schema slots for both active-first and background-first layouts. [1] [2] [3]Testing and compatibility:
[17]schema and verified that category-to-label mappings, keypoint assignments, and remapping logic work as intended for both schema types. [1] [2] [3] [4] [5] [6] [7] [8] [9]Documentation improvements:
Category remapping logic:
These updates standardize the keypoint schema across the codebase, improve clarity for users, and maintain backward compatibility with older model checkpoints.