open-compass / open-compass/opencompass

[Bug] Base classes use `@abstractmethod` without subclassing from `abc.ABC`

Open
#1,124 0 comments 0 reactions 2 assignees View on GitHub

@Leymore is already working on this.

Since May 8, 2024.

Dominant language
Python
Stars
7.5k
Forks
869
Avg merge
17h 52m
Merged PRs (30d)
13

Description

Prerequisite
Type

I have modified the code (config is not considered code), or I'm working on my own tasks/models/datasets.

Environment

{'CUDA available': False,
'GCC': 'gcc (GCC) 4.8.5 20150623 (Red Hat 4.8.5-44)',
'MMEngine': '0.10.3',
'MUSA available': False,
'OpenCV': '4.9.0',
'PyTorch': '2.2.1',
'PyTorch compiling details': 'PyTorch built with:\n'
' - GCC 9.3\n'
' - C++ Version: 201703\n'
' - Intel(R) oneAPI Math Kernel Library Version '
'2023.1-Product Build 20230303 for Intel(R) 64 '
'architecture applications\n'
' - Intel(R) MKL-DNN v3.3.2 (Git Hash '
'2dc95a2ad0841e29db8b22fbccaf3e5da7992b01)\n'
' - OpenMP 201511 (a.k.a. OpenMP 4.5)\n'
' - LAPACK is enabled (usually provided by '
'MKL)\n'
' - NNPACK is enabled\n'
' - CPU capability usage: AVX512\n'
' - Build settings: BLAS_INFO=mkl, '
'BUILD_TYPE=Release, CUDA_VERSION=12.1, '
'CUDNN_VERSION=8.9.2, '
'CXX_COMPILER=/opt/rh/devtoolset-9/root/usr/bin/c++, '
'CXX_FLAGS= -D_GLIBCXX_USE_CXX11_ABI=0 '
'-fabi-version=11 -fvisibility-inlines-hidden '
'-DUSE_PTHREADPOOL -DNDEBUG -DUSE_KINETO '
'-DLIBKINETO_NOROCTRACER -DUSE_FBGEMM '
'-DUSE_QNNPACK -DUSE_PYTORCH_QNNPACK '
'-DUSE_XNNPACK -DSYMBOLICATE_MOBILE_DEBUG_HANDLE '
'-O2 -fPIC -Wall -Wextra -Werror=return-type '
'-Werror=non-virtual-dtor -Werror=bool-operation '
'-Wnarrowing -Wno-missing-field-initializers '
'-Wno-type-limits -Wno-array-bounds '
'-Wno-unknown-pragmas -Wno-unused-parameter '
'-Wno-unused-function -Wno-unused-result '
'-Wno-strict-overflow -Wno-strict-aliasing '
'-Wno-stringop-overflow -Wsuggest-override '
'-Wno-psabi -Wno-error=pedantic '
'-Wno-error=old-style-cast -Wno-missing-braces '
'-fdiagnostics-color=always -faligned-new '
'-Wno-unused-but-set-variable '
'-Wno-maybe-uninitialized -fno-math-errno '
'-fno-trapping-math -Werror=format '
'-Wno-stringop-overflow, LAPACK_INFO=mkl, '
'PERF_WITH_AVX=1, PERF_WITH_AVX2=1, '
'PERF_WITH_AVX512=1, TORCH_VERSION=2.2.1, '
'USE_CUDA=ON, USE_CUDNN=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_ROCM=OFF, '
'USE_ROCM_KERNEL_ASSERT=OFF, \n',
'Python': '3.10.13 (main, Sep 11 2023, 13:44:35) [GCC 11.2.0]',
'TorchVision': '0.17.1',
'numpy_random_seed': 2147483648,
'opencompass': '0.2.4+0afb85e',
'sys.platform': 'linux'}

Reproduces the problem - code/configuration sample

Not relevant

Reproduces the problem - command or script

Not relevant

Reproduces the problem - error message

Child class allowed to be instantiated (TypeError: Can't instantiate abstract class ... error not raised) even when it doesn't override all abstract methods.

Other information

Not sure if this is intended, but I've noticed that several base classes in opencompass (icl_base_retriever, runners.base, models.base to name a few) that uses the abc.abstractmethod decorator do not first subclass from abc.ABC or use the abc.ABCMeta metaclass. This will allow a child class to be instantiated even if it does not override all abstract methods defined in the parent class.

A possible solution is to have all these base classes inherit abc.ABC and/or remove the abstractmethod decorator from concrete methods, but downstream code will have to be tested with this change.

Contributor guide

No contributing guide indexed for this repository

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.