Skip to content

Commit b3e0cc6

Browse files
Extract ponder move and check it against PV
1 parent 517a316 commit b3e0cc6

14 files changed

Lines changed: 772 additions & 690 deletions

File tree

app/src/cli/cli.cpp

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -673,6 +673,8 @@ void parseAffinity(const std::vector<std::string>& params, ArgumentData& argumen
673673

674674
void parseMatePvs(ArgumentData& argument_data) { argument_data.tournament_config.check_mate_pvs = true; }
675675

676+
void parsePonderPvShort(ArgumentData& argument_data) { argument_data.tournament_config.warn_ponder_pv_short = true; }
677+
676678
void parseLatency(ArgumentData& argument_data) { argument_data.tournament_config.show_latency = true; }
677679

678680
void parseDebug(ArgumentData&) {
@@ -764,6 +766,7 @@ void OptionsParser::registerOptions() {
764766
addOption<ParamStyle::KeyValue>("quick", parseQuick);
765767
addOption("use-affinity", parseAffinity);
766768
addOption<ParamStyle::None>("check-mate-pvs", parseMatePvs);
769+
addOption<ParamStyle::None>("warn-ponder-pv-short", parsePonderPvShort);
767770
addOption<ParamStyle::None>("show-latency", parseLatency);
768771
addOption<ParamStyle::None>("debug", parseDebug);
769772
addOption<ParamStyle::None>("testEnv", parseTestEnv);

app/src/cli/man.hpp

Lines changed: 661 additions & 653 deletions
Large diffs are not rendered by default.

app/src/engine/compliance.hpp

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -67,7 +67,7 @@ inline bool compliant(int argc, char const* argv[]) {
6767
return false;
6868
}
6969

70-
std::cout << "\r" << "\033[1;32m Passed\033[0m Step " << step << ": " << description << std::endl;
70+
std::cout << "\r\033[1;32m Passed\033[0m Step " << step << ": " << description << std::endl;
7171

7272
return true;
7373
};
@@ -137,7 +137,7 @@ inline bool compliant(int argc, char const* argv[]) {
137137
{"Read bestmove after go wtime 100 btime 100",
138138
[&uci_engine] {
139139
return uci_engine.readEngine("bestmove").code == process::Status::OK &&
140-
uci_engine.bestmove() != std::nullopt;
140+
uci_engine.bestmove().first != std::nullopt;
141141
}},
142142
{"Verify info line format is valid after go wtime 100 btime 100",
143143
[&uci_engine] { return isValidInfoLine(uci_engine.lastInfoLine()); }},
@@ -147,7 +147,7 @@ inline bool compliant(int argc, char const* argv[]) {
147147
{"Read bestmove after position startpos moves e2e4 e7e5",
148148
[&uci_engine] {
149149
return uci_engine.readEngine("bestmove").code == process::Status::OK &&
150-
uci_engine.bestmove() != std::nullopt;
150+
uci_engine.bestmove().first != std::nullopt;
151151
}},
152152
{"Verify info line format is valid after position startpos moves e2e4 e7e5",
153153
[&uci_engine] { return isValidInfoLine(uci_engine.lastInfoLine()); }}};

app/src/engine/uci_engine.cpp

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -439,7 +439,7 @@ bool UciEngine::writeEngine(const std::string& input) {
439439
return process_.writeInput(input + "\n").code == process::Status::OK;
440440
}
441441

442-
std::optional<std::string> UciEngine::bestmove(bool warn_on_error) const {
442+
std::pair<std::optional<std::string>, std::optional<std::string>> UciEngine::bestmove(bool warn_on_error) const {
443443
const auto warn = [&](std::string_view reason, std::string_view last_line = {}) {
444444
if (!warn_on_error) {
445445
return;
@@ -457,24 +457,27 @@ std::optional<std::string> UciEngine::bestmove(bool warn_on_error) const {
457457

458458
if (lines.empty()) {
459459
warn("No output");
460-
return std::nullopt;
460+
return {std::nullopt, std::nullopt};
461461
}
462462

463463
const auto& last_line = lines.back()->line;
464464

465465
if (last_line.rfind("bestmove", 0) != 0) {
466466
warn("Line does not start with 'bestmove'", last_line);
467-
return std::nullopt;
467+
return {std::nullopt, std::nullopt};
468468
}
469469

470-
const auto bm = str_utils::findElement<std::string>(str_utils::splitString(last_line, ' '), "bestmove");
470+
const auto last_info = str_utils::splitString(last_line, ' ');
471+
const auto bm = str_utils::findElement<std::string>(last_info, "bestmove");
471472

472473
if (!bm.has_value()) {
473474
warn(bm.error(), last_line);
474-
return std::nullopt;
475+
return {std::nullopt, std::nullopt};
475476
}
476477

477-
return *bm;
478+
const auto pm = str_utils::findElement<std::string>(last_info, "ponder");
479+
480+
return {bm.value(), pm.has_value() ? std::optional<std::string>{pm.value()} : std::nullopt};
478481
}
479482

480483
std::chrono::milliseconds UciEngine::lastTime() const {

app/src/engine/uci_engine.hpp

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
#include <chrono>
44
#include <optional>
55
#include <string>
6+
#include <utility>
67
#include <vector>
78

89
#include <chess.hpp>
@@ -107,8 +108,8 @@ class UciEngine {
107108
"Warning; Failed to set CPU affinity for the engine process to {}. Please restart.", cpu_str);
108109
}
109110

110-
// Get the bestmove from the last output.
111-
[[nodiscard]] std::optional<std::string> bestmove(bool warn_on_error = true) const;
111+
// Get the bestmove (and possibly ponder move) from the last output.
112+
[[nodiscard]] std::pair<std::optional<std::string>, std::optional<std::string>> bestmove(bool warn_on_error = true) const;
112113

113114
// Get the last info line from the last output.
114115
[[nodiscard]] std::string lastInfoLine() const;

app/src/matchmaking/match/match.cpp

Lines changed: 32 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -152,11 +152,12 @@ std::string Match::convertScoreToString(engine::Score score) {
152152
return "ERR";
153153
}
154154

155-
void Match::addMoveData(const Player& player, const std::string& move, int64_t measured_time_ms, int64_t latency,
155+
void Match::addMoveData(const Player& player, const std::string& move, const std::string& ponder, int64_t measured_time_ms, int64_t latency,
156156
int64_t timeleft, bool legal) {
157157
MoveData move_data;
158158

159159
move_data.move = move;
160+
move_data.ponder = ponder;
160161
move_data.score_string = "0.00";
161162
move_data.elapsed_millis = measured_time_ms;
162163
move_data.legal = legal;
@@ -211,7 +212,8 @@ void Match::addMoveData(const Player& player, const std::string& move, int64_t m
211212
}
212213
}
213214

214-
verifyPvLines(player, legal ? move : "");
215+
// only warn on PV/bestmove mismatch if the move is legal
216+
verifyPvLines(player, legal ? move : "", ponder);
215217

216218
data_.moves.push_back(move_data);
217219
}
@@ -400,15 +402,17 @@ bool Match::playMove(Player& us, Player& them) {
400402
LOG_INFO_THREAD("Engine {} latency: {}ms (elapsed: {}, reported: {})", name, latency, elapsed_ms, last_time);
401403
}
402404

403-
const auto best_move = us.engine.bestmove(status.code == engine::process::Status::OK);
405+
const auto bestmove = us.engine.bestmove(status.code == engine::process::Status::OK);
406+
const auto best_move = bestmove.first;
404407
const auto move = best_move && uci::isUciMove(*best_move) ? uci::uciToMove(board_, *best_move) : Move::NO_MOVE;
405408
const auto legal = isLegal(move);
406409

407410
const auto timeout = us.hasTimeControl() ? !us.getTimeControl().updateTime(elapsed_ms) : false;
408411
const auto timeleft = us.hasTimeControl() ? us.getTimeControl().getTimeLeft() : 0;
409412

410413
if (best_move) {
411-
addMoveData(us, *best_move, elapsed_ms, latency, timeleft, legal);
414+
const auto ponder = bestmove.second;
415+
addMoveData(us, *best_move, ponder ? *ponder : "", elapsed_ms, latency, timeleft, legal);
412416
}
413417

414418
// there are two reasons why best_move could be empty
@@ -577,7 +581,7 @@ void Match::setEngineIllegalMoveStatus(Player& loser, Player& winner, const std:
577581
Logger::print<Logger::Level::WARN>("Warning; Illegal move {} played by {}", best_move.value_or("<none>"), name);
578582
}
579583

580-
void Match::verifyPvLines(const Player& us, const std::string& best_move) {
584+
void Match::verifyPvLines(const Player& us, const std::string& best_move, const std::string& ponder_move) {
581585
const static auto verifyPv = [](Board board, const std::string& startpos,
582586
const std::vector<std::string_view>& uci_moves, const std::string& info,
583587
std::string_view name) {
@@ -685,7 +689,7 @@ void Match::verifyPvLines(const Player& us, const std::string& best_move) {
685689
verifyPv(board_, start_position_, data_.getMoves(), *info, us.engine.getConfig().name);
686690
}
687691

688-
// finally check if the final PV matches bestmove
692+
// finally check if the final PV matches bestmove (and possibly ponder move)
689693
if (best_move.empty()) {
690694
return;
691695
}
@@ -706,17 +710,36 @@ void Match::verifyPvLines(const Player& us, const std::string& best_move) {
706710
return;
707711
}
708712

709-
if (best_move != (*pv)[0]) {
710-
auto warning = "Warning; Bestmove does not match beginning of last PV - move {} from {}";
713+
for (unsigned int i = 0; i < 2; i++) {
714+
if (i && (ponder_move.empty() || (!config::TournamentConfig->warn_ponder_pv_short && pv->size() < 2))) {
715+
break;
716+
}
717+
718+
const auto &value = i ? ponder_move : best_move;
719+
if (i < pv->size() && value == (*pv)[i]) {
720+
continue;
721+
}
722+
723+
std::string warning;
724+
if (i && pv->size() == 1) {
725+
warning = "Warning; PV has length 1 despite ponder move output - move {} from {}";
726+
} else {
727+
warning =
728+
"Warning; "s + (i ? "Ponder" : "Best") + "move does not match beginning of last PV - move {} from {}";
729+
}
730+
711731
auto start_pos = start_position_ == "startpos" ? "startpos" : ("fen " + start_position_);
712-
auto out = fmt::format(fmt::runtime(warning), best_move, us.engine.getConfig().name);
732+
auto out = fmt::format(fmt::runtime(warning), value, us.engine.getConfig().name);
713733
auto uci_info = fmt::format("Info; {}", info);
714734
auto position = fmt::format("Position; {}", start_pos);
715735
auto ucimoves = fmt::format("Moves; {}", str_utils::join(data_.getMoves(), " "));
716736

717737
auto separator = config::TournamentConfig->test_env ? " :: " : "\n";
718738

719739
Logger::print<Logger::Level::WARN>("{1}{0}{2}{0}{3}{0}{4}", separator, out, uci_info, position, ucimoves);
740+
741+
// Do not emit ponder move warning if already the bestmove does not match PV
742+
break;
720743
}
721744
}
722745

app/src/matchmaking/match/match.hpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -170,10 +170,10 @@ class Match {
170170
void setEngineTimeoutStatus(Player& loser, Player& winner, const std::optional<std::string>& best_move);
171171
void setEngineIllegalMoveStatus(Player& loser, Player& winner, const std::optional<std::string>& best_move);
172172

173-
void verifyPvLines(const Player& us, const std::string& best_move);
173+
void verifyPvLines(const Player& us, const std::string& best_move, const std::string& ponder_move);
174174

175175
// append the move data to the match data
176-
void addMoveData(const Player& player, const std::string& move, int64_t measured_time_ms, int64_t latency,
176+
void addMoveData(const Player& player, const std::string& move, const std::string& ponder, int64_t measured_time_ms, int64_t latency,
177177
int64_t timeleft, bool legal);
178178

179179
// returns false if the next move could not be played

app/src/types/match_data.hpp

Lines changed: 18 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -17,9 +17,10 @@ namespace fastchess {
1717
struct MoveData {
1818
MoveData() = default;
1919
explicit MoveData(std::string _move) : move(std::move(_move)) {}
20-
MoveData(std::string _move, std::string _score_string, int64_t _elapsed_millis, int _depth, int _seldepth,
20+
MoveData(std::string _move, std::string _ponder, std::string _score_string, int64_t _elapsed_millis, int _depth, int _seldepth,
2121
int _score, int _nodes, bool _legal = true, bool _book = false, std::string _pv = "")
2222
: move(std::move(_move)),
23+
ponder(std::move(_ponder)),
2324
score_string(std::move(_score_string)),
2425
pv(_pv),
2526
nodes(_nodes),
@@ -29,9 +30,23 @@ struct MoveData {
2930
score(_score),
3031
legal(_legal),
3132
book(_book) {}
32-
33-
std::vector<std::string> additional_lines;
33+
MoveData(std::string _move, std::string _score_string, int64_t _elapsed_millis, int _depth, int _seldepth,
34+
int _score, int _nodes, bool _legal = true, bool _book = false, std::string _pv = "")
35+
: MoveData(std::move(_move),
36+
"",
37+
std::move(_score_string),
38+
_elapsed_millis,
39+
_depth,
40+
_seldepth,
41+
_score,
42+
_nodes,
43+
_legal,
44+
_book,
45+
std::move(_pv)) {}
46+
47+
std::vector<std::string> additional_lines;
3448
std::string move;
49+
std::string ponder;
3550
std::string score_string;
3651
std::string pv = "";
3752

app/src/types/tournament.hpp

Lines changed: 10 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -65,13 +65,14 @@ struct Tournament {
6565
uint32_t wait = 0;
6666
bool force_concurrency = false;
6767

68-
bool noswap = false;
69-
bool reverse = false;
70-
bool recover = false;
71-
bool affinity = false;
72-
bool check_mate_pvs = false;
73-
bool show_latency = false;
74-
bool test_env = false;
68+
bool noswap = false;
69+
bool reverse = false;
70+
bool recover = false;
71+
bool affinity = false;
72+
bool check_mate_pvs = false;
73+
bool warn_ponder_pv_short = false;
74+
bool show_latency = false;
75+
bool test_env = false;
7576

7677
std::chrono::milliseconds startup_time = std::chrono::seconds(10);
7778
std::chrono::milliseconds ucinewgame_time = std::chrono::seconds(60);
@@ -84,6 +85,7 @@ struct Tournament {
8485
NLOHMANN_DEFINE_TYPE_NON_INTRUSIVE(Tournament, resign, draw, maxmoves, tb_adjudication, opening, pgn, epd, sprt,
8586
config_name, output, seed, variant, type, gauntlet_seeds, ratinginterval,
8687
scoreinterval, wait, autosaveinterval, games, rounds, concurrency, force_concurrency,
87-
recover, noswap, reverse, report_penta, affinity, check_mate_pvs, show_latency, log)
88+
recover, noswap, reverse, report_penta, affinity, check_mate_pvs, warn_ponder_pv_short,
89+
show_latency, log)
8890

8991
} // namespace fastchess::config

app/tests/cli_test.cpp

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -557,6 +557,7 @@ TEST_SUITE("Option Parsing Tests") {
557557
const auto config = parser.getTournamentConfig();
558558
CHECK(config.rounds == 5);
559559
CHECK(config.check_mate_pvs);
560+
CHECK(config.warn_ponder_pv_short);
560561
CHECK(config.show_latency);
561562
CHECK(config.log.realtime);
562563
CHECK(config.config_name == "custom.json");

0 commit comments

Comments
 (0)