open-mmlab / mmdetection3d

OpenMMLab's next-generation platform for general 3D object detection.
https://mmdetection3d.readthedocs.io/en/latest/
Apache License 2.0
5.3k stars 1.54k forks source link

A replacement BUG in PointPillars feature extraction process #1150

Closed zzj403 closed 2 years ago

zzj403 commented 2 years ago

Describe the issue

The feature extracted here f_center = features[:, :, :2] without .clone() will cause the first 2 columns in original features be replaced by f_center in the subsequent processing. This violates the original paper's meaning(refer to https://openaccess.thecvf.com/content_CVPR_2019/papers/Lang_PointPillars_Fast_Encoders_for_Object_Detection_From_Point_Clouds_CVPR_2019_paper.pdf). in https://github.com/open-mmlab/mmdetection3d/blob/dfcdef394086fee3a0a2a5490ed465ec13ec6cec/mmdet3d/models/voxel_encoders/pillar_encoder.py#L125-L128

However, if I fix this bug by replace this line with f_center = features[:, :, :2].clone(), the predictions go wrong. If I leave this BUG, the predictions are good.

Checklist

  1. I have searched related issues but cannot get the expected help.
  2. The issue has not been fixed in the latest version(0.17.3).

Reproduction

  1. When does this issue happen? Any times you use PointPillars, this will happen. For example, when I demo PointPillars.

  2. What config dir you run? The config is :

    configs/pointpillars/hv_pointpillars_secfpn_6x8_160e_kitti-3d-3class.py

    And the checkpoint is:

    hv_pointpillars_secfpn_6x8_160e_kitti-3d-3class_20200620_230421-aa0f3adb.pth
  3. Did you make any modifications on the code or config? Did you understand what you have modified? Yeah. I added .clone() , and the prediction messed up. Yes, I understood.

Environment

sys.platform: linux
Python: 3.7.11 (default, Jul 27 2021, 14:32:16) [GCC 7.5.0]
CUDA available: True
GPU 0,1: Tesla V100-PCIE-16GB
GPU 2,3: GeForce GTX 1080 Ti
CUDA_HOME: /usr/local/cuda
NVCC: Build cuda_11.0_bu.TC445_37.28845127_0
GCC: gcc (Ubuntu 7.5.0-3ubuntu1~18.04) 7.5.0
PyTorch: 1.6.0
PyTorch compiling details: PyTorch built with:
  - GCC 7.3
  - C++ Version: 201402
  - Intel(R) oneAPI Math Kernel Library Version 2021.4-Product Build 20210904 for Intel(R) 64 architecture applications
  - Intel(R) MKL-DNN v1.5.0 (Git Hash e2ac1fac44c5078ca927cb9b90e1b3066a0b2ed0)
  - OpenMP 201511 (a.k.a. OpenMP 4.5)
  - NNPACK is enabled
  - CPU capability usage: AVX2
  - CUDA Runtime 10.2
  - NVCC architecture flags: -gencode;arch=compute_37,code=sm_37;-gencode;arch=compute_50,code=sm_50;-gencode;arch=compute_60,code=sm_60;-gencode;arch=compute_61,code=sm_61;-gencode;arch=compute_70,code=sm_70;-gencode;arch=compute_75,code=sm_75;-gencode;arch=compute_37,code=compute_37
  - CuDNN 7.6.5
  - Magma 2.5.2
  - Build settings: BLAS=MKL, BUILD_TYPE=Release, CXX_FLAGS= -Wno-deprecated -fvisibility-inlines-hidden -DUSE_PTHREADPOOL -fopenmp -DNDEBUG -DUSE_FBGEMM -DUSE_QNNPACK -DUSE_PYTORCH_QNNPACK -DUSE_XNNPACK -DUSE_VULKAN_WRAPPER -O2 -fPIC -Wno-narrowing -Wall -Wextra -Werror=return-type -Wno-missing-field-initializers -Wno-type-limits -Wno-array-bounds -Wno-unknown-pragmas -Wno-sign-compare -Wno-unused-parameter -Wno-unused-variable -Wno-unused-function -Wno-unused-result -Wno-unused-local-typedefs -Wno-strict-overflow -Wno-strict-aliasing -Wno-error=deprecated-declarations -Wno-stringop-overflow -Wno-error=pedantic -Wno-error=redundant-decls -Wno-error=old-style-cast -fdiagnostics-color=always -faligned-new -Wno-unused-but-set-variable -Wno-maybe-uninitialized -fno-math-errno -fno-trapping-math -Werror=format -Wno-stringop-overflow, PERF_WITH_AVX=1, PERF_WITH_AVX2=1, PERF_WITH_AVX512=1, USE_CUDA=ON, USE_EXCEPTION_PTR=1, USE_GFLAGS=OFF, USE_GLOG=OFF, USE_MKL=ON, USE_MKLDNN=ON, USE_MPI=OFF, USE_NCCL=ON, USE_NNPACK=ON, USE_OPENMP=ON, USE_STATIC_DISPATCH=OFF, 

TorchVision: 0.7.0
OpenCV: 4.5.4
MMCV: 1.4.0
MMCV Compiler: GCC 7.3
MMCV CUDA Compiler: 10.2
MMDetection: 2.14.0
MMSegmentation: 0.14.1
MMDetection3D: 0.17.2+b8706dd
Tai-Wang commented 2 years ago

This is a legacy issue. As indicated by the flag self.legacy, if it is set to True, we will take this version for implementation (the original release of PointPillars). In contrast, we will use the correct implementation when it is set to False. This problem is also fixed in the mentioned repo, but it can not bring any performance gain (even affect it), so we just keep it for reference.

zzj403 commented 2 years ago

This is a legacy issue. As indicated by the flag self.legacy, if it is set to True, we will take this version for implementation (the original release of PointPillars). In contrast, we will use the correct implementation when it is set to False. This problem is also fixed in the mentioned repo, but it can not bring any performance gain (even affect it), so we just keep it for reference.

Thanks, I understand this now. Does this mean that the first 2 columns of features (absolute x and y) are not useful in Pointpillars (even affect it)? This sounds wired.

Tai-Wang commented 2 years ago

Maybe. In my view, these features are not much important in voxel-based approaches for 3D object detection in large-scale scenes. What matters much more is the distribution of voxels (with non-empty features). Maybe these 2-dim noisy features can be compensated by other features.