bug: add tests for pv table + fix typo bug

This commit adds tests for PV table.
Also it helped me find the bug I was looking for. Now the search seems
good!

Also, the compile_commands.json file was only symlinked if the build
suceeded. Not very practical :)

Signed-off-by: Hans Binderup <[email protected]>

scripts/build.sh

@@ -19,6 +19,8 @@ if [ ! -d "$BUILD_DIR" ]; then
     meson setup "$BUILD_DIR" --buildtype=release
 fi
 
+ln -sf "$BUILD_DIR"/compile_commands.json .
+
 # Compile
 meson compile -C "$BUILD_DIR"
 
@@ -28,4 +30,3 @@ if $RUN; then
     "$BUILD_DIR/meltdown-chess-engine"
 fi
 
-ln -sf "$BUILD_DIR"/compile_commands.json .

scripts/debug.sh

@@ -8,7 +8,7 @@ if [ ! -d "$BUILD_DIR" ]; then
     meson setup "$BUILD_DIR"
 fi
 
+ln -sf "$BUILD_DIR"/compile_commands.json .
+
 # only run the actual executable - we don't care about "tests" here
 meson test --gdb meltdown-chess-engine -C "$BUILD_DIR"
-
-ln -sf "$BUILD_DIR"/compile_commands.json .

scripts/unit_test.sh

@@ -9,8 +9,7 @@ if [ ! -d "$BUILD_DIR" ]; then
     meson setup "$BUILD_DIR" -Dunit-tests=true -Db_coverage=true
 fi
 
-# only run the unit tests - we don't want to start the application here
-meson test unit-tests -C "$BUILD_DIR" --print-errorlogs --timeout=10
-
 ln -sf "$BUILD_DIR"/compile_commands.json .
 
+# only run the unit tests - we don't want to start the application here
+meson test unit-tests -C "$BUILD_DIR" --print-errorlogs --timeout=10

src/evaluation/pv_table.h

@@ -44,7 +44,7 @@ class PVTable {
         m_pvLength.at(ply) = ply;
     }
 
-    bool updateTable(const movement::Move& move, uint8_t ply)
+    void updateTable(const movement::Move& move, uint8_t ply)
     {
         auto& currentRow = m_pvTable.at(ply);
         currentRow.at(ply) = move;
@@ -53,7 +53,6 @@ class PVTable {
             currentRow.begin() + ply + 1);
 
         m_pvLength.at(ply) = m_pvLength.at(ply + 1);
-        return true;
     }
 
     void setIsFollowing(bool val)
@@ -63,7 +62,7 @@ class PVTable {
 
     void setIsScoring(bool val)
     {
-        m_isFollowing = val;
+        m_isScoring = val;
     }
 
     bool isFollowing() const
@@ -76,7 +75,7 @@ class PVTable {
         return m_isScoring;
     }
 
-    constexpr bool isPvMove(const movement::Move& move, uint8_t ply) const
+    bool isPvMove(const movement::Move& move, uint8_t ply) const
     {
         return m_pvTable.at(0).at(ply) == move;
     }

tests/meson.build

@@ -9,6 +9,7 @@ unit_test_sources = files(
   'src/test_move_ordering.cpp',
   'src/test_killer_moves.cpp',
   'src/test_history_moves.cpp',
+  'src/test_pv_table.cpp',
 )
 
 exe = executable(

tests/src/test_move_ordering.cpp

@@ -46,19 +46,12 @@ TEST_CASE("Movegen Hashing", "[move ordering]")
         s_evaluator.sortMoves(board.value(), allMoves, 0);
 
         const auto movesBeforeEval = allMoves.getMoves();
-        REQUIRE(movesBeforeEval[0].piece() == WhitePawn);
+        REQUIRE(movesBeforeEval[0].piece() == WhitePawn); // pawn is attacking queen
         REQUIRE(movesBeforeEval[1].piece() == WhiteRook);
         REQUIRE(movesBeforeEval[2].piece() == WhiteKing);
         REQUIRE(movesBeforeEval[3].piece() == WhiteQueen);
 
         const auto move = s_evaluator.getBestMove(board.value(), 4);
-        REQUIRE(move.piece() == WhiteQueen);
-
-        s_evaluator.sortMoves(board.value(), allMoves, 0);
-        const auto movesAfterEval = allMoves.getMoves();
-        REQUIRE(movesAfterEval[0].piece() == WhiteQueen);
-        REQUIRE(movesAfterEval[1].piece() == WhitePawn);
-        REQUIRE(movesAfterEval[2].piece() == WhiteRook);
-        REQUIRE(movesAfterEval[3].piece() == WhiteKing);
+        REQUIRE(move.piece() == WhiteQueen); // evading attack + checking king = better move!
     }
 }

