Skip to content

Fix logical error in true negative calculation - #447

Open
mhd-medfa wants to merge 1 commit into
IDEA-Research:mainfrom
Unified-Promptable-Panoptic-Mapping:main
Open

Fix logical error in true negative calculation#447
mhd-medfa wants to merge 1 commit into
IDEA-Research:mainfrom
Unified-Promptable-Panoptic-Mapping:main

Conversation

@mhd-medfa

Copy link
Copy Markdown
Contributor

Fix two copy-paste errors in APOPMeter that caused self.fn to never be incremented, silently corrupting any metrics derived from false negatives (recall, F1, miss rate, etc.):

  • update(): line 291 wrote to self.tn with the FP condition (pred==1, gt==0) instead of self.fn with the FN condition (pred==0, gt==1)
  • update_cm(): line 297 accumulated fn into self.tn instead of self.fn

Fix two copy-paste errors in APOPMeter that caused self.fn to never
be incremented, silently corrupting any metrics derived from false
negatives (recall, F1, miss rate, etc.):

- update(): line 291 wrote to self.tn with the FP condition (pred==1,
  gt==0) instead of self.fn with the FN condition (pred==0, gt==1)
- update_cm(): line 297 accumulated fn into self.tn instead of self.fn
phreed added a commit to phreed/GroundingDINO that referenced this pull request Aug 14, 2026
…suite

Merges every pull request open at github.com/IDEA-Research/GroundingDINO/pulls,
retaining only what belongs to the library, its documentation or its tests.
Against upstream main 856dde2: 63 files changed, 37 added, 26 modified, none
deleted. Upstream's own 67 files are all still present.

Earlier states are preserved as tags: "pre-integration-backup" is upstream main
untouched, "pre-squash" is the full 138-commit history with each PR a separate
merge commit carrying its original authorship, and "full-integration" is this
work with the PR IDEA-Research#452 application still included.

=======================================================================
1. PRs merged
=======================================================================
29 applied cleanly; 8 needed hand resolution.

  IDEA-Research#277 Bugfix/prompt ending
  IDEA-Research#279 Batched inference API and support for float16 inference
  IDEA-Research#295 Fix gradio demo
  IDEA-Research#299 Update inference.py
  IDEA-Research#313 Update inference.py
  IDEA-Research#319 Fix/bert uncased
  IDEA-Research#329 Edited docker_test.py file
  IDEA-Research#346 Quick fix to avoid error when not enough proposals post augmentation
  IDEA-Research#349 MPS Fix: masked_fill_ returning incorrect result on mac GPU
  IDEA-Research#357 Update docker_test.py
  IDEA-Research#371 Flexible requirements (opencv-python or opencv-python-headless)
  IDEA-Research#372 Update fixed README.md typo
  IDEA-Research#376 Single class train and coco format dataset loading
  IDEA-Research#380 onnx输出支持 (ONNX export support)
  IDEA-Research#381 Fix Dockerfile to ensure proper build
  IDEA-Research#383 Cuda 11.8 support
  IDEA-Research#393 make it compatible with torch 2.6.0
  IDEA-Research#409 Add torch2.6 support for ms_deform_attn_cuda
  IDEA-Research#414 [fix] Change 'pyy' to 'py'
  IDEA-Research#415 Fix for pytorch 2.8 - Replace value.type() with value.scalar_type
  IDEA-Research#417 Bug Fixes for CUDA Custom Ops and timm Layer Import
  IDEA-Research#419 Add pyproject.toml
  IDEA-Research#423 make sure right_idx bigger than left_idx
  IDEA-Research#424 Support 5090
  IDEA-Research#433 Fix typo in docker_test.py
  IDEA-Research#447 Fix logical error in true negative calculation
  IDEA-Research#449 Add ROCm inference support for MsDeformAttn
  IDEA-Research#452 IP Camera 多路即時串流監測與 UI 看板系統
  IDEA-Research#453 [Fix] inference_on_a_image: UnboundLocalError text_prompt (IDEA-Research#390)
  IDEA-Research#455 fix: update BoxAnnotator API for supervision >=0.16
  IDEA-Research#456 Fix load_model ignoring device parameter
  IDEA-Research#458 fix: restore compatibility with transformers >= 4.49
  IDEA-Research#459 fix: include cuda_fp16.h in the MsDeformAttn CUDA kernel
  IDEA-Research#460 feat: expose __version__ on the groundingdino package
  IDEA-Research#461 feat: extend the SwinB config with training parameters
  IDEA-Research#462 fix: replace deprecated Tensor::type() so the CUDA kernel compiles
  IDEA-Research#463 chore: add uv-based local setup script

