add bounds check for debug builds, run all unit tests on CI with DEBUG=1 (#194)

Author: slayoo

.github/workflows/tests+pypi.yml

@@ -23,6 +23,7 @@ jobs:
           submodules: recursive
           fetch-depth: 0  # https://github.com/pypa/setuptools_scm/issues/480
       - run: DEBUG=1 VERBOSE=1 pip install --verbose -e .[tests]
+      - run: pytest -v -s -We -p no:unraisableexception tests
 
   build:
     needs: [debug_build_ok]
@@ -106,7 +107,7 @@ jobs:
       - run: |
           cd tests
           python -c "import PyPartMC"
-          pytest --durations=10 -v -s -We -p no:unraisableexception -k 'not test_todos_annotated' .
+          pytest --durations=10 -v -s -We -p no:unraisableexception .
           cd ..
 
       - if: matrix.python-version != '3.7'

CMakeLists.txt

@@ -23,6 +23,8 @@ if(CMAKE_Fortran_COMPILER_ID STREQUAL GNU)
   add_compile_options($<$<COMPILE_LANGUAGE:Fortran>:-ffree-line-length-none>)
   # https://gcc.gnu.org/bugzilla/show_bug.cgi?id=58175
   add_compile_options($<$<COMPILE_LANGUAGE:Fortran>:-Wno-surprising>)
+
+  add_compile_options($<$<AND:$<COMPILE_LANGUAGE:Fortran>,$<CONFIG:DEBUG>>:-fcheck=bounds>)
 endif()
 
 macro(add_prefix prefix rootlist)

src/aero_state.F90

@@ -219,17 +219,6 @@ subroutine f_aero_state_bin_average_comp(ptr_c, bin_grid_ptr_c, &
 
   end subroutine
 
-  subroutine f_aero_state_copy(ptr_c,ptr_new_c) bind(C)
-    type(c_ptr) :: ptr_c, ptr_new_c
-    type(aero_state_t), pointer :: ptr_f => null()
-    type(aero_state_t), pointer :: ptr_new_f => null()
-
-    call c_f_pointer(ptr_c,ptr_f)
-    call c_f_pointer(ptr_new_c, ptr_new_f)
-
-    ptr_new_f = ptr_f
-  end subroutine
-
   subroutine f_aero_state_particle(ptr_c, ptr_particle_c, index) bind(C)
     type(c_ptr) :: ptr_c, ptr_particle_c
     integer(c_int) :: index

src/aero_state.hpp

@@ -101,11 +101,6 @@ extern "C" void f_aero_state_bin_average_comp(
     const void *aero_data_ptr
 ) noexcept;
 
-extern "C" void f_aero_state_copy(
-    const void *ptr_c,
-    const void *aero_dataptr
-) noexcept;
-
 extern "C" void f_aero_state_particle(
     const void *ptr_c,
     const void *ptr_particle_c,
@@ -327,19 +322,6 @@ struct AeroState {
         );
     }
 
-    static AeroState* __deepcopy__(
-        AeroState &self,
-        py::dict &memo
-    ) {
-        double n_part = 1.0;
-        AeroState *ptr = new AeroState(n_part, self.aero_data);
-        f_aero_state_copy(
-            self.ptr.f_arg(),
-            ptr
-        );
-        return ptr;
-    }
-
     static AeroParticle* get_particle(
         const AeroState &self,
         const int &idx

src/fake_spec_file.F90

@@ -131,7 +131,7 @@ subroutine spec_file_read_real_named_array(file, max_lines, names, vals)
 
         allocate(names(n_rows))
         allocate(vals(n_rows, n_cols))
-        allocate(vals_row(n_cols))
+        allocate(vals_row(max(1, n_cols)))
         do row = 1, n_rows
             name_size = len(names(row))
             call c_spec_file_read_real_named_array_data( &
@@ -246,15 +246,15 @@ subroutine spec_file_read_timed_real_array(file, name, times, vals)
         )
         allocate(times(times_size))
         allocate(vals(vals_size))
+        if (size(times) < 1) then
+            print*, 'must have at least one data poin in file ' // trim(file%name) // '::' // name
+            call pmc_stop(925956383)
+        end if
         call c_spec_file_read_timed_real_array_data(&
             name, len(name), &
             times(1), size(times), &
             vals(1), size(vals) &
         )
-        if (size(times) < 1) then
-            print*, 'must have at least one data poin in file ' // trim(file%name) // '::' // name
-            call pmc_stop(925956383)
-        end if
     end subroutine
 
     subroutine spec_file_open(filename, file)

src/gas_data.F90

@@ -50,14 +50,17 @@ subroutine f_gas_data_spec_by_name(ptr_c, value, name_data, name_size) &
          bind(C)
       type(gas_data_t), pointer :: ptr_f => null()
       type(c_ptr), intent(in) :: ptr_c
-      character(kind=c_char), intent(in) :: name_data
+      character(kind=c_char), dimension(*), intent(in) :: name_data
       integer(c_int), intent(in) :: name_size
-      integer(c_int) :: value
+      integer(c_int), intent(out) :: value
 
+      integer :: i
       character(len=name_size) :: name
 
       call c_f_pointer(ptr_c, ptr_f)
-      name = name_data(1:name_size) 
+      do i = 1, name_size
+          name(i:i) = name_data(i)
+      end do
       value = gas_data_spec_by_name(ptr_f, name)
 
     end subroutine

src/pypartmc.cpp

@@ -160,7 +160,6 @@ PYBIND11_MODULE(_PyPartMC, m) {
     )
         .def(py::init<const double, std::shared_ptr<AeroData>>())
         .def("__len__", AeroState::__len__)
-        .def("__deepcopy__", AeroState::__deepcopy__)
         .def_property_readonly("total_num_conc", AeroState::total_num_conc,
             "returns the total number concentration of the population")
         .def_property_readonly("total_mass_conc", AeroState::total_mass_conc,

tests/test_aero_state.py

@@ -5,7 +5,6 @@
 ####################################################################################################
 
 import gc
-from copy import deepcopy
 
 import numpy as np
 import pytest
@@ -69,17 +68,6 @@ def test_len(n_part):
         assert int(size) > n_part * 0.5
         assert int(size) < n_part * 2
 
-    @staticmethod
-    def test_copy(sut_minimal):  # pylint: disable=redefined-outer-name
-        # act
-        aero_state_copy = deepcopy(sut_minimal)
-
-        # assert
-        assert aero_state_copy is not sut_minimal
-        expected = np.sum(sut_minimal.diameters)
-        actual = np.sum(aero_state_copy.diameters)
-        assert expected == actual
-
     @staticmethod
     def test_total_num_conc(sut_minimal):  # pylint: disable=redefined-outer-name
         # act

tests/test_gas_data.py

@@ -6,6 +6,8 @@
 
 import json
 
+import pytest
+
 import PyPartMC as ppmc
 
 GAS_DATA_CTOR_ARG_MINIMAL = ("SO2",)
@@ -46,3 +48,17 @@ def test_str():
 
         # assert
         assert string == json.dumps(data, separators=(",", ":"))
+
+    @staticmethod
+    @pytest.mark.parametrize(
+        "ctor_arg", (GAS_DATA_CTOR_ARG_MINIMAL, ("SO2", "NO2"), ("A", "B", "C"))
+    )
+    def test_spec_by_name(ctor_arg):
+        # arrange
+        sut = ppmc.GasData(ctor_arg)
+
+        # act
+        indices = [sut.spec_by_name(name) for name in ctor_arg]
+
+        # assert
+        assert indices == list(range(len(ctor_arg)))