tests/src/test_pv_table.cpp

@@ -0,0 +1,96 @@
+#include "src/evaluation/pv_table.h"
+#include "src/movement/move_types.h"
+
+#include <catch2/catch_test_macros.hpp>
+
+using namespace heuristic;
+using namespace movement;
+
+void prepareTableLength(PVTable& table, uint8_t ply)
+{
+    for (uint8_t p = 0; p <= ply + 1; p++) {
+        table.updateLength(p);
+    }
+}
+
+TEST_CASE("Test PVTable heuristic", "[PVTable]")
+{
+    PVTable pvTable;
+    prepareTableLength(pvTable, 1); // most tests are just one deep
+
+    SECTION("PVTable: Best move retrieval", "[PVTable]")
+    {
+        Move move = Move::create(E2, E4, WhitePawn, false);
+
+        pvTable.updateTable(move, 0);
+
+        REQUIRE(pvTable.bestMove() == move);
+    }
+
+    SECTION("PVTable: Updating and retrieving moves", "[PVTable]")
+    {
+        uint8_t size = G3 + 1;
+        prepareTableLength(pvTable, size);
+
+        // NOTE: moves have to be added in descending order as we're creating the PV nodes
+        // This makes sense as we don't add any PV nodes UNTILL we've searched as deep as we need,
+        // and THEN we will start adding moves
+        for (int8_t p = G3; p >= A1; p--) {
+            Move move = Move::create(p, p + 8, WhitePawn, false);
+            pvTable.updateTable(move, p);
+        }
+
+        auto moves = pvTable.getMoves();
+        REQUIRE(moves.size() == size);
+        for (uint8_t p = A1; p <= G3; p++) {
+            Move move = Move::create(p, p + 8, WhitePawn, false);
+            REQUIRE(moves[p] == move);
+        }
+    }
+
+    SECTION("PVTable: Checking PV move status", "[PVTable]")
+    {
+        Move move = Move::create(B1, C3, WhiteKnight, false);
+
+        pvTable.updateTable(move, 0);
+
+        REQUIRE(pvTable.isPvMove(move, 0) == true);
+    }
+
+    SECTION("PVTable: Reset functionality", "[PVTable]")
+    {
+        Move move = Move::create(E7, E5, WhitePawn, false);
+
+        pvTable.updateTable(move, 0);
+        REQUIRE(pvTable.bestMove() == move);
+
+        pvTable.reset();
+        REQUIRE(pvTable.bestMove() == Move {});
+    }
+
+    SECTION("PVTable: Following and scoring flags", "[PVTable]")
+    {
+        pvTable.setIsFollowing(true);
+        REQUIRE(pvTable.isFollowing() == true);
+
+        pvTable.setIsScoring(true);
+        REQUIRE(pvTable.isScoring() == true);
+
+        pvTable.setIsFollowing(false);
+        REQUIRE(pvTable.isFollowing() == false);
+    }
+
+    SECTION("PVTable: Updating PV scoring", "[PVTable]")
+    {
+        Move move = Move::create(H2, H4, WhitePawn, false);
+        ValidMoves validMoves;
+
+        validMoves.addMove(move);
+        pvTable.updateTable(move, 0);
+
+        REQUIRE(pvTable.isScoring() == false);
+        pvTable.updatePvScoring(validMoves, 0);
+        REQUIRE(pvTable.isScoring() == true);
+    }
+}
+