Search before asking
Bug
Bug
The three dataset builders read the same set of pipeline options with three different access disciplines, and the two that use getattr fallbacks pick fallback values that contradict the config defaults. A caller passing a partial args namespace silently gets a different training pipeline instead of an error.
build_coco uses direct attribute access (coco.py:1335-1338):
multi_scale=args.multi_scale,
expanded_scales=args.expanded_scales,
patch_size=args.patch_size,
num_windows=args.num_windows,
(and again at coco.py:1351-1355 / 1378-1382, which add skip_random_resize=not args.do_random_resize_via_padding)
build_roboflow_from_coco (coco.py:1422-1428) and build_roboflow_from_yolo (yolo.py:1054-1060) use getattr with a literal fallback for the same fields. Five of those fallbacks disagree with the real default:
| field |
getattr fallback |
actual default |
source |
square_resize_div_64 |
False |
True |
TrainConfig (config.py:1061) |
segmentation_head |
False |
True on every seg variant |
ModelConfig |
multi_scale |
False |
True |
TrainConfig (config.py:1066) |
expanded_scales |
False |
True |
TrainConfig (config.py:1067) |
num_windows |
4 |
2 for every released variant (4 only on the deprecated base) |
ModelConfig — required field, no default |
patch_size |
16 |
16 nano/small/medium/large, 14 base, 12 seg + keypoint |
ModelConfig — required field, no default |
patch_size and num_windows are declared without defaults on ModelConfig (config.py:481-482), so there is no correct constant to fall back to. The pair the builders actually choose — (16, 4) — matches no shipped variant.
The keypoint fields (use_grouppose_keypoints, num_keypoints_per_class) diverge on the keypoint variant too, but their fallback is deliberate and documented at coco.py:1301 — absence means "detection-only", which is a safe default. The five above have no such reading.
Why it is reachable
build_coco, build_roboflow_from_coco and build_roboflow_from_yolo are all publicly re-exported from rfdetr.datasets (__init__.py:27,29).
coco.py:1321-1325 documents calling a builder directly, bypassing the DataModule, as supported usage:
"This branch only fires when build_coco() is called directly with args.augmentation_backend == "auto" (a supported direct usage, since build_coco is re-exported from rfdetr.datasets), bypassing the DataModule."
- The repo's own tests already rely on it —
test_coco.py:632 passes types.SimpleNamespace(dataset_dir=..., augmentation_backend="gpu") with none of these fields set, which only works because the fallbacks absorb them.
To be fair to the current code: on the RFDETRDataModule path this is inert. _namespace_from_configs populates every one of these fields, so normal model.train(...) is unaffected. The exposure is the documented direct-call path — and the asymmetry means one builder fails loudly there while its two siblings quietly mistrain.
Measured impact
Same partial namespace, resolution=512:
build_roboflow_from_coco -> silently succeeds
square_resize_div_64 = False (config default True — different resize branch entirely)
multi_scale = False (config default True — multi-scale training off)
expanded_scales = False (config default True)
patch_size = 16
num_windows = 4 (released variants use 2)
build_coco -> AttributeError: 'types.SimpleNamespace' object has no attribute 'multi_scale'
num_windows alone reshapes the multi-scale scale set, since it feeds compute_multi_scale_scales():
real (patch=16, windows=2): [352, 384, 416, 448, 480, 512, 544, 576, 608, 640, 672]
fallback (patch=16, windows=4): [192, 256, 320, 384, 448, 512, 576, 640, 704, 768, 832]
It also feeds draft_size_for_transforms(), so the JPEG draft-decode floor moves (672 → 600 here), decoding images below the largest training scale.
Proposed fix
Make the three builders agree on one discipline. Two options, and I would rather have your call than guess:
A. Direct attribute access everywhere (match build_coco). Fails loudly on an incomplete namespace instead of substituting a wrong pipeline. On the supported DataModule path args is always complete, so nothing regresses. Cost: the handful of tests passing partial namespaces need the fields added.
B. Keep getattr, but source the fallbacks from the config field defaults rather than restating them as literals, so they cannot drift again. This does not work for patch_size / num_windows, which have no default to read — those would still need option A.
I lean toward A for the five fields that feed the resize pipeline, since a silently different scale distribution is worse than an AttributeError, and patch_size / num_windows have no defensible fallback. Happy to implement whichever you prefer, with tests covering builder parity so the three cannot drift apart again.
Environment
- RF-DETR:
1.10.0.dev1, develop @ 6674d858, installed with -e .
- OS: Ubuntu (Linux 7.0.0-30-generic)
- Python: 3.12
- CPU-only reproduction; no GPU needed
Minimal Reproducible Example
import types
from unittest.mock import MagicMock, patch
from rfdetr.datasets.coco import build_coco, build_roboflow_from_coco
# Exactly the namespace shape tests/datasets/test_coco.py:632 already uses.
partial = types.SimpleNamespace(dataset_dir="/fake/ds", augmentation_backend="cpu")
with (
patch("rfdetr.datasets.coco.Path") as mock_path,
patch("rfdetr.datasets.coco.make_coco_transforms_square_div_64") as sq,
patch("rfdetr.datasets.coco.make_coco_transforms") as ms,
patch("rfdetr.datasets.coco.CocoDetection", return_value=MagicMock()),
):
mock_path.return_value.exists.return_value = True
sq.return_value = ms.return_value = MagicMock()
build_roboflow_from_coco("train", partial, resolution=512)
used = sq if sq.called else ms
print("square-resize builder used:", sq.called) # False; config default is True
for k in ("multi_scale", "expanded_scales", "patch_size", "num_windows"):
print(f" {k:20} = {used.call_args.kwargs.get(k)}") # False, False, 16, 4
# Same namespace, sibling builder:
with (
patch("rfdetr.datasets.coco.Path") as mock_path,
patch("rfdetr.datasets.coco.CocoDetection", return_value=MagicMock()),
):
mock_path.return_value.exists.return_value = True
build_coco("train", partial, resolution=512) # AttributeError
Additional
No response
Are you willing to submit a PR?
Search before asking
Bug
Bug
The three dataset builders read the same set of pipeline options with three different access disciplines, and the two that use
getattrfallbacks pick fallback values that contradict the config defaults. A caller passing a partialargsnamespace silently gets a different training pipeline instead of an error.build_cocouses direct attribute access (coco.py:1335-1338):(and again at
coco.py:1351-1355/1378-1382, which addskip_random_resize=not args.do_random_resize_via_padding)build_roboflow_from_coco(coco.py:1422-1428) andbuild_roboflow_from_yolo(yolo.py:1054-1060) usegetattrwith a literal fallback for the same fields. Five of those fallbacks disagree with the real default:getattrfallbacksquare_resize_div_64FalseTrueTrainConfig(config.py:1061)segmentation_headFalseTrueon every seg variantModelConfigmulti_scaleFalseTrueTrainConfig(config.py:1066)expanded_scalesFalseTrueTrainConfig(config.py:1067)num_windows42for every released variant (4only on the deprecated base)ModelConfig— required field, no defaultpatch_size1616nano/small/medium/large,14base,12seg + keypointModelConfig— required field, no defaultpatch_sizeandnum_windowsare declared without defaults onModelConfig(config.py:481-482), so there is no correct constant to fall back to. The pair the builders actually choose —(16, 4)— matches no shipped variant.The keypoint fields (
use_grouppose_keypoints,num_keypoints_per_class) diverge on the keypoint variant too, but their fallback is deliberate and documented atcoco.py:1301— absence means "detection-only", which is a safe default. The five above have no such reading.Why it is reachable
build_coco,build_roboflow_from_cocoandbuild_roboflow_from_yoloare all publicly re-exported fromrfdetr.datasets(__init__.py:27,29).coco.py:1321-1325documents calling a builder directly, bypassing the DataModule, as supported usage:test_coco.py:632passestypes.SimpleNamespace(dataset_dir=..., augmentation_backend="gpu")with none of these fields set, which only works because the fallbacks absorb them.To be fair to the current code: on the
RFDETRDataModulepath this is inert._namespace_from_configspopulates every one of these fields, so normalmodel.train(...)is unaffected. The exposure is the documented direct-call path — and the asymmetry means one builder fails loudly there while its two siblings quietly mistrain.Measured impact
Same partial namespace,
resolution=512:num_windowsalone reshapes the multi-scale scale set, since it feedscompute_multi_scale_scales():It also feeds
draft_size_for_transforms(), so the JPEG draft-decode floor moves (672 → 600 here), decoding images below the largest training scale.Proposed fix
Make the three builders agree on one discipline. Two options, and I would rather have your call than guess:
A. Direct attribute access everywhere (match
build_coco). Fails loudly on an incomplete namespace instead of substituting a wrong pipeline. On the supported DataModule pathargsis always complete, so nothing regresses. Cost: the handful of tests passing partial namespaces need the fields added.B. Keep
getattr, but source the fallbacks from the config field defaults rather than restating them as literals, so they cannot drift again. This does not work forpatch_size/num_windows, which have no default to read — those would still need option A.I lean toward A for the five fields that feed the resize pipeline, since a silently different scale distribution is worse than an
AttributeError, andpatch_size/num_windowshave no defensible fallback. Happy to implement whichever you prefer, with tests covering builder parity so the three cannot drift apart again.Environment
1.10.0.dev1,develop@6674d858, installed with-e .Minimal Reproducible Example
Additional
No response
Are you willing to submit a PR?