masks_to_boxes Does Not Enforce 0 <= x1 < x2 and 0 <= y1 < y2, Leading to Invalid Bounding Boxes

[H4nz0u]

🐛 Describe the bug

def masks_to_boxes(masks: torch.Tensor) -> torch.Tensor:
    """
    Compute the bounding boxes around the provided masks.

    Returns a [N, 4] tensor containing bounding boxes. The boxes are in ``(x1, y1, x2, y2)`` format with
    ``0 <= x1 < x2`` and ``0 <= y1 < y2``.

    Args:
        masks (Tensor[N, H, W]): masks to transform where N is the number of masks
            and (H, W) are the spatial dimensions.

    Returns:
        Tensor[N, 4]: bounding boxes
    """
    if not torch.jit.is_scripting() and not torch.jit.is_tracing():
        _log_api_usage_once(masks_to_boxes)
    if masks.numel() == 0:
        return torch.zeros((0, 4), device=masks.device, dtype=torch.float)

    n = masks.shape[0]

    bounding_boxes = torch.zeros((n, 4), device=masks.device, dtype=torch.float)

    for index, mask in enumerate(masks):
        y, x = torch.where(mask != 0)

        bounding_boxes[index, 0] = torch.min(x)
        bounding_boxes[index, 1] = torch.min(y)
        bounding_boxes[index, 2] = torch.max(x)
        bounding_boxes[index, 3] = torch.max(y)

    return bounding_boxes
    

This is the official implementation of the masks_to_boxes function intended to calculate the bounding box for a given mask. The "bug" hereby is that while the documentation clearly states that 0 <= x1 < x2 and 0 <= y1 < y2 but this format clearly is not enforced. Returning a Bounding Box with the width or height 0 can result in significant issues, since e.g. some model architectures like Mask R-CNN normalize the ground truth bounding box before loss calculation. This normalization with a bounding height/width of 0 leads to the coordinates being nan which results in the loss being nan which then destabelizes the training process. And due to the missleading documentation users look for the root cause of their loss being nan at a lot of places but this one.

Due to the potential consequences of such a misformated bounding box, I would recommend to enforce the documented behavior and to raise an error if this condition is violated. Alternatively, the bounding box could be expanded by a single pixel but this might lead to inaccuracies for very small objects. At the very least the documentation should be adapted and the user should be made aware to check for this edge case for themself.

Versions

Collecting environment information...
PyTorch version: 2.1.2+cu121
Is debug build: False
CUDA used to build PyTorch: 12.1
ROCM used to build PyTorch: N/A

OS: AlmaLinux 9.4 (Seafoam Ocelot) (x86_64)
GCC version: (GCC) 11.4.1 20231218 (Red Hat 11.4.1-3)
Clang version: Could not collect
CMake version: Could not collect
Libc version: glibc-2.34

Python version: 3.11.3 (main, Oct 1 2024, 01:42:12) [GCC 12.3.0] (64-bit runtime)
Python platform: Linux-5.14.0-427.33.1.el9_4.x86_64-x86_64-with-glibc2.34
Is CUDA available: False
CUDA runtime version: No CUDA
CUDA_MODULE_LOADING set to: N/A
GPU models and configuration: No CUDA
Nvidia driver version: No CUDA
cuDNN version: No CUDA
HIP runtime version: N/A
MIOpen runtime version: N/A
Is XNNPACK available: True

