Skip to content

add config change warning#1017

Open
isaacrob wants to merge 12 commits intodevelopfrom
isaac/warn_on_pretrained_config_change
Open

add config change warning#1017
isaacrob wants to merge 12 commits intodevelopfrom
isaac/warn_on_pretrained_config_change

Conversation

@isaacrob
Copy link
Copy Markdown
Contributor

What does this PR do?

Right now, it's easy to change something in a configuration that causes pretrained weights to silently fail to load or have unintended pipeline. In these cases, changing the config can lead to a surprisingly large degradation of performance without the user being aware.

This PR adds warnings such that when configurations are changed in such a way that they are less compatible or incompatible with pretrained weights, the user is made aware.

Type of Change

  • New feature (non-breaking change that adds functionality)

Testing

  • I have tested this change locally
  • I have added/updated tests for this change

Test details:

Tests cover changing configurations and confirming that warnings are raised when appropriate.

Checklist

  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code
  • I have commented my code where necessary, particularly in hard-to-understand areas
  • My changes generate no new warnings or errors
  • I have updated the documentation accordingly (if applicable)

@codecov
Copy link
Copy Markdown

codecov Bot commented Apr 30, 2026

Codecov Report

❌ Patch coverage is 97.89474% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 81%. Comparing base (a9f5693) to head (2f17ac6).

❌ Your project check has failed because the head coverage (81%) 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   #1017   +/-   ##
=======================================
  Coverage       80%     81%           
=======================================
  Files          101     101           
  Lines         8523    8616   +93     
=======================================
+ Hits          6848    6939   +91     
- Misses        1675    1677    +2     
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Borda
Borda previously approved these changes Apr 30, 2026
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Adds user-facing warnings to surface cases where changing a model configuration can lead to pretrained weights loading partially (or becoming effectively incompatible), helping users avoid silent accuracy regressions.

Changes:

  • Add config-time PretrainWeightsCompatibilityWarning emitted from ModelConfig when variant-default pretrained weights are likely to be incompatible with user overrides.
  • Add load-time partial-load detection in load_pretrain_weights() that warns when load_state_dict(strict=False) leaves unexpected missing/unexpected keys.
  • Add/extend tests for both the config-time warnings and the load-time partial-load warning, and adjust pytest warning filters for the test suite.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 4 comments.

Show a summary per file
File Description
src/rfdetr/config.py Introduces PretrainWeightsCompatibilityWarning and a model validator to warn about override patterns that break (or degrade) variant pretrained weight loading.
src/rfdetr/models/weights.py Adds _warn_on_partial_load() and wires it into load_pretrain_weights() to warn on unexpected missing/unexpected keys.
tests/models/test_config.py Adds tests asserting when config-time warnings should/shouldn’t be emitted.
tests/models/test_weights.py Adds unit + integration tests for the new partial-load detector behavior.
pyproject.toml Globally filters the new warnings in the test suite (re-enabled explicitly in dedicated tests).

Comment thread src/rfdetr/config.py Outdated
Comment thread src/rfdetr/config.py
Comment thread src/rfdetr/config.py
Comment thread pyproject.toml Outdated
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Borda and others added 9 commits May 1, 2026 00:42
Comment said "extra slots randomly initialised" but num_queries/group_detr
increases cause a shape mismatch → RuntimeError from load_state_dict, not
silent random init.

- [resolve #12] /review finding by foundry:sw-engineer (report: .temp/output-review-develop-2026-05-01.md)

---
Co-authored-by: Claude Code <noreply@anthropic.com>
…kpoint

When a variant's pretrain_weights default is None or PydanticUndefined,
the override-check block would still run and format the warning message
with "(None)" — confusing for users of the abstract base config.

Short-circuit before the override loop when no published checkpoint exists.

- [resolve #3] Review comment by @Copilot (gh)

---
Co-authored-by: Claude Code <noreply@anthropic.com>
…head

Previously the field was in _PRETRAIN_BREAKING_FIELDS unconditionally,
causing detector-only configs to warn about a field that has no effect
on non-segmentation checkpoints.

Removed from the shared tuple; added explicit conditional check in the
validator guarded by self.segmentation_head being True.

- [resolve #2] Review comment by @Copilot (gh)

---
Co-authored-by: Claude Code <noreply@anthropic.com>
…heckpoint

Case-2 early-return previously treated any non-None pretrain_weights as a
custom checkpoint, suppressing architecture-override warnings even when the
user explicitly passed the variant's published-default path string.

Fix: compare the user-set (expanded) path against the expanded class default.
When they match, fall through to the case-3 architecture-override check.

- [resolve #1] Review comment by @Copilot (gh)

---
Co-authored-by: Claude Code <noreply@anthropic.com>
…urface

Users need to import the warning class to use it with
warnings.filterwarnings(category=...) — without this export they have to
reach into rfdetr.config directly.

Added import and __all__ entry in rfdetr/__init__.py.

- [resolve #6] /review finding by foundry:solution-architect (report: .temp/output-review-develop-2026-05-01.md)

---
Co-authored-by: Claude Code <noreply@anthropic.com>
Plain substring check (pat in key) could match unrelated module names that
happen to share a common prefix with the pattern. Switched to anchor-based
matching: pattern must appear at key start or immediately after a module
separator (dot), preventing false-positive suppression.

- [resolve #10] /review finding by foundry:sw-engineer (report: .temp/output-review-develop-2026-05-01.md)

---
Co-authored-by: Claude Code <noreply@anthropic.com>
- TestBreakingListIntegrity.test_all_breaking_fields_exist_in_model_config — meta-test guards against stale entries in _PRETRAIN_BREAKING_FIELDS / _PRETRAIN_BREAKING_ON_INCREASE
- test_explicit_variant_default_path_runs_arch_override_check — regresses the case-2 bypass where passing the variant's own default checkpoint path skipped arch checks
- test_product_preserving_group_detr_increase_still_warns — documents per-field (not product-aware) warning behaviour for group_detr + num_queries
- TestPartialLoadDetector.test_mixed_intentional_and_unintentional_keys_warn_only_for_unexpected — verifies boundary-aware key filtering keeps head-reinit keys out of warning while backbone mismatches still fire

---
Co-authored-by: Claude Code <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants