feat(training): hotcoco as default COCO evaluator - #1402
Open
Borda wants to merge 1 commit into
Open
Conversation
- Add `TrainConfig.eval_backend`, a `Literal["hotcoco", "faster_coco_eval"]` field defaulting to `"hotcoco"`, selecting the COCO evaluator used for detection and segmentation validation and test mAP. `build_trainer` passes it to `COCOEvalCallback`, which forwards it to the validation, train-split and EMA metrics alike; `OnePassCocoMeanAveragePrecision` now validates its `backend` argument against `_SUPPORTED_BACKENDS` instead of pinning it to `faster_coco_eval`. - Add `_HotCocoBackend`, a TorchMetrics `CocoBackend` subclass that overrides the resolved COCO, evaluator and mask surfaces. TorchMetrics resolves those from a closed backend-name enum with no hotcoco member, so the parent is constructed with the supported `faster_coco_eval` name and the three surfaces are replaced afterwards; faster-coco-eval therefore remains a required dependency. Every private helper the adapter calls on the backend stays TorchMetrics' own. - Work around four hotcoco behaviors that return a silently wrong answer rather than raising, each covered by a test that fails if the handling is dropped: `params` is assigned as a whole object because its getter returns a copy, which would otherwise leave `maxDets` at COCO's default 100; `_coco_datasets` returns the source COCO-format dictionary as a third element because hotcoco's constructor keeps only the fields it recognizes and drops the `area_bbox`/`area_segm` values a multi-IoU-type evaluation switches between; `_build_coco` decodes bytes RLE `counts` to `utf-8` because the constructor reads a bytes payload back as an empty mask and collapses mask AP to `0.0`; and `_HotCocoMaskUtils.encode` coerces TorchMetrics' boolean mask array to `uint8`, which the Rust binding requires. - Load box-only hotcoco predictions through `loadRes` as one `[image_id, x, y, width, height, score, category_id]` array instead of the per-detection annotation dictionaries TorchMetrics materializes, which dominate `compute()` once the evaluator itself is fast. Rows stay in stored-state order so equal-scoring detections keep the tie order the dictionary path produced, and boxes need no conversion because `update()` already stored them as COCO `xywh`. Segmentation, the `faster_coco_eval` backend, and states holding no boxes keep the dictionary path; `_prediction_dataset_for_iou_type` raises if a multi-IoU-type evaluation ever reaches an array-built dataset. - Silence hotcoco's evaluation output at the file-descriptor level via `_silenced_rust_output`, since it prints from Rust and `contextlib.redirect_stdout` does not reach it. The window covers only `evaluate`, `accumulate` and `summarize`, so the adapter's own contract-failure logging is not suppressed along with the backend's per-parameter warnings. Failures still surface as Python exceptions. - Pass the IoU type by each backend's own keyword rather than positionally, so a parameter inserted before it upstream cannot bind a detection run to segmentation. - Skip `_get_coco_datasets` in `_validate_private_contract` when the hotcoco backend is active: that helper ends with a `dataset` assignment and `createIndex()` call, neither of which hotcoco supports, so the adapter never calls it and an upstream rename must not block hotcoco users. - Add `hotcoco>=0.5,<0.6` to the `train` extra. The ceiling is deliberate rather than conventional: three of the worked-around behaviors fail silently, so each pre-1.0 minor has to be re-verified against the parity tests before it is allowed in. - Add parity tests asserting exact equality across every aggregate, per-class and class-ID output of both backends for box-only and box-plus-mask evaluation, plus coverage for `maxDets` reaching the evaluator, for evaluation printing nothing, for the missing-dependency `ImportError` naming the extra, and for each IoU type's annotation areas driving its own COCO size buckets. - Document the knob in the training-parameters table and the `[train]` extra description, and record the default change in the changelog. --- Co-authored-by: claude[bot] <209825114+claude[bot]@users.noreply.github.com>
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
Contributor
There was a problem hiding this comment.
Pull request overview
Adds hotcoco as the default training-time COCO evaluator while retaining faster-coco-eval as a fallback.
Changes:
- Adds backend selection throughout training configuration and callbacks.
- Implements hotcoco adaptation and a detection-array fast path.
- Adds parity/regression tests, dependency metadata, and documentation.
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
src/rfdetr/config.py |
Defines eval_backend. |
src/rfdetr/training/trainer.py |
Forwards backend configuration. |
src/rfdetr/training/callbacks/coco_eval.py |
Applies the backend to all metrics. |
src/rfdetr/training/coco_map.py |
Implements hotcoco integration and optimizations. |
tests/training/test_coco_map.py |
Tests parity and compatibility behavior. |
tests/training/callbacks/test_coco_eval_callback.py |
Tests callback backend propagation. |
pyproject.toml |
Adds the hotcoco dependency. |
docs/learn/train/training-parameters.md |
Documents backend selection. |
CHANGELOG.md |
Records the evaluator change. |
AGENTS.md |
Updates dependency guidance. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| segmentation: bool = False, | ||
| eval_interval: int = 1, | ||
| log_per_class_metrics: bool = True, | ||
| eval_backend: Literal["hotcoco", "faster_coco_eval"] = "hotcoco", |
Comment on lines
+144
to
+148
| saved = {descriptor: os.dup(descriptor) for descriptor in (1, 2)} | ||
| devnull = os.open(os.devnull, os.O_WRONLY) | ||
| try: | ||
| for descriptor in saved: | ||
| os.dup2(devnull, descriptor) |
Comment on lines
+140
to
+142
| eval_backend: COCO evaluation backend, mirroring :attr:`~rfdetr.config.TrainConfig.eval_backend`. Both | ||
| backends return identical metrics; ``"hotcoco"`` is the faster default and ``"faster_coco_eval"`` is | ||
| the previous evaluator. |
Comment on lines
+907
to
+913
| def missing_dependency() -> Any: | ||
| raise ImportError("hotcoco requires the optional dependency; install it with: pip install 'rfdetr[hotcoco]'") | ||
|
|
||
| monkeypatch.setattr("rfdetr.training.coco_map._hotcoco", missing_dependency) | ||
|
|
||
| with pytest.raises(ImportError, match=r"rfdetr\[hotcoco\]"): | ||
| OnePassCocoMeanAveragePrecision(backend="hotcoco") |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## develop #1402 +/- ##
=======================================
Coverage 86% 86%
=======================================
Files 114 114
Lines 14877 14976 +99
=======================================
+ Hits 12832 12928 +96
- Misses 2045 2048 +3 🚀 New features to boost your workflow:
|
Member
Author
|
cross-ref: derekallman/hotcoco#5 |
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.
Summary
Detection and segmentation COCO evaluation (validation and test mAP) now runs on hotcoco, a Rust COCO evaluator distributed under MIT with
numpyas its only runtime dependency. The newTrainConfig.eval_backendknob selects between it and the previous evaluator:Reported metrics do not change. The parity tests compare every aggregate, per-class and class-ID output of both backends for box-only and box-plus-mask evaluation and require exact equality (
rtol=0, atol=0), which they reach.faster-coco-evalis not removed and is not optional. TorchMetrics resolves its COCO helpers from a closed backend-name enum with no hotcoco member, so the adapter constructs the backend under the supported name and replaces the three resolved module surfaces (coco,cocoeval,mask_utils) afterwards. Every private helper the adapter calls — COCO-format construction, statistics conversion — stays TorchMetrics' own and is shared by both backends.Keypoint evaluation uses its own OKS path and is untouched. The ONNX/TensorRT benchmark evaluator in
rfdetr.evaluation.coco_evalcontinues to callfaster-coco-evaldirectly.What changed
TrainConfig.eval_backend: Literal["hotcoco", "faster_coco_eval"] = "hotcoco", threaded throughbuild_trainerintoCOCOEvalCallbackand from there into all three metric constructions: validation, train split, and EMA._HotCocoBackend(CocoBackend)insrc/rfdetr/training/coco_map.py, plus_HotCocoMaskUtilsfor the one mask utility that needs input coercion.[image_id, x, y, width, height, score, category_id]array throughCOCO.loadResinstead of as one Python dictionary per detection.contextlib.redirect_stdoutdoes not reach it.hotcoco>=0.5,<0.6added to thetrainextra.Measured impact
Both backends driven through
OnePassCocoMeanAveragePrecisionitself — not through a standalone harness — on synthetic accumulated state with a plausible score and IoU distribution at each dataset's real image, annotation and class counts.eval_max_dets=500, macOS CPU, single machine.class_metricsThese are
compute()timings. They do not include the validation forward pass, which usually dominates a validation epoch.Why the first integration only reached 1.7x
The first working version swapped the evaluator and nothing else, and measured 1.7x. The stage split at COCO-val scale shows why that was the wrong place to stop:
The evaluator itself was already about 8x faster after the swap, but 83% of what remained was TorchMetrics building one Python dictionary per detection — over 1.5 million of them at that scale — which a backend swap does not touch. The detection-array path removes that work rather than accelerating it. Boxes need no conversion on that path because TorchMetrics already stores them as COCO
xywhatupdate()time, and rows stay in stored-state order so equal-scoring detections keep the tie order the dictionary path produced.The array path is deliberately narrow. It is skipped for
faster_coco_eval, whose ownloadResis slower than the dictionary path it would replace (3.7 s against 1.8 s at COCO-val scale); for mask evaluation, where an array carries no segmentation; and for states holding no boxes._prediction_dataset_for_iou_typeraises rather than proceeding if a multi-IoU-type evaluation ever reaches an array-built dataset.Projected impact on the training loop
This is arithmetic on two prior measurements, not a run. No training epoch was timed with this branch. The projection is included because "4x faster evaluation" invites the wrong conclusion about epoch time, and the honest answer is that it is small.
Anchor. The M1 profiling run (
.plans/closed/results_training-perf.md): Colab L4,rfdetr-small, Playing-Cards val split (1,978 images, 39 classes, 300 queries per image), median 323.8 s/epoch — train 256.5 s (79.2%),val_bar46.0 s, post-validation gap 21.3 s. In that runmap_computewas 17.1 s/epoch across the base and EMA metrics (8.5 s + 8.6 s), 5.3% of the epoch and 80% of the post-validation gap. Two changes have landed since and both shrink it before this PR applies: PR11 (the_get_coco_formatscore hoist) took it to roughly 15 s/epoch for both metrics, and PR12 made EMA-only evaluation the default, so the default path computes one metric rather than two — call it ~7.5 s/epoch.Method and its limits. The same source measures the machine behind the microbenchmarks above as roughly 3.7x faster than the Colab L4 vCPU, so absolute seconds do not transfer between the two. Only the ratio is carried over, and carrying it over assumes the speedup factor is insensitive to CPU speed. That assumption is untested: the removed dictionary construction is single-threaded Python and the remaining evaluation is native code, and those two do not necessarily scale together across machines.
map_computebeforeeval_base_model=False,log_per_class_metrics=False)log_per_class_metrics=Trueeval_base_model=True(two metrics)The segmentation row is the weakest of the five: the array path does not apply there, so only the evaluator swap contributes, and that 1.7x was measured on box state rather than mask state. Treat it as an order of magnitude, not a figure.
Verdict: this is not a training-throughput change. On the anchor workload it is worth under 2% of an epoch on the default path, and proportionally less as the dataset or model grows, because the validation forward pass and training step both scale while
compute()scales with detection count alone. It matters wherecompute()is the whole job rather than a rounding error next to the forward pass: standalone evaluation of a checkpoint, large validation sets, sweeps that evaluate far more often than they train, andeval_interval=1on small models. On COCO-val-shaped state that job goes from 6.4 s to 1.5 s.Compatibility traps handled
Five behaviors differ from
faster-coco-evalin ways the adapter has to absorb. Four of them produce a silently wrong number rather than an error, so each has a dedicated test that fails if the handling is dropped.paramsis copy-on-read.ev.params.maxDets = [...]is a no-op. The adapter assigns the whole object back. Mutating in place — which both TorchMetrics and RF-DETR previously did — would have silently reducedeval_max_dets=500to COCO's default 100.datasetis also copy-on-read, and the constructor discards unrecognized fields. Thearea_bbox/area_segmswap that a("bbox", "segm")evaluation performs would be invisible to the evaluator, leaking one IoU type's areas into the other's COCO size buckets. The adapter keeps the source dictionary and rebuilds. In the shipped regression fixture, dropping this doublesbbox_map_smallfrom 0.5 to 1.0.mask.encodereturns RLEcountsas bytes, and the COCO constructor decodes only the string form, reading a bytes payload back as an empty mask. Mask AP collapses to0.000with nothing raised. The adapter decodes to UTF-8.mask.encodeacceptsuint8only and rejects the boolean array TorchMetrics hands it. This one is loud, but with a confusingTypeError: 'ndarray' object is not an instance of 'ndarray'.iou_type, notiouType. Loud. The adapter dispatches the keyword per backend rather than passing it positionally: positional works on both today, but a parameter inserted before it upstream would then bind the IoU type to the wrong slot and evaluate a detection run as segmentation, silently.Separately, hotcoco writes to the standard output and error descriptors from Rust: a twelve-line summary table on descriptor 1, and one warning per evaluator parameter that differs from the COCO defaults on descriptor 2. RF-DETR overrides
maxDetsandiouThrson every evaluation, so those warnings would repeat each validation epoch and each IoU type while describing intended configuration. Both descriptors are redirected around the three evaluator calls only, so the adapter's own contract-failure logging is not swallowed with them; genuine failures still surface, because the bindings raise Python exceptions rather than reporting errors on descriptor 2.Tests
maxDetseffectiveness, parametrized over both backends.bbox_map_smallfrom 0.5 to 1.0. Verified by mutation — reverting the per-IoU-type rebuild fails it on both backends.compute().except ImportError, so the import has to happen eagerly in_HotCocoBackend.__init__to survive it._validate_private_contractskips_get_coco_datasetson the hotcoco path: that helper ends with adatasetassignment and acreateIndex()call, neither of which hotcoco supports, so the adapter never calls it and an upstream rename must not block hotcoco users over a call they do not make.tests/training/passes at 1182 passed, 15 skipped.pre-commit run --all-filesis clean.Packaging
hotcoco0.5.0 is MIT, depends onnumpyat runtime and nothing else, and shipscp39-abi3wheels for manylinux2014 x86_64 and aarch64, macOS x86_64 and arm64, and win_amd64.uv sync --dry-run --extra trainresolves it on every interpreter inrequires-python, 3.10 through 3.14.Two consequences worth knowing before merge:
rfdetr[train]will fall back to building from source, which requires a Rust toolchain. This is a new constraint on that platform, since every othertraindependency ships a musl wheel or is pure Python. No CI job covers musl today, so this is stated rather than measured.cococonsole script onPATH, which can shadow an unrelatedcococommand in the same environment.The
>=0.5,<0.6ceiling is deliberate rather than conventional. Three of the worked-around behaviors fail silently, so each pre-1.0 minor has to be re-verified against the parity tests before it is allowed in, and a tight ceiling is what forces that.Not measured
compute()is entered, so no collective is issued inside the descriptor-redirect window. That is an ordering argument from the code, not a measurement.Rollback
eval_backend="faster_coco_eval"restores the previous evaluator on a per-run basis with no reinstall — it remains a required dependency and a fully tested path. Reverting the commit removes the knob and thetrain-extra entry.