Skip to content

Imports - #5

Closed
Matvezy wants to merge 11 commits into
export_fixfrom
main
Closed

Matvezy wants to merge 11 commits into
export_fixfrom
main

Conversation

@Matvezy

@Matvezy Matvezy commented Mar 20, 2025

Copy link
Copy Markdown
Contributor

Description

Please include a summary of the change and which issue is fixed or implemented. Please also include relevant motivation and context (e.g. links, docs, tickets etc.).

List any dependencies that are required for this change.

Type of change

Please delete options that are not relevant.

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • This change requires a documentation update

How has this change been tested, please provide a testcase or example of how you tested the change?

YOUR_ANSWER

Any specific deployment considerations

For example, documentation changes, usability, usage/costs, secrets, etc.

Docs

  • Docs updated? What were the changes:

@Matvezy Matvezy closed this Mar 20, 2025
Borda added a commit to omkar-334/rf-detr that referenced this pull request Mar 30, 2026
[resolve roboflow#4,roboflow#5] Review comments by Copilot + review (PR roboflow#664):
"weights_only=False...please add a similar warning to the docstring..."

Adds a Warning: section noting torch.load unpickling risk for untrusted
checkpoints. Also adds FileNotFoundError and OSError to the Raises section.

---
Co-authored-by: Claude Code <noreply@anthropic.com>
Borda added a commit to farukalamai/rf-detr that referenced this pull request Apr 1, 2026
…e tests

- test_model_name_with_whitespace_is_stripped: confirms model_name with
  leading/trailing spaces resolves correctly via .strip()
- test_model_name_deprecated_class_raises_runtime_error: unmocked constructor
  path confirms RuntimeError propagates for RFDETRBase and RFDETRSegPreview
  resolved via model_name (mirrors test_from_checkpoint_raises_for_deprecated_class)
- Refactor test_plus_model_name_without_plus_raises_import_error: replace
  for-loop over (RFDETRXLarge, RFDETR2XLarge) with @pytest.mark.parametrize

[resolve roboflow#3] /review finding by qa-specialist: missing unmocked deprecated-class via model_name test
[resolve roboflow#4] /review finding by qa-specialist: no test for whitespace-padded model_name
[resolve roboflow#5] /review finding by qa-specialist: use parametrize instead of for-loop

---
Co-authored-by: Claude Code <noreply@anthropic.com>
Borda added a commit that referenced this pull request Apr 1, 2026
- deepcopy failure → inference_model=None, _is_optimized_for_inference=False (clean state)
- export() failure → _is_optimized_for_inference stays False
- jit.trace failure → _optimized_has_been_compiled stays False, _optimized_batch_size stays None

[resolve #5] /review finding by qa-specialist (report: _outputs/2026/04/output-review-copilot-fix-cuda-context-leak-2026-04-01.md):
"No test for exception recovery during optimization — inconsistent state possible after mid-execution failure"

---
Co-authored-by: Claude Code <noreply@anthropic.com>
Borda added a commit that referenced this pull request Apr 2, 2026
[resolve #4] /review finding by qa-specialist (report: _outputs/2026/04/output-review-fix-stable-2026-04-02.md):
"no test verifies include_source_image=False (default) with tensor input omits source_image"

[resolve #5] /review finding by qa-specialist (report: _outputs/2026/04/output-review-fix-stable-2026-04-02.md):
"no test verifies _is_optimized_for_inference=False and inference_model=None after jit.trace failure"

---
Co-authored-by: Claude Code <noreply@anthropic.com>
Borda added a commit to pirnerjonas/rf-detr that referenced this pull request Apr 2, 2026
- test_rle_size_mismatch_behavior: documents that mismatched size/height/width
  raises ValueError from pycocotools
- test_compressed_rle_bytes_counts_decode: exercises bytes counts branch through
  full convert_coco_poly_to_mask decode path
- test_malformed_rle_counts_none_raises_value_error: confirms new type guard raises
  ValueError for counts=None
- test_model_name_args_missing_num_classes_does_not_inject_kwarg: covers new
  top-level model_name checkpoint format when args has no num_classes

[resolve roboflow#3,roboflow#4,roboflow#5,roboflow#6] /review findings by qa-specialist
(report: _outputs/2026/04/output-review-feat-416-native-rle-annotation-support-2026-04-02.md)

---
Co-authored-by: Claude Code <noreply@anthropic.com>
Borda added a commit that referenced this pull request Apr 9, 2026
- Add TestMSDeformAttnModule: 4 tests for MSDeformAttn.forward covering backward compat (no hw param), hw param path, export mode with hw param, and _export flag guard
- Add test_ms_deform_attn_core_pytorch_export_compatible: end-to-end torch.export.export regression test validating the FakeTensor fix
- Fix gen_encoder_output_proposals backward compat: memory_padding_mask and spatial_shapes now default to None, restoring callers that omit memory_padding_mask

[resolve #5] /review finding by qa-specialist (report: .temp/output-review-copilot-fix-ptl-gradient-accumulation-bug-2026-04-09-2.md):
"[MEDIUM] Missing module-level test for MSDeformAttn.forward"
[resolve #2] @Copilot (gh): "Add torch.export.export smoke test on MSDeformAttn"

---
Co-authored-by: Claude Code <noreply@anthropic.com>
Borda added a commit that referenced this pull request Apr 9, 2026
- Replace bare try/except/pass in test_rejects_non_square_ij_indexing with non-square spatial shapes and an assertion that xy indexing produces different proposals than ij
- Add test_gen_encoder_output_proposals_accepts_python_int_pair_spatial_shapes covering the export-driven list[tuple[int,int]] path with memory_padding_mask=None

[resolve #4] Review comment by @Copilot (PR #936):
"This test will pass regardless of behavior: it swallows all exceptions..."

[resolve #5] Review comment by @Copilot (PR #936):
"Consider adding/adjusting a regression test that passes spatial_shapes as list[tuple[int,int]]..."

---
Co-authored-by: Claude Code <noreply@anthropic.com>
Borda added a commit that referenced this pull request Apr 9, 2026
…strings

Both `make_coco_transforms` and `make_coco_transforms_square_div_64` described
the train pipeline as always applying augmentation + normalization. Added a
paragraph explaining that `gpu_postprocess=True` omits both, deferring them
to `RFDETRDataModule.on_after_batch_transfer`.

[resolve #5] /review finding by doc-scribe (report: .temp/output-review-aug-kornia-2026-04-09.md): "make_coco_transforms() docstring not updated for gpu_postprocess"

---
Co-authored-by: Claude Code <noreply@anthropic.com>
Borda added a commit that referenced this pull request Apr 9, 2026
- resolve_augmentation_backend() now raises ValueError for unrecognised
  backend strings instead of silently returning the raw value (which
  would leave gpu_postprocess=True while no Kornia pipeline is built,
  producing unnormalised training inputs)
- _make_affine() converts Albumentations translate_percent=(min, max)
  to Kornia RandomAffine(translate=(tx, ty)) by taking the max absolute
  value of the range, matching the intended symmetric translation bound

[resolve #5] Review comment by @Copilot (PR #874):
"resolve_augmentation_backend() falls through to `return backend` for any unexpected string..."

[resolve #7] Review comment by @Copilot (PR #874):
"_make_affine() forwards Albumentations-style translate_percent directly to Kornia RandomAffine..."

---
Co-authored-by: Claude Code <noreply@anthropic.com>
Borda added a commit to tr-teja/rf-detr that referenced this pull request Apr 10, 2026
…ch fix

[resolve roboflow#5] /review finding by doc-scribe (report: .temp/output-review-copilot-fix-model-config-resolution-issue-2026-04-10-2.md):
"CHANGELOG not updated — fix resolves a user-facing issue (roboflow#949) but no CHANGELOG entry was added"

---
Co-authored-by: Claude Code <noreply@anthropic.com>
Borda added a commit to mfazrinizar/rf-detr that referenced this pull request Apr 10, 2026
- Add missing `]` closing the tflite extra array — parse error broke all 20 CI checks
- Add `onnx>=1.16.0,<1.20` to tflite extra — converter imports onnx directly; users installing rfdetr[tflite] alone would hit ImportError at runtime
- Add `tf-keras>=2.16.0` to tflite extra — onnx2tf requires tf-keras at runtime (noted in PR description but omitted from diff)

[resolve roboflow#5] /review finding by sw-engineer (report: .temp/output-review-feat-tflite-export-2026-04-10.md): pyproject.toml missing ] on tflite extra
[resolve roboflow#8] /review finding by sw-engineer (report: .temp/output-review-feat-tflite-export-2026-04-10.md): tflite extra must declare onnx dependency
[resolve roboflow#2] @copilot (gh): tf-keras discrepancy between PR description and diff

---
Co-authored-by: Claude Code <noreply@anthropic.com>
Borda added a commit to farukalamai/rf-detr that referenced this pull request Apr 13, 2026
- Empty string falls back to inference_model.onnx (falsy branch)
- Path separators in variant_name are stripped to basename by sanitizer
- Absolute path inputs are also stripped to basename

[resolve roboflow#5][resolve roboflow#6] /review finding by qa-specialist
(report: .temp/output-review-feat-onnx-export-naming-2026-04-13.md)

---
Co-authored-by: Claude Code <noreply@anthropic.com>
Borda added a commit that referenced this pull request Apr 14, 2026
The interpolation upcasts to float32 internally and converts back with
`.to(ckpt_pe.dtype)`. Use a float16 input tensor and assert the output dtype
matches float16 to catch any regression in the dtype round-trip.

[resolve #5] /review finding by qa-specialist (report: .temp/output-review-fix-960-2026-04-14.md):
"No test verifying dtype preservation after interpolation"

---
Co-authored-by: Claude Code <noreply@anthropic.com>
Borda added a commit that referenced this pull request Apr 15, 2026
[resolve #5] Review comment by @Copilot (gh) (PR #973):
"mtime_ns may not change on filesystems with coarse mtime resolution —
replace with checkpoint content comparison (epoch field)"

---
Co-authored-by: Claude Code <noreply@anthropic.com>
Borda added a commit that referenced this pull request Apr 15, 2026
…ismatch

[resolve #5] Review comment by @Copilot (PR #971):
"ValueError message for patch_size mismatch is duplicated. Consider
factoring message construction into a small helper to avoid future
divergence when wording needs to change."

---
Co-authored-by: Claude Code <noreply@anthropic.com>
Borda added a commit to mfazrinizar/rf-detr that referenced this pull request Apr 20, 2026
…egression test

The existing test only checked callback_metrics. Both pl_module.log()
and callback_metrics were changed by the fix; cover both paths so a
regression on the log() side is caught independently.

[resolve roboflow#5] /review finding by qa-specialist (report: .temp/output-review-fix-ema-segm-metrics-978-2026-04-20.md)

---
Co-authored-by: Claude Code <noreply@anthropic.com>
Borda added a commit to Irfan-Hamid-creates/rf-detr that referenced this pull request Apr 23, 2026
- Save/restore global NumPy and Torch RNG state around seed loops in
  TestNonSquareResizeDivisibilityRegression to prevent cross-test leakage
- Add test_val_pipeline_structural_contains_pad_if_needed: verifies that
  make_coco_transforms("val") third wrapper is AlbumentationsWrapper(PadIfNeeded)
  with pad_height_divisor == pad_width_divisor == block_size

[resolve roboflow#4] Review comment by @Copilot (PR roboflow#991):
"test loop resets global NumPy/Torch RNG state via np.random.seed/torch.manual_seed without restoring..."

[resolve roboflow#5] Review comment by @Copilot (PR roboflow#991):
"This loop also mutates global RNG state (np.random.seed / torch.manual_seed) and doesn't restore it..."

[resolve roboflow#6] /review finding by foundry:qa-specialist (report: .temp/output-review-fix-non-square-resize-divisibility-2026-04-23.md):
"Missing test: val/test pipeline structural assertion..."

---
Co-authored-by: Claude Code <noreply@anthropic.com>
Borda added a commit to Irfan-Hamid-creates/rf-detr that referenced this pull request Apr 24, 2026
[resolve roboflow#5] /review finding by foundry:linting-expert (report):
"Optional[int] instead of int | None; project requires-python >= 3.10 and uses modern union syntax elsewhere"

---
Co-authored-by: Claude Code <noreply@anthropic.com>
Borda added a commit to valavanisleonidas/rf-detr that referenced this pull request Apr 24, 2026
Replace bare ModelConfig (missing required fields) with RFDETRBaseConfig
so the expand_path field validator is exercised end-to-end, catching the
CWD-vs-cache regression that SimpleNamespace bypassed.

[resolve roboflow#5] Review comment by @Copilot (gh): test helper bypassed
ModelConfig.pretrain_weights validation via SimpleNamespace, masking real behavior.

---
Co-authored-by: Claude Code <noreply@anthropic.com>
Co-authored-by: OpenAI Codex <codex@openai.com>
Borda added a commit that referenced this pull request Apr 28, 2026
Adds a second bullet describing the new bicubic PE interpolation
(regression fix for #998) between the legacy-format and EMA-stash steps.

[resolve #5] /review finding by foundry:doc-scribe (report: .temp/output-review-feat-997-2026-04-28.md):
"on_load_checkpoint docstring documents 2 legacy formats but not the new PE interpolation step"

---
Co-authored-by: Claude Code <noreply@anthropic.com>
Borda added a commit that referenced this pull request Apr 28, 2026
Use pytest.importorskip("kornia") to match the pattern used by all other
_require_kornia fixtures in the same file. The pytest.fail variant caused
ModuleNotFoundError in the CPU CI matrix (9 jobs) where the optional
[kornia] extra is not installed.

[resolve #5] /review finding by foundry:qa-specialist, foundry:linting-expert (report: .temp/output-review-fix-998-2026-04-28.md)
Also flagged by @Copilot inline comment.

---
Co-authored-by: Claude Code <noreply@anthropic.com>
Borda added a commit that referenced this pull request Apr 28, 2026
- test_finetuned_model_with_coco_names_and_matching_num_classes_uses_direct_indexing:
  num_logit_slots==n must take else branch (not sparse remap)
- test_custom_class_names_with_high_num_classes_does_not_activate_coco_remap:
  custom names + high num_classes must not misfire as COCO-pretrained
- test_background_class_id_under_pretrained_coco_branch_maps_to_background:
  class_id=90 (COCO 'toothbrush') must resolve '__background__', not class name

[resolve #5] foundry:qa-specialist (report): num_logit_slots==n boundary
[resolve #6] foundry:qa-specialist (report): custom class_names guard
[resolve #7] foundry:qa-specialist (report): background ID 90 under COCO branch

---
Co-authored-by: Claude Code <noreply@anthropic.com>
Borda added a commit to mfazrinizar/rf-detr that referenced this pull request Apr 28, 2026
The API call site uses load_optimizer() and get_supported_optimizers()
from the 3.x series. An unpinned dep risks a silent breaking change on
the next major release.

[resolve roboflow#5] /review finding by foundry:linting-expert (report: .temp/output-review-develop-2026-04-28.md):
"S1: pytorch-optimizer unpinned — fast release cadence risks install breakage"

---
Co-authored-by: Claude Code <noreply@anthropic.com>
Borda added a commit that referenced this pull request Apr 29, 2026
[resolve #5] Review comment by @Copilot (PR #1011):
"using types.SimpleNamespace is less faithful to real import semantics t..."

---
Co-authored-by: Claude Code <noreply@anthropic.com>
Borda added a commit that referenced this pull request Apr 30, 2026
… path

[resolve #1] @Copilot (gh): grammar fix in _labelled_query_tensor docstring
[resolve #2] @Copilot (gh): e2e tests now assert query_feat.weight ordering in all three scenarios
[resolve #5] /review finding by foundry:qa-specialist: add Namespace-style args e2e test covering _ckpt_args_get getattr branch
[resolve #6] /review finding by foundry:qa-specialist: add test for args-present-but-keys-absent legacy fallback path

---
Co-authored-by: Claude Code <noreply@anthropic.com>
Co-authored-by: OpenAI Codex <codex@openai.com>
Borda added a commit that referenced this pull request May 13, 2026
[resolve #5] Review finding by foundry:qa-specialist (report: .temp/output-review-copilot-fix-segmentation-training-error-2026-05-13.md):
"Test uses resolution=624; user-reported failure in #1023 at resolution=1008 (84×84=7057 tokens). Defer to follow-up parametrization."

---
Co-authored-by: Claude Code <noreply@anthropic.com>
Borda added a commit that referenced this pull request May 15, 2026
Replace `RFDETRModelModule._load_pretrain_weights` and `_apply_lora` with
delegations to the canonical implementations in `rfdetr.models.weights`:

- `load_pretrain_weights(self.model, self.model_config)` — the canonical
  loader handles PE interpolation, PTL `.ckpt` normalisation, per-group
  query slicing via `_slice_query_param_per_group`, class-name extraction,
  `_warn_on_partial_load`, and writes auto-aligned `num_classes` back onto
  `model_config`. The hand-rolled helper silently dropped each of these.
- `apply_lora(self.model)` — canonical helper already carries a clear
  install-hint ImportError; private duplicate removed.

Drops ~100 LoC of duplicated logic plus four now-unused imports
(`os`, `download_pretrain_weights`, `validate_pretrain_weights`,
`validate_checkpoint_compatibility`).

[resolve #9] /review finding by foundry:sw-engineer (report: .reports/review/2026-05-15T07-09-07Z/review-report.md): `_load_pretrain_weights` reimplements and degrades canonical `load_pretrain_weights` — drops per-group query slicing, PTL `.ckpt` normalisation, class-name extraction, and partial-load warnings

This commit also absorbs the following related action items, since the
underlying code paths are deleted by this refactor:
- #5  [gh][req] Per-group query slicing for `group_detr > 1` — canonical
       `load_pretrain_weights` uses `_slice_query_param_per_group`.
- #11 [report][suggest] Document `weights_only=False`; narrow bare except —
       legacy duplicate site removed; documented in canonical helper.
- #12 [report][suggest] Rewrite `_load_pretrain_weights` docstring — the
       function no longer exists.

---
Co-authored-by: Claude Code <noreply@anthropic.com>
Borda added a commit to omkar-334/rf-detr that referenced this pull request May 19, 2026
… edge cases

Resolve /review findings roboflow#4, roboflow#5, roboflow#8, roboflow#9, roboflow#10 from
.reports/review/2026-05-19T10-06-08Z/review-report.md

- test_run_inference_name_based_mask_detection: exercises primary name-based
  path (`"masks" in output name`) — previously dead code under test; every
  existing seg test used `Identity_N` which bypassed it
- test_run_inference_seg_model_no_detections_returns_none_mask: seg model +
  all logits below threshold → `Detections.mask is None` (post-keep.any() gate)
- test_decode_masks_raises_on_wrong_rank: _decode_masks rejects rank-4 input
  with a clear ValueError (exercises new guard from prior commit)
- test_decode_masks_exact_zero_logit_decodes_to_false: boundary at strict >0;
  uniform-zero logit map → all-False (not all-True)
- test_decode_masks_non_square_logit_input: (3,7,14) logits → (3,28,56) output,
  guards PIL bilinear resize for non-square feature maps
- test_decode_masks_parity_positive_negative_regions: high-magnitude ±10 logits
  verify correct True/False regions after bilinear upsample; parity regression
  guard for PostProcess.forward equivalent contract

---
Co-authored-by: Claude Code <noreply@anthropic.com>
Borda added a commit to omkar-334/rf-detr that referenced this pull request May 26, 2026
- [resolve roboflow#5] /review finding by foundry:sw-engineer (report: .reports/review/2026-05-26T19-40-29Z/review-report.md): ai_edge_litert masking dict was duplicated in fixture and tf-fallback test; extract as module-level constant and spread into both patch.dict calls

---
Co-authored-by: Claude Code <noreply@anthropic.com>
Borda added a commit that referenced this pull request May 27, 2026
…ry tests

Add TestDownloadLock: verifies TimeoutError raised immediately when lock
file is pre-created and timeout_s=0 (covers documented but uncovered path).

Add TestDownloadAndExtract: monkeypatches urlretrieve to supply a crafted
ZIP with a '../evil.txt' member; verifies RuntimeError is raised by the
path-traversal guard (is_relative_to check at line 154).

Parametrize TestCocoValImagesComplete threshold cases (below/at/above) to
document >= semantics and close the N+1 boundary gap.

[resolve #5] Review by @foundry:qa-specialist (report: .reports/review/2026-05-27T19-00-46Z/review-report.md):
"_download_lock TimeoutError path has zero coverage..."
Challenge: evidence=VALID suggestion=VALID resolution=as-suggested

[resolve #6] Review by @foundry:qa-specialist (report: .reports/review/2026-05-27T19-00-46Z/review-report.md):
"_download_and_extract path-traversal guard has zero coverage..."
Challenge: evidence=VALID suggestion=VALID resolution=as-suggested

[resolve #10] Review by @foundry:qa-specialist (report: .reports/review/2026-05-27T19-00-46Z/review-report.md):
"_coco_val_images_complete missing N+1 boundary test..."
Challenge: evidence=VALID suggestion=VALID resolution=self-resolved (parametrized)

---
Co-authored-by: Claude Code <noreply@anthropic.com>
Borda added a commit to Aaryan562/rf-detr that referenced this pull request Jun 2, 2026
…itions

Add Note section listing the three cases where the method is a no-op:
metric is None, distributed group not initialised, or world_size==1.
Caller-observable contract was undocumented; state is unchanged in these
cases and callers inspecting metric state after the call need to know.

[resolve roboflow#5] /review finding by foundry:doc-scribe (report: .reports/review/2026-06-02T05-43-56Z/review-report.md)

---
Co-authored-by: Claude Code <noreply@anthropic.com>
Borda added a commit that referenced this pull request Jun 2, 2026
…ings

- Add Raises section (PermissionError, OSError) to export_for_roboflow
- Clarify description: bundle always embeds class_names in args copy
- Document overwrite behaviour for existing files
- Add Note cross-reference in deploy_to_roboflow pointing to export_for_roboflow

[resolve #5] [resolve #10] [resolve #11] /review findings by foundry:doc-scribe (report: .reports/review/2026-06-02T07-59-16Z/review-report.md)

---
Co-authored-by: Claude Code <noreply@anthropic.com>
Borda added a commit that referenced this pull request Jun 12, 2026
- Add postprocess_trace_alpha to _MC_NAMESPACE_FIELDS and pass to PostProcess in inference.py — trace-based uncertainty score fusion was silently always 0.2 regardless of config
- A6: class-schema mismatch changes from raise ValueError to logger.warning + return zeros — avoids crashing training on a mislabeled batch
- A4: raise RFDETRLarge init_error from retry_exc (not from None) — preserves retry traceback in exception chain
- H12: add Example sections to all __all__ exports in keypoints.py (modulate, ConditionalQueryInitializer, compute_l1_keypoint_loss, compute_keypoint_matching_cost)
- H13: add keypoint_oks_sigmas row to training-parameters.md plus flat-vs-per-keypoint OKS sigma note
- H14: add RFDETRKeypointPreview, KeypointTrainConfig, infer_coco_keypoint_schema, CocoKeypointSchema to CHANGELOG [Unreleased]
- A7: add SIGKILL-leak note to _download_lock docstring; A9: add forward-compat comment to KeypointSchema alias
- Add @pytest.mark.coco17 to keypoint benchmark tests (#5, #6)
- T1: n_targets=0 returns (B, Q, 0) cost tensors; T2: class-schema mismatch now returns zeros without raising; T3: non-finite precision_cholesky propagates NaN; T4: infer_coco_keypoint_schema error paths; T5: active_keypoint_counts parametrized coverage; T6: empty-detections early-return; T7: PostProcess validates masks XOR keypoints raises ValueError

---
Co-authored-by: claude[bot] <209825114+claude[bot]@users.noreply.github.com>
Co-authored-by: OpenAI Codex <codex@openai.com>
Borda added a commit that referenced this pull request Jun 12, 2026
- Add postprocess_trace_alpha to _MC_NAMESPACE_FIELDS and pass to PostProcess in inference.py — trace-based uncertainty score fusion was silently always 0.2 regardless of config
- A6: class-schema mismatch changes from raise ValueError to logger.warning + return zeros — avoids crashing training on a mislabeled batch
- A4: raise RFDETRLarge init_error from retry_exc (not from None) — preserves retry traceback in exception chain
- H12: add Example sections to all __all__ exports in keypoints.py (modulate, ConditionalQueryInitializer, compute_l1_keypoint_loss, compute_keypoint_matching_cost)
- H13: add keypoint_oks_sigmas row to training-parameters.md plus flat-vs-per-keypoint OKS sigma note
- H14: add RFDETRKeypointPreview, KeypointTrainConfig, infer_coco_keypoint_schema, CocoKeypointSchema to CHANGELOG [Unreleased]
- A7: add SIGKILL-leak note to _download_lock docstring; A9: add forward-compat comment to KeypointSchema alias
- Add @pytest.mark.coco17 to keypoint benchmark tests (#5, #6)
- T1: n_targets=0 returns (B, Q, 0) cost tensors; T2: class-schema mismatch now returns zeros without raising; T3: non-finite precision_cholesky propagates NaN; T4: infer_coco_keypoint_schema error paths; T5: active_keypoint_counts parametrized coverage; T6: empty-detections early-return; T7: PostProcess validates masks XOR keypoints raises ValueError

---
Co-authored-by: claude[bot] <209825114+claude[bot]@users.noreply.github.com>
Co-authored-by: OpenAI Codex <codex@openai.com>
Borda added a commit to anatoly-ryabchenko/rf-detr that referenced this pull request Jun 14, 2026
…s count

Add three regression tests to TestFromCheckpointNumClassesProvenance and
a supporting fixture (args_only_checkpoint):

1. test_checkpoint_num_classes_from_args_not_marked_user_set — verifies
   that num_classes injected via the legacy checkpoint args dict (not via
   model_config) is also cleared from model_fields_set by from_checkpoint.
   Addresses the gap where the two_class_checkpoint fixture only exercised
   the model_config injection path.

2. test_explicit_default_num_classes_does_not_block_alignment — documents
   that passing num_classes=<ModelConfig default> explicitly does not pin
   the head; _align_num_classes_from_dataset treats user_overrode=False
   when value == default, so adaptation still proceeds.  Guards against
   accidental removal of the value!=default guard.

3. test_equal_class_count_does_not_rebuild_head — confirms the fix has
   no side-effect when dataset class count matches checkpoint class count;
   head shape must remain unchanged.

[resolve roboflow#2] foundry:qa-specialist (report: .reports/review/2026-06-13T18-05-46Z/review-report.md)
[resolve roboflow#4] foundry:qa-specialist (report: .reports/review/2026-06-13T18-05-46Z/review-report.md)
[resolve roboflow#5] foundry:qa-specialist (report: .reports/review/2026-06-13T18-05-46Z/review-report.md)

---
Co-authored-by: claude[bot] <209825114+claude[bot]@users.noreply.github.com>
Borda added a commit that referenced this pull request Jul 3, 2026
…eak memory

Float32 chunks accumulated in list + torch.cat allocated a second full float
tensor; > 0.0 threshold fired after both were live. At K=300 @ 1080p peak
was ~5 GB (1.6× baseline). Thresholding inside the comprehension keeps only
one float chunk transient; accumulated list + concat are bool, reducing peak
to ~1.5 GB.

- Move > 0.0 inside list comprehension so each chunk becomes bool immediately
- Update empty-path dtype to torch.bool for consistency
- Remove redundant > 0.0 at assignment site (already bool)

[resolve #5] /review finding by foundry:perf-optimizer (report: .reports/review/2026-07-03T07-37-38Z/review-report.md)

---
Co-authored-by: claude[bot] <209825114+claude[bot]@users.noreply.github.com>
Borda added a commit to anatoly-ryabchenko/rf-detr that referenced this pull request Jul 17, 2026
…tch probe

[resolve roboflow#5,roboflow#6] /review finding by foundry:perf-optimizer (report: .reports/review/2026-07-17T05-57-25Z/review-report.md)

evaluate() built a fresh RFDETRModelModule while the original model
stayed resident on the accelerator, risking OOM on the largest
variants right after train() just fit them. Move the original off
the accelerator for the transplant window and restore it immediately
after, so the two copies are never both resident on the (scarcer)
accelerator memory at once.

batch_size="auto" also ran resolve_auto_batch_config's forward+backward
training-mode probe inside evaluate(), which never backprops and
under-sizes the eval batch to the training memory envelope. Add a
for_eval flag to the shared _prepare_run_config so evaluate() falls
back to TrainConfig's default micro-batch instead of probing.
Borda added a commit to congbrian/rf-detr that referenced this pull request Jul 28, 2026
[resolve group] PR roboflow#1235 — items #3,4,5,8,12

Addresses review feedback on src/rfdetr/export/_coreml/{converter,torch_ops,op_coverage,__init__}.py
and the coremltools pin in pyproject.toml:
- #3 [gh][req] @Borda: _check_coremltools_available uses the module-level
  _IS_COREMLTOOLS_AVAILABLE flag instead of re-importing coremltools per call;
  also fixes the broad `except Exception` misreporting a broken-but-installed
  coremltools as missing (related /review finding, same function).
- roboflow#4 [gh][question] @Borda: documents why the ImportError branch in
  export_coreml's except clause is not dead code (ct.convert lazily imports
  private coremltools submodules); also narrows the ValueError/RuntimeError
  docstring contract to match actual behavior.
- roboflow#5 [report][req] sw-engineer: registry-patch critical section is now
  lock-guarded (thread-safety) and the idempotency check validates actual
  handler identity instead of trusting the _PATCHED bool alone.
- roboflow#8 [report][req] solution-architect: documents the coremltools private-API
  coupling and tested version matrix at the pin site and call sites.
- roboflow#12 [report][suggest] codex (converter.py portion): documents that CoreML
  output names are coremltools-inferred and must be matched positionally.

Challenge: evidence=VALID suggestion=VALID resolution=as-suggested (all five)

---
Co-authored-by: claude[bot] <209825114+claude[bot]@users.noreply.github.com>
Borda added a commit that referenced this pull request Jul 29, 2026
training-parameters.md stated the pre-fix default and omitted the
None/[]/[...] hflip-disable semantics. CHANGELOG [Unreleased] was
empty for this silent augmentation-behavior fix.

[resolve #1,#5] PR #1248 — doc-scribe findings
Borda added a commit to adhavan18/rf-detr that referenced this pull request Aug 4, 2026
CLAHE test now reads the public transform.clip_limit attribute
instead of the private _param_generator.sampler_dict["clip_limit_factor"]
(RandomClahe exposes it publicly; no need for the private path).
Sharpen's sampler_dict read has no public equivalent in the
installed Kornia version, so it stays, but is now wrapped with a
documented helper that fails with a clear message instead of a
raw AttributeError/KeyError if Kornia's private internals shift.

[resolve roboflow#5, roboflow#6] PR roboflow#1277 — @Copilot (gh) + foundry:qa-specialist

---
Co-authored-by: claude[bot] <209825114+claude[bot]@users.noreply.github.com>
Borda added a commit to JESUSROYETH/rf-detr that referenced this pull request Aug 5, 2026
Resolves review findings from /oss:review PR roboflow#1279 (report: .reports/review/2026-08-05T09-41-01Z/review-report.md), items roboflow#2,3,4,5,6,7,8,9,10,14,15,18. All findings cross-validated by foundry:challenger/sw-engineer/qa-specialist before implementation.

- Reset torch.cuda.reset_peak_memory_stats at fit start (gated by _is_cuda), so max_mem reflects this run's footprint rather than a process-lifetime / batch_size="auto"-probe peak (roboflow#2)
- Warn via rank_zero_warn + setdefault instead of silently overwriting a same-named user-logged metric (roboflow#3)
- Reword class docstrings from "GPU" to "CUDA" for accuracy — implementation is CUDA-only (roboflow#4)
- Add ProgressBar base to _GpuMemoryMetricsMixin, dropping the now-unresolvable type:ignore[misc] (roboflow#5, subsumes roboflow#17)
- Test peak-reset semantics across sequential get_metrics() calls (roboflow#6)
- Add missing Rich progress bar coverage for the cuda-present-but-not-initialized branch (roboflow#7)
- Test _is_cuda with None/non-torch.device input (roboflow#8)
- Add rounding-boundary test cases (0 bytes, 0.5MB boundary) (roboflow#9)
- Fold xla/tpu device-type case into the existing non-cuda parametrize (roboflow#10)
- Fix module docstring overstating on_test_end's gating mechanism (roboflow#14)
- Document max_mem as rank-local under DDP (roboflow#15)
- Match legacy "123 MB" spacing (space before unit) for restoration parity (roboflow#18)

34/34 tests pass (tests/training/callbacks/test_gpu_memory_progress_bar_callback.py + tests/training/test_trainer.py).

---
Co-authored-by: claude[bot] <209825114+claude[bot]@users.noreply.github.com>
Borda added a commit that referenced this pull request Aug 5, 2026
- Reset torch.cuda.reset_peak_memory_stats at fit start (gated by _is_cuda), so max_mem reflects this run's footprint rather than a process-lifetime / batch_size="auto"-probe peak (#2)
- Warn via rank_zero_warn + setdefault instead of silently overwriting a same-named user-logged metric (#3)
- Reword class docstrings from "GPU" to "CUDA" for accuracy — implementation is CUDA-only (#4)
- Add ProgressBar base to _GpuMemoryMetricsMixin, dropping the now-unresolvable type:ignore[misc] (#5, subsumes #17)
- Test peak-reset semantics across sequential get_metrics() calls (#6)
- Add missing Rich progress bar coverage for the cuda-present-but-not-initialized branch (#7)
- Test _is_cuda with None/non-torch.device input (#8)
- Add rounding-boundary test cases (0 bytes, 0.5MB boundary) (#9)
- Fold xla/tpu device-type case into the existing non-cuda parametrize (#10)
- Fix module docstring overstating on_test_end's gating mechanism (#14)
- Document max_mem as rank-local under DDP (#15)
- Match legacy "123 MB" spacing (space before unit) for restoration parity (#18)

---------

Co-authored-by: Jesús Royeth <JESUSROYETH@users.noreply.github.com>
Co-authored-by: Jirka Borovec <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

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants