More unit tests for MATLAB replacer classes

tdms/matlabio/matlabio.cpp

@@ -62,7 +62,10 @@ void assert_is_struct_with_n_fields(const mxArray* ptr, int num, const std::stri
 mxArray* ptr_to_nd_array_in(const mxArray* ptr, int n, const std::string &name, const std::string &struct_name){
   auto element = mxGetField((mxArray *) ptr, 0, name.c_str());
 
-  if (mxGetNumberOfDimensions(element) != n) {
+  // mxGetField returns NULL if the field does not exist. Prevent SEG faults by catching and throwing
+  if (element == NULL) {
+    throw runtime_error("Field " + name + " does not exist in struct " + struct_name);
+  } else if (mxGetNumberOfDimensions(element) != n) {
     throw runtime_error("Incorrect dimension on " + struct_name + "." + name + ". Required " +
                         to_string(n) + "D");
   }

tdms/src/arrays.cpp

@@ -1,5 +1,6 @@
 #include "arrays.h"
 
+#include <spdlog/spdlog.h>
 #include <iostream>
 #include <utility>
 
@@ -48,7 +49,7 @@ void CCollection::init_xyz_vectors(const mxArray *ptr, XYZVectors &arrays, const
   for (char component : {'x', 'y', 'z'}) {
 
     auto element = ptr_to_matrix_in(ptr, prefix + component, "C");
-    is_multilayer = mxGetDimensions(element)[0] != 1;
+    is_multilayer = mxGetDimensions(element)[0] != 1; // this only matters when we check the 'z' component right? No point re-setting it each time when it's not used? TODO: check this.
     arrays.set_ptr(component, mxGetPr(element));
   }
 }
@@ -127,6 +128,8 @@ GratingStructure::GratingStructure(const mxArray *ptr, int I_tot) {
 
 GratingStructure::~GratingStructure() {
   free_cast_matlab_2D_array(matrix);
+  // prevent double free when calling ~Matrix, superclass destructor
+  matrix = nullptr;
 }
 
 FrequencyExtractVector::FrequencyExtractVector(const mxArray *ptr, double omega_an) {
@@ -143,9 +146,9 @@ FrequencyExtractVector::FrequencyExtractVector(const mxArray *ptr, double omega_
     if (n_dims != 2 || !(dims[0] == 1 || dims[1] == 1)){
       throw runtime_error("f_ex_vec should be a vector with N>0 elements");
     }
-    cerr << "f_ex_vec has ndims=" << n_dims << " N=" << dims[0] << endl;
-
+    // compute the number of elements prior to displaying
     n = std::max(dims[0], dims[1]);
+    spdlog::info("f_ex_vec has ndims={} N={}", n_dims, n);
     vector = (double *) mxGetPr(ptr);
   }
 }

tdms/tests/unit/array_tests/test_CollectionBase.cpp

@@ -0,0 +1,79 @@
+/**
+ * @file test_CollectionBase.cpp
+ * @author William Graham ([email protected])
+ * @brief Unit tests for CollectionBase class and its subclasses (CCollection, DCollection)
+ */
+#include <catch2/catch_test_macros.hpp>
+#include <spdlog/spdlog.h>
+
+#include "arrays.h"
+
+TEST_CASE("CCollection") {
+
+  // setup - an empty mxArray and the names of the field components
+  mxArray *matlab_input;
+  const int n_numeric_elements = 8;
+  const char *fieldnames[] = {"Cax", "Cay", "Caz", "Cbx", "Cby", "Cbz", "Ccx", "Ccy", "Ccz"};
+
+  // we need to provide a struct array with either 6 or 9 fields
+  // incorrect number of fields should throw an error
+  SECTION("Incorrect number of fields") {
+    matlab_input = mxCreateStructMatrix(1, 1, 3, fieldnames);
+    REQUIRE_THROWS_AS(CCollection(matlab_input), std::runtime_error);
+  }
+
+  // now create an array with the correct fieldnames
+  SECTION("Expected input (6 fields)") {
+    matlab_input = mxCreateStructMatrix(1, 1, 6, fieldnames);
+    mxArray *elements6[6];
+    for (int i = 0; i < 6; i++) {
+      elements6[i] = mxCreateNumericMatrix(1, n_numeric_elements, mxDOUBLE_CLASS, mxREAL);
+      mxSetField(matlab_input, 0, fieldnames[i], elements6[i]);
+    }
+    CCollection cc6(matlab_input);
+    REQUIRE(!cc6.is_disp_ml);
+    REQUIRE(!cc6.is_multilayer);
+  }
+  SECTION("Expected input (9 fields)") {
+    matlab_input = mxCreateStructMatrix(1, 1, 9, fieldnames);
+    mxArray *elements9[9];
+    for (int i = 0; i < 9; i++) {
+      elements9[i] = mxCreateNumericMatrix(2, n_numeric_elements, mxDOUBLE_CLASS, mxREAL);
+      mxSetField(matlab_input, 0, fieldnames[i], elements9[i]);
+    }
+    CCollection cc9(matlab_input);
+    REQUIRE(cc9.is_disp_ml);
+    REQUIRE(cc9.is_multilayer);
+  }
+
+  // tear down
+  mxDestroyArray(matlab_input);
+}
+
+TEST_CASE("DCollection") {
+
+  mxArray *matlab_input;
+  const char *fieldnames[] = {"Dax", "Day", "Daz", "Dbx", "Dby", "Dbz"};
+  const int n_numeric_elements = 8;
+
+  // we need to provide a struct array with either 6 or 9 fields
+  // incorrect number of fields should throw an error
+  SECTION("Incorrect number of fields") {
+    matlab_input = mxCreateStructMatrix(1, 1, 3, fieldnames);
+    REQUIRE_THROWS_AS(CCollection(matlab_input), std::runtime_error);
+  }
+
+  // now create an array with the correct fieldnames
+  SECTION("Expected input") {
+    matlab_input = mxCreateStructMatrix(1, 1, 6, fieldnames);
+    mxArray *elements6[6];
+    for (int i = 0; i < 6; i++) {
+      elements6[i] = mxCreateNumericMatrix(1, n_numeric_elements, mxDOUBLE_CLASS, mxREAL);
+      mxSetField(matlab_input, 0, fieldnames[i], elements6[i]);
+    }
+    REQUIRE_NOTHROW(DCollection(matlab_input));
+  }
+
+  // tear down - cleanup MATLAB memory
+  mxDestroyArray(matlab_input);
+};

tdms/tests/unit/array_tests/test_ComplexAmplitudeSample.cpp

@@ -0,0 +1,54 @@
+/**
+ * @file test_ComplexAmplitudeSample.cpp
+ * @author William Graham ([email protected])
+ * @brief Unit tests for the ComplexAmplitudeSample class and its subclasses
+ */
+#include <catch2/catch_test_macros.hpp>
+#include <spdlog/spdlog.h>
+
+#include "arrays.h"
+#include "globals.h"
+
+using namespace std;
+
+TEST_CASE("ComplexAmplitudeSample") {
+
+  mxArray *matlab_input;
+  int dimensions[2] = {1, 1};
+  const int n_fields = 2;
+  const char *fieldnames[n_fields] = {"vertices", "components"};
+
+  // Constructor should just exit if recieving an empty struct
+  SECTION("Empty input") {
+    dimensions[0] = 0;
+    matlab_input = mxCreateStructArray(2, (const mwSize *) dimensions, n_fields, fieldnames);
+    CHECK_NOTHROW(ComplexAmplitudeSample(matlab_input));
+    ComplexAmplitudeSample empty_test(matlab_input);
+    // n_vertices should return 0, since Vector has not been set so should default initialise to 0-vertex matrix
+    // similarly, vector & components should be flagged as having no elements
+    CHECK(empty_test.n_vertices() == 0);
+    CHECK(!empty_test.vertices.has_elements());
+    CHECK(!empty_test.components.has_elements());
+  }
+
+  // For successful construction, we need to build a MATLAB struct with 2 fields
+  // these are the fieldnames that are expected
+  SECTION("Expected input") {
+    matlab_input = mxCreateStructArray(2, (const mwSize *) dimensions, n_fields, fieldnames);
+    // each entry is a numeric array, setup for vertices and components
+    // copy code from Vertices and FieldComponentsVector once those tests have been added
+    const int n_elements = 8;
+    // array for "vertices" field
+    mxArray *vertices_array = mxCreateNumericMatrix(n_elements, 3, mxINT32_CLASS, mxREAL);
+    // array for "components" field
+    mxArray *components_vector = mxCreateNumericMatrix(1, n_elements, mxINT16_CLASS, mxREAL);
+    // add fields to struct that will be passed to CAS constructor function
+    mxSetField(matlab_input, 0, fieldnames[0], vertices_array);
+    mxSetField(matlab_input, 0, fieldnames[1], components_vector);
+    // create an instance using this struct
+    REQUIRE_NOTHROW(ComplexAmplitudeSample(matlab_input));
+  }
+
+  // tear down
+  mxDestroyArray(matlab_input);
+}

tdms/tests/unit/array_tests/test_DTilde.cpp

@@ -10,11 +10,11 @@
 
 using namespace std;
 
-TEST_CASE("DTilde: allocation and deallocation") {
+TEST_CASE("DTilde") {
 
   DTilde dt;
   bool no_information_stored;
-  int dims[2];
+  int dimensions[2];
   mxArray *test_input;
 
   // check no information is contained in the DTilde object
@@ -27,9 +27,9 @@ TEST_CASE("DTilde: allocation and deallocation") {
   // initialise() method should exit without assignment if we pass in a pointer to an empty array
   // create empty array
   SECTION("Empty array input") {
-    dims[0] = 0;
-    dims[1] = 1;
-    test_input = mxCreateNumericArray(2, (const mwSize *) dims, mxUINT8_CLASS, mxREAL);
+    dimensions[0] = 0;
+    dimensions[1] = 1;
+    test_input = mxCreateNumericArray(2, (const mwSize *) dimensions, mxUINT8_CLASS, mxREAL);
     // attempt assignment (2nd and 3rd args don't matter)
     dt.initialise(test_input, 0, 0);
     // should still be unassigned vectors
@@ -40,43 +40,43 @@ TEST_CASE("DTilde: allocation and deallocation") {
 
   // assignment will throw error if we attempt to provide a non-empty, non-struct array
   SECTION("Non-struct input") {
-    dims[0] = 2;
-    dims[1] = 3;
-    test_input = mxCreateNumericArray(2, (const mwSize *) dims, mxUINT8_CLASS, mxREAL);
+    dimensions[0] = 2;
+    dimensions[1] = 3;
+    test_input = mxCreateNumericArray(2, (const mwSize *) dimensions, mxUINT8_CLASS, mxREAL);
     CHECK_THROWS_AS(dt.initialise(test_input, 0, 0), runtime_error);
   }
 
   // assignment will throw error if we attempt to provide a struct array that doesn't have two fields
   SECTION("Struct with too many fields") {
-    dims[0] = 1;
-    dims[1] = 1;
+    dimensions[0] = 1;
+    dimensions[1] = 1;
     const char *too_mny_names[3] = {"field1", "field2", "field3"};
-    test_input = mxCreateStructArray(2, (const mwSize *) dims, 3, too_mny_names);
+    test_input = mxCreateStructArray(2, (const mwSize *) dimensions, 3, too_mny_names);
     CHECK_THROWS_AS(dt.initialise(test_input, 0, 0), runtime_error);
   }
   SECTION("Struct with too few fields") {
-    dims[0] = 1;
-    dims[1] = 1;
+    dimensions[0] = 1;
+    dimensions[1] = 1;
     const char *too_few_names[1] = {"field1"};
-    test_input = mxCreateStructArray(2, (const mwSize *) dims, 1, too_few_names);
+    test_input = mxCreateStructArray(2, (const mwSize *) dimensions, 1, too_few_names);
     CHECK_THROWS_AS(dt.initialise(test_input, 0, 0), runtime_error);
   }
 
   // otherwise, we need to provide a struct with two fields, Dx_tilde and Dy_tilde
   // these fields must contain (n_det_modes, n_rows, n_cols) arrays of doubles/complex
   SECTION("Correct input") {
     const char *fieldnames[] = {"Dx_tilde", "Dy_tilde"};
-    dims[0] = 1;
-    dims[1] = 1;
+    dimensions[0] = 1;
+    dimensions[1] = 1;
     const int target_n_det_modes = 5, n_rows = 6, n_cols = 4;
-    const int field_array_dims[3] = {target_n_det_modes, n_rows, n_cols};
+    const int field_array_dimensions[3] = {target_n_det_modes, n_rows, n_cols};
     // create the struct
-    test_input = mxCreateStructArray(2, (const mwSize *) dims, 2, fieldnames);
+    test_input = mxCreateStructArray(2, (const mwSize *) dimensions, 2, fieldnames);
     // create the data for the fields of our struct
     mxArray *field_array_ptrs[2];
     for (int i = 0; i < 2; i++) {
       field_array_ptrs[i] =
-              mxCreateNumericArray(3, (const mwSize *) field_array_dims, mxDOUBLE_CLASS, mxCOMPLEX);
+              mxCreateNumericArray(3, (const mwSize *) field_array_dimensions, mxDOUBLE_CLASS, mxCOMPLEX);
       mxSetField(test_input, 0, fieldnames[i], field_array_ptrs[i]);
     }
     // attempt to create a vector from this struct
@@ -87,6 +87,6 @@ TEST_CASE("DTilde: allocation and deallocation") {
     CHECK(information_stored);
   }
 
-  // cleanup MATLAB memory
+  // tear down
   mxDestroyArray(test_input);
 }

tdms/tests/unit/array_tests/test_DetectorSensitivityArrays.cpp

@@ -15,19 +15,18 @@
 using namespace std;
 using namespace tdms_math_constants;
 
-const double tol = 1e-16;
-
 TEST_CASE("DetectorSensitivityArrays") {
 
+  mxArray *matlab_input;
+  DetectorSensitivityArrays dsa;
+  const int n_rows = 8, n_cols = 4;
+
   // default constructor should set everything to nullptrs
   // destructor uses fftw destroy, which handles nullptrs itself
-  DetectorSensitivityArrays empty_dsa;
-  bool all_are_nullptrs = (empty_dsa.cm==nullptr) && (empty_dsa.plan==nullptr) && (empty_dsa.v==nullptr);
+  bool all_are_nullptrs = (dsa.cm==nullptr) && (dsa.plan==nullptr) && (dsa.v==nullptr);
   REQUIRE(all_are_nullptrs);
 
   // now let's construct a non-trival object
-  DetectorSensitivityArrays dsa;
-  const int n_rows = 8, n_cols = 4;
   dsa.initialise(n_rows, n_cols);
   // we should be able to assign complex doubles to the elements of cm now
   // to test the plan executation, we'd need an analytic DFT to hand

tdms/tests/unit/array_tests/test_DispersiveMultiLayer.cpp

@@ -12,9 +12,9 @@
 using namespace std;
 using Catch::Approx;
 
-const double tol = 1e-16;
+const double TOLERANCE = 1e-16;
 
-TEST_CASE("DispersiveMultiLayer: allocation and deallocation") {
+TEST_CASE("DispersiveMultiLayer") {
 
   mxArray *matlab_input;
   // Constructor should throw runtime_error at not recieving struct
@@ -29,8 +29,8 @@ TEST_CASE("DispersiveMultiLayer: allocation and deallocation") {
   // create a MATLAB pointer to an empty struct
   SECTION("Empty struct input") {
     const char* empty_fields[] = {};
-    const int empty_dims[2] = {1,1};
-    matlab_input = mxCreateStructArray(2, (const mwSize *) empty_dims, 0, empty_fields);
+    const int empty_dimensions[2] = {1,1};
+    matlab_input = mxCreateStructArray(2, (const mwSize *) empty_dimensions, 0, empty_fields);
     CHECK_THROWS_AS(DispersiveMultiLayer(matlab_input), runtime_error);
   }
 
@@ -42,8 +42,8 @@ TEST_CASE("DispersiveMultiLayer: allocation and deallocation") {
                                         "kappa_z", "sigma_x", "sigma_y", "sigma_z"};
     const int n_field_elements = 5;
     // build our struct
-    const int dims[2] = {1, 1};
-    matlab_input = mxCreateStructArray(2, (const mwSize *) dims, n_fields, fieldnames);
+    const int dimensions[2] = {1, 1};
+    matlab_input = mxCreateStructArray(2, (const mwSize *) dimensions, n_fields, fieldnames);
     // build "data" for each of the fields, which is going to be the same array filled with consecutive integers
     const int array_size[2] = {1, n_field_elements};
     mxArray *field_array_ptrs[n_fields];
@@ -59,18 +59,18 @@ TEST_CASE("DispersiveMultiLayer: allocation and deallocation") {
     DispersiveMultiLayer dml(matlab_input);
     // now check that the data has been correctly assigned
     for (int i = 0; i < 5; i++) {
-      CHECK(dml.alpha[i] == Approx(i).epsilon(tol));
-      CHECK(dml.beta[i] == Approx(i).epsilon(tol));
-      CHECK(dml.gamma[i] == Approx(i).epsilon(tol));
-      CHECK(dml.kappa.x[i] == Approx(i).epsilon(tol));
-      CHECK(dml.kappa.y[i] == Approx(i).epsilon(tol));
-      CHECK(dml.kappa.z[i] == Approx(i).epsilon(tol));
-      CHECK(dml.sigma.x[i] == Approx(i).epsilon(tol));
-      CHECK(dml.sigma.y[i] == Approx(i).epsilon(tol));
-      CHECK(dml.sigma.z[i] == Approx(i).epsilon(tol));
+      CHECK(dml.alpha[i] == Approx(i).epsilon(TOLERANCE));
+      CHECK(dml.beta[i] == Approx(i).epsilon(TOLERANCE));
+      CHECK(dml.gamma[i] == Approx(i).epsilon(TOLERANCE));
+      CHECK(dml.kappa.x[i] == Approx(i).epsilon(TOLERANCE));
+      CHECK(dml.kappa.y[i] == Approx(i).epsilon(TOLERANCE));
+      CHECK(dml.kappa.z[i] == Approx(i).epsilon(TOLERANCE));
+      CHECK(dml.sigma.x[i] == Approx(i).epsilon(TOLERANCE));
+      CHECK(dml.sigma.y[i] == Approx(i).epsilon(TOLERANCE));
+      CHECK(dml.sigma.z[i] == Approx(i).epsilon(TOLERANCE));
     }
   }
 
-  // cleanup MATLAB array
+  // tear down
   mxDestroyArray(matlab_input);
 }