PR IDEA-Research#452 contributed roughly 500 files: an IP-camera monitoring application and
three dashboards, none of which GroundingDINO imports. It was merged, examined,
then dropped as a consumer of the library rather than part of it. Its one
vendored third-party component, a verbatim copy of github.com/grafana/mcp-grafana
carrying that project's own go.mod, LICENSE, CODEOWNERS and CI, went with it;
conda-forge packages it as mcp-grafana if it is ever wanted back.

=======================================================================
2. Conflict resolutions that changed the outcome
=======================================================================
IDEA-Research#409, IDEA-Research#415 and IDEA-Research#462 are recorded merges with no tree change. Each re-does the
Tensor::type() -> scalar_type() modernisation IDEA-Research#383 had already landed, in an
older form (AT_ASSERTM, .data<T>(), non-half dispatch).

IDEA-Research#380 replaced GroundingDINO.forward() with a signature taking pre-computed BERT
output, which would have broken all four caption-based callers. Main already had
a forward()/forward_nn() split, so the caption API was kept and forward_nn()
gained an optional last_hidden_state: pass it to skip the internal BERT call,
omit it for the previous behaviour. Its torch.amp.autocast(str(tgt.device)) was
corrected to tgt.device.type, since 'cuda:0' is not a device type. Its global
use_checkpoint=False was reverted, as that regresses training added by IDEA-Research#376 and
IDEA-Research#461; load_model() instead gained a config_overrides argument.

IDEA-Research#452 renamed groundingdino/ to groundingdino_source/ -- a local workaround per
its own development log, contradicted by its own setup.py. Not carried over.
Its LICENSE replacement was MediaMTX's MIT licence, Copyright (c) 2019 aler9,
evidently pasted by accident; Apache-2.0 is kept. Its transformers==4.33.2 pin
would have undone IDEA-Research#458.

IDEA-Research#393, IDEA-Research#417, IDEA-Research#449 and IDEA-Research#279 resolved by keeping whichever side was newer, and
unioning where both changes were wanted.

=======================================================================
3. Defects found and fixed
=======================================================================
ExportFlag.is_export read off the class returns the property object, which is
always truthy. The guard from IDEA-Research#277, carried into IDEA-Research#449's use_custom_kernel,
therefore evaluated "not ExportFlag.is_export" as False every time, silently
forcing every forward pass down the pure-PyTorch fallback even on CUDA. Now
read through ExportFlag.current(). ExportFlag.__enter__ also stored the bound
classmethod instead of calling it.

APOPMeter.update() never recorded false negatives. PR IDEA-Research#447 corrected the mask to
(pred==0, gt==1) but kept adding the result to self.tn, so fn was always zero
and tn inflated. Completed here; the four counters now sum to the sample count.

