Add unit testing framework and tests for existing code (and assorted …

…fixes and breaking API changes) (#23)

* wip: add catch2 and 1 == 1 test case

* chore: specify minimum meson version

* chore: don't commit catch2 source

* wip: add matrix multiplication test

* fix: specify visibility for all symbols

* fix: add include guard

* feat: add cpp visibility macros to logger

* fix: actually define the visbility macro when building shared lib

* fix: add missing include directives

i sure hope nothing else is missing and nothing else was messed up by
the merge

* wip: refactor meson build

* refactor: move logger.hpp to include directory to enable building

* wip: refactor meson build but on this branch

* fix: remove non-existent directory from includes

* fix: fix some memory allocation and constructor whoopsies in matrix and vector classes

* wip: test seems to pass after refactor

* feat!: add getter and test for matrix

* feat!: make getter and setter for matrix throw when oob

* refactor: use bdd-style macros for testing matrix getters and setters

* tests: add tests for activation functions

* test: test jml::Vector

* feat: enable use of any type in iterator overload for add_testing_data

* feat: add testing data getter

* fix: move model implementation around so it builds and links

* feat: add copy constructor to Vector

* tests: test Model

* docs: add information about testing

* fix: add missing includes for tuple

* ci: upload testlog as artifact

* ci: change artifact name

* docs: fix md link in readme

.github/workflows/build-test.yml

@@ -35,6 +35,12 @@ jobs:
       - name: Test with Meson
         run: meson test -C build
 
+      - name: Upload test log
+        uses: actions/upload-artifact@v3
+        with:
+          name: testlog-linux
+          path: build/meson-logs/testlog.txt
+
   macos:
     name: Build and Test on macOS
     runs-on: macos-latest
@@ -62,6 +68,12 @@ jobs:
       - name: Test with Meson
         run: meson test -C build
 
+      - name: Upload test log
+        uses: actions/upload-artifact@v3
+        with:
+          name: testlog-macos
+          path: build/meson-logs/testlog.txt
+
   windows-vs:
     name: Build and Test on Windows (Visual Studio)
     runs-on: windows-latest
@@ -88,3 +100,9 @@ jobs:
 
       - name: Test with Meson
         run: meson test -C build
+
+      - name: Upload test log
+        uses: actions/upload-artifact@v3
+        with:
+          name: testlog-windows
+          path: build/meson-logs/testlog.txt

.gitignore

@@ -17,3 +17,7 @@ test.cpp
 # --------------- #
 .vscode/
 # ----------------#
+
+# Don't commit Catch2 source or wrapdb cache
+subprojects/Catch2-3.4.0
+subprojects/packagecache

CONTRIBUTING.md

@@ -101,3 +101,14 @@ view the API docs for discussion.
 
 The entire project's functionality is in the C++ namespace `jml`, and we prefer
 `snake_case` over `camelCase.`
+
+## Testing
+Though tests are tedious to write, they ensure that our code (somewhat)
+functions consistently and as expected. As such, we encourage you to write
+tests for any code you add. JML uses the popular
+[Catch2](https://github.com/catchorg/Catch2) framework for testing. Their docs,
+located under `docs/` in their repository, should be all you need to write
+tests (they also have a Discord server for support inquiries). How you write,
+express, or format the tests does not matter, as long as they thoroughly test
+the code (and bear some semblance of readability). Make sure to also add your
+test source to the meson build file.

README.md

@@ -21,10 +21,11 @@ meson install -C build  # again, replace `build`
 ```
 
 ## Test
-> [!WARNING]
-> Add relevant information once testing framework is setup
+This project uses [Catch2](https://github.com/catchorg/Catch2) as its testing framework.
 
 Run `meson test` to test the project.
 ```bash
 meson test -C build     # replace `build`
 ```
+
+Unfortunately, meson doesn't provide good output when a test fails. Running the test executable manually is often more helpful. Look under `build/tests/` to find the right executable. The executable for testing the core library is `core_tests`.

core/include/jml/math/matrix.hpp

@@ -25,15 +25,16 @@ class JML_API Matrix {
     double *entries;
 
     // This gets the array position for a particular row and column combination.
-    int JML_LOCAL get_position(int i, int j);
+    int JML_LOCAL get_position(int i, int j) const;
 
   public:
     Matrix(int m, int n);
     ~Matrix();
-    int get_n_rows();
-    int get_n_cols();
+    int get_n_rows() const;
+    int get_n_cols() const;
     void set_entry(int i, int j, double value);
-    std::unique_ptr<Vector> multiply(std::unique_ptr<Vector>);
+    double get_entry(const int i, const int j) const;
+    std::unique_ptr<Vector> multiply(const Vector& in) const;
 };
 
 } // namespace jml

core/include/jml/math/vector.hpp

@@ -23,6 +23,7 @@ class JML_API Vector {
 public:
 
      Vector(int length);
+     Vector(const Vector& other);
     ~Vector();
 
     // This applies a given function to every component in the vector.
@@ -31,9 +32,9 @@ class JML_API Vector {
     // This adds every entry in v to this vector.
     void add(Vector& v);
 
-    int get_size();
+    int get_size() const;
 
-    double get_entry(int pos);
+    double get_entry(int pos) const;
     void set_entry(int pos, double val);
     void add_entry(int pos, double val); // Adds val to the current value in pos.
 

core/include/jml/model.hpp

@@ -9,6 +9,7 @@
 #pragma once
 
 #include <memory>
+#include <tuple>
 #include <vector>
 
 #include <jml/math/matrix.hpp>
@@ -60,13 +61,13 @@ class JML_API Model {
     std::unique_ptr<Vector> apply(Vector& in);
     // Adds testing data using iterators, for the start and end locations of
     // the wanted input and output data lists.
-    void add_testing_data (
-        std::vector<Vector>::iterator inb, std::vector<Vector>::iterator ine,
-        std::vector<Vector>::iterator otb, std::vector<Vector>::iterator ote
-    );
+    template<typename Iter>
+    void add_testing_data (Iter inb, Iter ine, Iter otb, Iter ote);
     void add_testing_data (std::vector<Vector> ins, std::vector<Vector> outs);
     void add_testing_datum(Vector in, Vector out);
     void clear_testing_data();
+    // Gets testing data. First vector is inputs, second vector is outputs.
+    const std::tuple<std::vector<Vector>, std::vector<Vector>> get_testing_data() const;
 
 private:
     std::vector<Model_Layer> layers; // Array of layers
@@ -76,3 +77,5 @@ class JML_API Model {
 };
 
 }
+
+#include "model.tpp"

core/include/jml/model.tpp

@@ -0,0 +1,13 @@
+#pragma once
+
+#include "model.hpp"
+
+namespace jml {
+
+template<typename Iter>
+void Model::add_testing_data (Iter inb, Iter ine, Iter otb, Iter ote) {
+    testing_data_inputs .insert(std::end(testing_data_inputs),  inb, ine);
+    testing_data_outputs.insert(std::end(testing_data_outputs), otb, ote);
+}
+
+}

core/meson.build

@@ -17,3 +17,5 @@ jmlcore = library('jmlcore',
                   install: true,
                   cpp_args: core_dll_defs,
                   gnu_symbol_visibility: 'inlineshidden')
+
+jmlcore_dep = declare_dependency(link_with: jmlcore, include_directories: core_inc)

core/src/math/matrix.cpp

@@ -2,45 +2,62 @@
 #include <jml/internal/logger.hpp>
 
 #include <iostream>
+#include <stdexcept>
 
 namespace jml {
 
-Matrix::Matrix(int m, int n) { this->entries = new double(m * n); }
+Matrix::Matrix(int m, int n): m(m), n(n) { this->entries = new double[m * n]; }
 
-Matrix::~Matrix() { delete this->entries; }
+Matrix::~Matrix() { delete[] this->entries; }
 
-int Matrix::get_position(int i, int j) { return i * this->n + j; }
+int Matrix::get_position(int i, int j) const { return i * this->n + j; }
 
-int Matrix::get_n_rows() { return m; }
+int Matrix::get_n_rows() const { return m; }
 
-int Matrix::get_n_cols() { return n; }
+int Matrix::get_n_cols() const { return n; }
 
 void Matrix::set_entry(int i, int j, double value) {
 
     if (i < 0 || i >= this->m) {
         LOGGER->log(Log() << ERR << "Row number " << i << " out of bounds.\n");
+        throw std::out_of_range("row out of bounds");
     }
     if (j < 0 || j >= this->n) {
         LOGGER->log(Log() << ERR << "Column number " << i
                           << " out of bounds.\n");
+        throw std::out_of_range("column out of bounds");
     }
 
     this->entries[get_position(i, j)] = value;
 }
 
-std::unique_ptr<Vector> Matrix::multiply(std::unique_ptr<Vector> in) {
+double Matrix::get_entry(const int i, const int j) const {
+    if (i < 0 || i >= this->m) {
+        LOGGER->log(Log() << ERR << "Row number " << i << " out of bounds.\n");
+        throw std::out_of_range("row out of bounds");
+    }
+    if (j < 0 || j >= this->n) {
+        LOGGER->log(Log() << ERR << "Column number " << i
+                          << " out of bounds.\n");
+        throw std::out_of_range("column out of bounds");
+    }
+
+    return this->entries[get_position(i, j)];
+}
+
+std::unique_ptr<Vector> Matrix::multiply(const Vector& in) const {
 
-    if (in->get_size() != this->n) {
+    if (in.get_size() != this->n) {
         LOGGER->log(Log(WARN) << "Tried to multiply a vector of size "
-                              << in->get_size() << " with a matrix containing "
+                              << in.get_size() << " with a matrix containing "
                               << this->n << " columns.\n");
     }
 
     std::unique_ptr<Vector> ret = std::make_unique<Vector>(this->m);
 
     for (int i = 0; i < this->m; ++i) {
         for (int j = 0; j < this->n; ++j)
-            ret->add_entry(i, in->get_entry(j) *
+            ret->add_entry(i, in.get_entry(j) *
                                   this->entries[get_position(i, j)]);
     }
 

core/src/math/vector.cpp

@@ -10,13 +10,19 @@ Logger *LOGGER = Logger::Instance();
 
 Vector::Vector(int length) {
 
-    this->entries = new double(length);
+    this->entries = new double[length];
     memset(this->entries, 0, length * sizeof(double));
 
     this->length = length;
 }
 
-Vector::~Vector() { delete this->entries; }
+Vector::Vector(const Vector& other) {
+    this->length = other.length;
+    this->entries = new double[this->length];
+    memcpy(this->entries, other.entries, this->length * sizeof(double));
+}
+
+Vector::~Vector() { delete[] this->entries; }
 
 void Vector::apply(ActivationFunction *fn) {
     for (int i = 0; i < this->length; ++i) {
@@ -35,9 +41,9 @@ void Vector::add(Vector &v) {
     }
 }
 
-int Vector::get_size() { return this->length; }
+int Vector::get_size() const { return this->length; }
 
-double Vector::get_entry(int pos) {
+double Vector::get_entry(int pos) const {
     if (pos < 0 || pos >= this->length) {
         LOGGER->log(Log(ERR) << "Position " << pos << " out of bounds.\n");
     }

core/src/model.cpp

@@ -1,14 +1,17 @@
 #include <jml/model.hpp>
+#include <tuple>
 
 namespace jml {
 
+Model::Model() {}
+
 std::unique_ptr<Vector> Model::apply(Vector& in) {
 
     std::unique_ptr<Vector> inp  = std::make_unique<Vector>(in);
     std::unique_ptr<Vector> outp;
 
     for (Model_Layer ml : this->layers) {
-        outp = ml.matrix.multiply(std::move(inp));
+        outp = ml.matrix.multiply(*inp);
         inp = std::move(outp);
         inp->add(ml.bias_vector);
         inp->apply(ml.act.get());
@@ -18,14 +21,6 @@ std::unique_ptr<Vector> Model::apply(Vector& in) {
 
 }
 
-void Model::add_testing_data (
-    std::vector<Vector>::iterator inb, std::vector<Vector>::iterator ine,
-    std::vector<Vector>::iterator otb, std::vector<Vector>::iterator ote
-) {
-    testing_data_inputs .insert(std::end(testing_data_inputs),  inb, ine);
-    testing_data_outputs.insert(std::end(testing_data_outputs), otb, ote);
-}
-
 void Model::add_testing_data(std::vector<Vector> ins, std::vector<Vector> outs) {
     return add_testing_data(std::begin(ins), std::end(ins), std::begin(outs), std::end(outs));
 }
@@ -40,4 +35,8 @@ void Model::clear_testing_data() {
     testing_data_outputs.clear();
 }
 
+const std::tuple<std::vector<Vector>, std::vector<Vector>> Model::get_testing_data() const {
+    return {this->testing_data_inputs, this->testing_data_outputs};
+}
+
 }

meson.build

@@ -1,4 +1,10 @@
 project('jml', 'cpp',
-        default_options: ['cpp_std=c++14'])
+        default_options: ['cpp_std=c++14'],
+        meson_version: '>=1.2.0')
+
+catch2_proj = subproject('catch2', default_options: {'tests': false})
+catch2_dep = catch2_proj.get_variable('catch2_with_main_dep')
 
 subdir('core')
+
+subdir('tests')

subprojects/catch2.wrap

@@ -0,0 +1,11 @@
+[wrap-file]
+directory = Catch2-3.4.0
+source_url = https://github.com/catchorg/Catch2/archive/v3.4.0.tar.gz
+source_filename = Catch2-3.4.0.tar.gz
+source_hash = 122928b814b75717316c71af69bd2b43387643ba076a6ec16e7882bfb2dfacbb
+source_fallback_url = https://github.com/mesonbuild/wrapdb/releases/download/catch2_3.4.0-1/Catch2-3.4.0.tar.gz
+wrapdb_version = 3.4.0-1
+
+[provide]
+catch2 = catch2_dep
+catch2-with-main = catch2_with_main_dep

tests/core/math/test_activation_functions.cpp

@@ -0,0 +1,59 @@
+#include "catch2/catch_test_macros.hpp"
+#include "catch2/matchers/catch_matchers.hpp"
+#include "catch2/matchers/catch_matchers_floating_point.hpp"
+#include "jml/math/activation_functions.hpp"
+
+TEST_CASE("FastSigmoid function", "[activation_function]") {
+    jml::FastSigmoid act;
+
+    // FIXME: I don't know what mathematically significant values to check so here's some random ones
+    // FIXME: FastSigmoid impl appears to have forgotten the constant offset
+    REQUIRE_THAT(act.f(-1), Catch::Matchers::WithinRel(-0.25, 0.00001));
+    REQUIRE_THAT(act.f(0), Catch::Matchers::WithinRel(0, 0.00001));
+    REQUIRE_THAT(act.f(1), Catch::Matchers::WithinRel(0.25, 0.00001));
+}
+
+TEST_CASE("Sigmoid activation function", "[activation_function]") {
+    jml::Sigmoid act;
+
+    REQUIRE_THAT(act.f(-1), Catch::Matchers::WithinRel(0.2689414, 0.00001));
+    REQUIRE_THAT(act.f(0), Catch::Matchers::WithinRel(0.5, 0.00001));
+    REQUIRE_THAT(act.f(1), Catch::Matchers::WithinRel(0.7310586, 0.00001));
+
+    REQUIRE_THAT(act.df(-1), Catch::Matchers::WithinRel(0.1966119, 0.00001));
+    REQUIRE_THAT(act.df(0), Catch::Matchers::WithinRel(0.25, 0.00001));
+    REQUIRE_THAT(act.df(1), Catch::Matchers::WithinRel(0.1966119, 0.00001));
+}
+
+TEST_CASE("ReLU activation function", "[activation_function]") {
+    jml::ReLU act;
+
+    REQUIRE(act.f(-1) == 0);
+    REQUIRE(act.f(0) == 0);
+    REQUIRE(act.f(1) == 1);
+
+    REQUIRE(act.df(-1) == 0);
+    // REQUIRE(act.df(0) == 0); FIXME: is this UB?
+    REQUIRE(act.df(1) == 1);
+}
+
+TEST_CASE("LeakyReLU activation function", "[activation_function]") {
+    jml::LeakyReLU act;
+
+    REQUIRE_THAT(act.f(-1), Catch::Matchers::WithinRel(-0.01, 0.00001));
+    REQUIRE(act.f(0) == 0);
+    REQUIRE(act.f(1) == 1);
+
+    REQUIRE_THAT(act.df(-1), Catch::Matchers::WithinRel(0.01, 0.00001));
+    // REQUIRE(act.df(0) == 0); FIXME: is this UB?
+    REQUIRE(act.df(1) == 1);
+}
+
+TEST_CASE("LeakyReLU activation function with custom leak", "[activation_function]") {
+    jml::LeakyReLU act(0.0001);
+
+    // TODO: is it necessary to check function values?
+
+    REQUIRE_THAT(act.df(-1), Catch::Matchers::WithinRel(0.0001, 0.00001));
+    REQUIRE(act.df(1) == 1);
+}