CPU:
Architecture: x86_64
CPU op-mode(s): 32-bit, 64-bit
Address sizes: 52 bits physical, 57 bits virtual
Byte Order: Little Endian
CPU(s): 128
On-line CPU(s) list: 0-63
Off-line CPU(s) list: 64-127
Vendor ID: AuthenticAMD
Model name: AMD EPYC 9334 32-Core Processor
CPU family: 25
Model: 17
Thread(s) per core: 1
Core(s) per socket: 32
Socket(s): 2
Stepping: 1
Frequency boost: enabled
CPU(s) scaling MHz: 69%
CPU max MHz: 3910.2529
CPU min MHz: 0.0000
BogoMIPS: 5400.13
Flags: fpu vme de pse tsc msr pae mce cx8 apic sep mtrr pge mca cmov pat pse36 clflush mmx fxsr sse sse2 ht syscall nx mmxext fxsr_opt pdpe1gb rdtscp lm constant_tsc rep_good amd_lbr_v2 nopl nonstop_tsc cpuid extd_apicid aperfmperf rapl pni pclmulqdq monitor ssse3 fma cx16 pcid sse4_1 sse4_2 movbe popcnt aes xsave avx f16c rdrand lahf_lm cmp_legacy extapic cr8_legacy abm sse4a misalignsse 3dnowprefetch osvw ibs skinit wdt tce topoext perfctr_core perfctr_nb bpext perfctr_llc mwaitx cpb cat_l3 cdp_l3 hw_pstate ssbd mba perfmon_v2 ibrs ibpb stibp ibrs_enhanced vmmcall fsgsbase bmi1 avx2 smep bmi2 erms invpcid cqm rdt_a avx512f avx512dq rdseed adx smap avx512ifma clflushopt clwb avx512cd sha_ni avx512bw avx512vl xsaveopt xsavec xgetbv1 xsaves cqm_llc cqm_occup_llc cqm_mbm_total cqm_mbm_local avx512_bf16 clzero irperf xsaveerptr rdpru wbnoinvd amd_ppin cppc arat npt lbrv svm_lock nrip_save tsc_scale vmcb_clean flushbyasid decodeassists pausefilter pfthreshold avic v_vmsave_vmload vgif x2avic v_spec_ctrl vnmi avx512vbmi umip pku ospke avx512_vbmi2 gfni vaes vpclmulqdq avx512_vnni avx512_bitalg avx512_vpopcntdq la57 rdpid overflow_recov succor smca fsrm flush_l1d debug_swap
L1d cache: 2 MiB (64 instances)
L1i cache: 2 MiB (64 instances)
L2 cache: 64 MiB (64 instances)
L3 cache: 256 MiB (8 instances)
NUMA node(s): 8
NUMA node0 CPU(s): 0-7
NUMA node1 CPU(s): 8-15
NUMA node2 CPU(s): 16-23
NUMA node3 CPU(s): 24-31
NUMA node4 CPU(s): 32-39
NUMA node5 CPU(s): 40-47
NUMA node6 CPU(s): 48-55
NUMA node7 CPU(s): 56-63
Vulnerability Gather data sampling: Not affected
Vulnerability Itlb multihit: Not affected
Vulnerability L1tf: Not affected
Vulnerability Mds: Not affected
Vulnerability Meltdown: Not affected
Vulnerability Mmio stale data: Not affected
Vulnerability Retbleed: Not affected
Vulnerability Spec rstack overflow: Mitigation; Safe RET
Vulnerability Spec store bypass: Mitigation; Speculative Store Bypass disabled via prctl
Vulnerability Spectre v1: Mitigation; usercopy/swapgs barriers and __user pointer sanitization
Vulnerability Spectre v2: Mitigation; Enhanced / Automatic IBRS, IBPB conditional, STIBP always-on, RSB filling, PBRSB-eIBRS Not affected
Vulnerability Srbds: Not affected
Vulnerability Tsx async abort: Not affected

Versions of relevant libraries:
[pip3] numpy==1.26.4
[pip3] torch==2.1.2+cu121
[pip3] torchaudio==2.1.2+cu121
[pip3] torchvision==0.16.2+cu121
[conda] Could not collect

[NicolasHug]

Thanks for the report @H4nz0u .

From what I can tell, the situation where the output has x1 = x2 or y1 = y2 only happens if the input mask is a single line / single column? E.g.:

masks = torch.tensor([[
    [0, 0, 0, 0],
    [0, 1, 1, 0],
    [0, 0, 0, 0],
    [0, 0, 0, 0],
]])
masks

masks_to_boxes(masks) # Gives tensor([[1., 1., 2., 1.]])

The docs aren't really accurate here, that's fair. I'm not sure if raising an error in such cases is desirable - it might have make sense to do it when this function was introduced, but doing it know would potentially cause existing code to crash.

I guess the best way forward would be to edit the docs to better reflect the output format, and let users know that they should enforce their own constraints if needed.

[H4nz0u]

Thank you very much for your quick response.

You are right, masks with a height/width of 1 would be the only uncovered edge case where this could occur.

However, I am unsure if adapting the documentation and leaving this edge case to the user is the right way to proceed. At the end of the day, the function receives a non-empty mask and produces a bounding box with the area 0. Since the entire point of the bounding box is to encompass the existing (but admittedly very small) object, producing an empty bounding box does not make much sense. Additionally, the fact that the area is 0 would lead to unexpected behavior downstream. Either with issues like mine where the bounding box normalization of a well-established framework (mmdetection) results in a nan loss or a visualization does not show a bounding box for an existing mask or what else.

Breaking legacy code should obviously always be avoided but since the current behavior does not make much sense for the intended purpose of a bounding box and results in very hard-to-detect problems when this edge case occurs, it might be worth considering a more holistic solution.

When raising an error is not an option, maybe the bounding box could be expanded by 1 pixel to better reflect the fact that there is actual content within or simply return an entirely empty box with torch.zeros((0, 4), device=masks.device, dtype=torch.float).
I think the approach there would come down to how you formally want to define what a bounding box is and isn't but I am not experienced enough to come to a conclusion.