torch.amp.autocast was passed a device index rather than a device type (IDEA-Research#380).

Two defects in PR IDEA-Research#452 code were fixed before that code was dropped, and are
recorded here only because the same bugs may resurface if it is ever restored:
get_class_mapping() did not skip blank lines, and convert_to_yolo_format()
clamped centre and size independently so boxes could still leave the image.

=======================================================================
4. What is added
=======================================================================
Inside the library:
  groundingdino/version.py                          IDEA-Research#460
  groundingdino/util/export_flag.py                 IDEA-Research#277
  groundingdino/config/GroundingDINO_SwinT_OGC_export.py  IDEA-Research#380
  groundingdino/models/.../ms_deform_attn_hip.h     IDEA-Research#449, ROCm forward kernel
  groundingdino/models/.../ms_deform_attn_hip_forward.cu

Alongside it:
  export_onnx/    IDEA-Research#277, IDEA-Research#380   ONNX and TensorRT export, plus test/cat.jpg.
                               Note export_model.py and export.py are two
                               independent exporters and are not reconciled.
  training/       IDEA-Research#376         single-class training and COCO loading
  requirements/   IDEA-Research#371         runtime/cv2/cv2-headless split that setup.py reads
  bin/            IDEA-Research#463         uv-based setup script
  demo/           IDEA-Research#279         batched float16 notebook and nine sample images
  scripts/setup_environment.py  diagnoses groundingdino._C loading
  pyproject.toml  IDEA-Research#279/IDEA-Research#417/IDEA-Research#419
  pixi.toml                    conda dependencies for default, export and test

=======================================================================
5. Tests
=======================================================================
Upstream had no test suite: two files named test_*, neither containing an
assertion, and no CI.

tests/test_upstream_regressions.py demonstrates that the merged changes were
needful, on the standard that a change is needful only if its absence is
observable. Every test was also run against a worktree of
pre-integration-backup, where the file reports 11 failed and 1 passed -- the
pass being the deliberate control. Nine PRs are covered this way.

The decisive case is IDEA-Research#458: on upstream main build_model() raises
"AttributeError: 'BertModel' object has no attribute 'get_head_mask'" under
transformers 5.x, so the library cannot construct a model at all.

Result: 12 passed. Coverage and its limits are recorded in tests/README.md.
The CUDA and HIP kernel work (IDEA-Research#383, IDEA-Research#409, IDEA-Research#415, IDEA-Research#424, IDEA-Research#449, IDEA-Research#459, IDEA-Research#462), the
MPS fixes (IDEA-Research#349) and the fp16 kernel path (IDEA-Research#279) cannot be exercised without
the hardware. IDEA-Research#417 and IDEA-Research#393 were checked and are forward-looking rather than
currently broken: timm 1.0.28 still ships the timm.models.layers shim, and
torch 2.13 only warns about meshgrid indexing= and checkpoint use_reentrant=.

=======================================================================
6. Verification
=======================================================================
groundingdino imports and reports version 0.1.0; build_model() succeeds on CPU;
forward() and forward_nn() produce bitwise-identical output; tests 12 passed;
find_packages() returns the 7 groundingdino.* packages; pixi resolves 3
environments; documentation links resolve.

Not verified: the CUDA and HIP kernels are uncompiled, no GPU or nvcc being
available, and the ONNX export path was not run end to end.

=======================================================================
Attribution
=======================================================================
Squashing removes the per-PR authorship the "pre-squash" tag still carries.
The upstream contributors whose work is included:

Abb :D, Alex, Andrew Choi, andy371643, Artemis Panagopoulou,
ASHWIN UNNIKRISHNAN, awpbash, ayushpixis5941, dcaustin33, Deltaheavy6,
dpkimball-cursor, Dr. Artificial曾小健, Freax Ruby, Functionhx, hujian,
Jiani Huang, jinwoo, joncrall, jvarshini99, Kenshi Takayama,
Mohamad Al Mdfaa, pftq, PierreMarieCurie, Ren Tianhe, robostoneDK,
Salvador Belenguer, Skwarson96, szsteven008, Xiaoke JIANG, Yu-Chung Wang,
Yushuo Sun, zhengjia, Ziqi Gao (Roy), zjli2013, 叨叨

Co-Authored-By: Claude Opus 5 (1M context) <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.

1 participant