From e9997afb1cb046ed1812974f27c532f6e4d8dbcc Mon Sep 17 00:00:00 2001 From: Carlos Esparza Date: Mon, 3 Feb 2025 22:44:09 -0800 Subject: [PATCH] Replace hint_common_parent_position() by backwards accumulator updates Only calls to `evaluate()` now trigger NNUE accumulator updates. To make sure that we are likely to find parent positions from which to update the accumulators we perform a backwards NNUE update whenever we compute the accumulator from scratch for some position. passed STC LLR: 2.93 (-2.94,2.94) <0.00,2.00> Total: 39680 W: 10474 L: 10164 D: 19042 Ptnml(0-2): 171, 4068, 11042, 4398, 161 https://tests.stockfishchess.org/tests/view/67a27f26eb183d11c65945be passed LTC LLR: 2.94 (-2.94,2.94) <0.50,2.50> Total: 337308 W: 86408 L: 85550 D: 165350 Ptnml(0-2): 276, 30551, 106126, 31441, 260 https://tests.stockfishchess.org/tests/view/67a287efeb183d11c65945ee then simplified: STC LLR: 2.94 (-2.94,2.94) <-1.75,0.25> Total: 28608 W: 7641 L: 7413 D: 13554 Ptnml(0-2): 132, 3036, 7744, 3256, 136 https://tests.stockfishchess.org/tests/view/67a4703719f522d3866d3345 LTC LLR: 2.96 (-2.94,2.94) <-1.75,0.25> Total: 200226 W: 51026 L: 50990 D: 98210 Ptnml(0-2): 170, 18468, 62799, 18508, 168 https://tests.stockfishchess.org/tests/view/67a4f255229c1a170cc08964 The version in this PR is a bit different from the simplified version, but it's compile-time changes only. closes https://github.com/official-stockfish/Stockfish/pull/5870 No functional change --- src/nnue/network.cpp | 6 -- src/nnue/network.h | 3 - src/nnue/nnue_feature_transformer.h | 141 ++++++++++++++++++---------- src/nnue/nnue_misc.cpp | 10 -- src/nnue/nnue_misc.h | 3 - src/position.h | 2 +- src/search.cpp | 4 - 7 files changed, 92 insertions(+), 77 deletions(-) diff --git a/src/nnue/network.cpp b/src/nnue/network.cpp index b96625a79..5ac8b8d98 100644 --- a/src/nnue/network.cpp +++ b/src/nnue/network.cpp @@ -278,12 +278,6 @@ void Network::verify(std::string } -template -void Network::hint_common_access( - const Position& pos, AccumulatorCaches::Cache* cache) const { - featureTransformer->hint_common_access(pos, cache); -} - template NnueEvalTrace Network::trace_evaluate(const Position& pos, diff --git a/src/nnue/network.h b/src/nnue/network.h index f99fa1182..764481d94 100644 --- a/src/nnue/network.h +++ b/src/nnue/network.h @@ -67,9 +67,6 @@ class Network { AccumulatorCaches::Cache* cache) const; - void hint_common_access(const Position& pos, - AccumulatorCaches::Cache* cache) const; - void verify(std::string evalfilePath, const std::function&) const; NnueEvalTrace trace_evaluate(const Position& pos, AccumulatorCaches::Cache* cache) const; diff --git a/src/nnue/nnue_feature_transformer.h b/src/nnue/nnue_feature_transformer.h index 4f0ce6cf6..931d9aed5 100644 --- a/src/nnue/nnue_feature_transformer.h +++ b/src/nnue/nnue_feature_transformer.h @@ -41,6 +41,11 @@ using BiasType = std::int16_t; using WeightType = std::int16_t; using PSQTWeightType = std::int32_t; +enum IncUpdateDirection { + FORWARD, + BACKWARDS +}; + // If vector instructions are enabled, we update and refresh the // accumulator tile by tile such that each tile fits in the CPU's // vector registers. @@ -249,6 +254,7 @@ class FeatureTransformer { // Number of output dimensions for one side static constexpr IndexType HalfDimensions = TransformedFeatureDimensions; + static constexpr bool Big = TransformedFeatureDimensions == TransformedFeatureDimensionsBig; private: using Tiling = SIMDTiling; @@ -468,38 +474,20 @@ class FeatureTransformer { return psqt; } // end of function transform() - void hint_common_access(const Position& pos, - AccumulatorCaches::Cache* cache) const { - update_accumulator(pos, cache); - update_accumulator(pos, cache); - } - private: - template - StateInfo* try_find_computed_accumulator(const Position& pos) const { - // Look for a usable accumulator of an earlier position. We keep track - // of the estimated gain in terms of features to be added/subtracted. - StateInfo* st = pos.state(); - int gain = FeatureSet::refresh_cost(pos); - while (st->previous && !(st->*accPtr).computed[Perspective]) - { - // This governs when a full feature refresh is needed and how many - // updates are better than just one full refresh. - if (FeatureSet::requires_refresh(st, Perspective) - || (gain -= FeatureSet::update_cost(st) + 1) < 0) - break; - st = st->previous; - } - return st; - } - - // Given a computed accumulator, computes the accumulator of the next position. - template - void update_accumulator_incremental(const Position& pos, StateInfo* computed) const { + // Given a computed accumulator, computes the accumulator of another position. + template + void update_accumulator_incremental(const Square ksq, + StateInfo* target_state, + const StateInfo* computed) const { + [[maybe_unused]] constexpr bool Forward = Direction == FORWARD; + [[maybe_unused]] constexpr bool Backwards = Direction == BACKWARDS; assert((computed->*accPtr).computed[Perspective]); - assert(computed->next != nullptr); - const Square ksq = pos.square(Perspective); + StateInfo* next = Forward ? computed->next : computed->previous; + + assert(next != nullptr); + assert(!(next->*accPtr).computed[Perspective]); // The size must be enough to contain the largest possible update. // That might depend on the feature set and generally relies on the @@ -508,11 +496,11 @@ class FeatureTransformer { // In this case, the maximum size of both feature addition and removal // is 2, since we are incrementally updating one move at a time. FeatureSet::IndexList removed, added; - FeatureSet::append_changed_indices(ksq, computed->next->dirtyPiece, removed, - added); - - StateInfo* next = computed->next; - assert(!(next->*accPtr).computed[Perspective]); + if constexpr (Forward) + FeatureSet::append_changed_indices(ksq, next->dirtyPiece, removed, added); + else + FeatureSet::append_changed_indices(ksq, computed->dirtyPiece, added, + removed); if (removed.size() == 0 && added.size() == 0) { @@ -527,7 +515,10 @@ class FeatureTransformer { { assert(added.size() == 1 || added.size() == 2); assert(removed.size() == 1 || removed.size() == 2); - assert(added.size() <= removed.size()); + if (Forward) + assert(added.size() <= removed.size()); + else + assert(removed.size() <= added.size()); #ifdef VECTOR auto* accIn = @@ -539,13 +530,15 @@ class FeatureTransformer { const IndexType offsetR0 = HalfDimensions * removed[0]; auto* columnR0 = reinterpret_cast(&weights[offsetR0]); - if (removed.size() == 1) + if ((Forward && removed.size() == 1) || (Backwards && added.size() == 1)) { + assert(added.size() == 1 && removed.size() == 1); for (IndexType i = 0; i < HalfDimensions * sizeof(WeightType) / sizeof(vec_t); ++i) accOut[i] = vec_add_16(vec_sub_16(accIn[i], columnR0[i]), columnA0[i]); } - else if (added.size() == 1) + else if (Forward && added.size() == 1) { + assert(removed.size() == 2); const IndexType offsetR1 = HalfDimensions * removed[1]; auto* columnR1 = reinterpret_cast(&weights[offsetR1]); @@ -553,8 +546,19 @@ class FeatureTransformer { accOut[i] = vec_sub_16(vec_add_16(accIn[i], columnA0[i]), vec_add_16(columnR0[i], columnR1[i])); } + else if (Backwards && removed.size() == 1) + { + assert(added.size() == 2); + const IndexType offsetA1 = HalfDimensions * added[1]; + auto* columnA1 = reinterpret_cast(&weights[offsetA1]); + + for (IndexType i = 0; i < HalfDimensions * sizeof(WeightType) / sizeof(vec_t); ++i) + accOut[i] = vec_add_16(vec_add_16(accIn[i], columnA0[i]), + vec_sub_16(columnA1[i], columnR0[i])); + } else { + assert(added.size() == 2 && removed.size() == 2); const IndexType offsetA1 = HalfDimensions * added[1]; auto* columnA1 = reinterpret_cast(&weights[offsetA1]); const IndexType offsetR1 = HalfDimensions * removed[1]; @@ -576,14 +580,15 @@ class FeatureTransformer { const IndexType offsetPsqtR0 = PSQTBuckets * removed[0]; auto* columnPsqtR0 = reinterpret_cast(&psqtWeights[offsetPsqtR0]); - if (removed.size() == 1) + if ((Forward && removed.size() == 1) + || (Backwards && added.size() == 1)) // added.size() == removed.size() == 1 { for (std::size_t i = 0; i < PSQTBuckets * sizeof(PSQTWeightType) / sizeof(psqt_vec_t); ++i) accPsqtOut[i] = vec_add_psqt_32(vec_sub_psqt_32(accPsqtIn[i], columnPsqtR0[i]), columnPsqtA0[i]); } - else if (added.size() == 1) + else if (Forward && added.size() == 1) { const IndexType offsetPsqtR1 = PSQTBuckets * removed[1]; auto* columnPsqtR1 = @@ -595,6 +600,18 @@ class FeatureTransformer { vec_sub_psqt_32(vec_add_psqt_32(accPsqtIn[i], columnPsqtA0[i]), vec_add_psqt_32(columnPsqtR0[i], columnPsqtR1[i])); } + else if (Backwards && removed.size() == 1) + { + const IndexType offsetPsqtA1 = PSQTBuckets * added[1]; + auto* columnPsqtA1 = + reinterpret_cast(&psqtWeights[offsetPsqtA1]); + + for (std::size_t i = 0; + i < PSQTBuckets * sizeof(PSQTWeightType) / sizeof(psqt_vec_t); ++i) + accPsqtOut[i] = + vec_add_psqt_32(vec_add_psqt_32(accPsqtIn[i], columnPsqtA0[i]), + vec_sub_psqt_32(columnPsqtA1[i], columnPsqtR0[i])); + } else { const IndexType offsetPsqtA1 = PSQTBuckets * added[1]; @@ -647,8 +664,8 @@ class FeatureTransformer { (next->*accPtr).computed[Perspective] = true; - if (next != pos.state()) - update_accumulator_incremental(pos, next); + if (next != target_state) + update_accumulator_incremental(ksq, target_state, next); } @@ -815,16 +832,40 @@ class FeatureTransformer { template void update_accumulator(const Position& pos, AccumulatorCaches::Cache* cache) const { - if ((pos.state()->*accPtr).computed[Perspective]) - return; - StateInfo* oldest = try_find_computed_accumulator(pos); + StateInfo* st = pos.state(); + if ((st->*accPtr).computed[Perspective]) + return; // nothing to do - if ((oldest->*accPtr).computed[Perspective] && oldest != pos.state()) - // Start from the oldest computed accumulator, update all the - // accumulators up to the current position. - update_accumulator_incremental(pos, oldest); - else - update_accumulator_refresh_cache(pos, cache); + [[maybe_unused]] // only used when !Big + int gain = FeatureSet::refresh_cost(pos); + // Look for a usable already computed accumulator of an earlier position. + // When computing the small accumulator, we keep track of the estimated gain in + // terms of features to be added/subtracted. + // When computing the big accumulator, we expect to be able to reuse any + // accumulators, so we always try to do an incremental update. + do + { + if (FeatureSet::requires_refresh(st, Perspective) + || (!Big && (gain -= FeatureSet::update_cost(st) < 0)) || !st->previous + || st->previous->next != st) + { + // compute accumulator from scratch for this position + update_accumulator_refresh_cache(pos, cache); + if (Big && st != pos.state()) + // when computing a big accumulator from scratch we can use it to + // efficiently compute the accumulator backwards, until we get to a king + // move. We expect that we will need these accumulators later anyway, so + // computing them now will save some work. + update_accumulator_incremental( + pos.square(Perspective), st, pos.state()); + return; + } + st = st->previous; + } while (!(st->*accPtr).computed[Perspective]); + + // Start from the oldest computed accumulator, update all the + // accumulators up to the current position. + update_accumulator_incremental(pos.square(Perspective), pos.state(), st); } template diff --git a/src/nnue/nnue_misc.cpp b/src/nnue/nnue_misc.cpp index 1e2690503..2220684da 100644 --- a/src/nnue/nnue_misc.cpp +++ b/src/nnue/nnue_misc.cpp @@ -30,7 +30,6 @@ #include #include -#include "../evaluate.h" #include "../position.h" #include "../types.h" #include "../uci.h" @@ -43,15 +42,6 @@ namespace Stockfish::Eval::NNUE { constexpr std::string_view PieceToChar(" PNBRQK pnbrqk"); -void hint_common_parent_position(const Position& pos, - const Networks& networks, - AccumulatorCaches& caches) { - if (Eval::use_smallnet(pos)) - networks.small.hint_common_access(pos, &caches.small); - else - networks.big.hint_common_access(pos, &caches.big); -} - namespace { // Converts a Value into (centi)pawns and writes it in a buffer. // The buffer must have capacity for at least 5 chars. diff --git a/src/nnue/nnue_misc.h b/src/nnue/nnue_misc.h index a7647f846..02212160a 100644 --- a/src/nnue/nnue_misc.h +++ b/src/nnue/nnue_misc.h @@ -54,9 +54,6 @@ struct Networks; struct AccumulatorCaches; std::string trace(Position& pos, const Networks& networks, AccumulatorCaches& caches); -void hint_common_parent_position(const Position& pos, - const Networks& networks, - AccumulatorCaches& caches); } // namespace Stockfish::Eval::NNUE } // namespace Stockfish diff --git a/src/position.h b/src/position.h index 53269c197..eeea1b74c 100644 --- a/src/position.h +++ b/src/position.h @@ -63,9 +63,9 @@ struct StateInfo { int repetition; // Used by NNUE + DirtyPiece dirtyPiece; Eval::NNUE::Accumulator accumulatorBig; Eval::NNUE::Accumulator accumulatorSmall; - DirtyPiece dirtyPiece; }; diff --git a/src/search.cpp b/src/search.cpp index 67e28d3e5..f292118ed 100644 --- a/src/search.cpp +++ b/src/search.cpp @@ -41,7 +41,6 @@ #include "nnue/network.h" #include "nnue/nnue_accumulator.h" #include "nnue/nnue_common.h" -#include "nnue/nnue_misc.h" #include "position.h" #include "syzygy/tbprobe.h" #include "thread.h" @@ -759,7 +758,6 @@ Value Search::Worker::search( else if (excludedMove) { // Providing the hint that this node's accumulator will be used often - Eval::NNUE::hint_common_parent_position(pos, networks[numaAccessToken], refreshTable); unadjustedStaticEval = eval = ss->staticEval; } else if (ss->ttHit) @@ -768,8 +766,6 @@ Value Search::Worker::search( unadjustedStaticEval = ttData.eval; if (!is_valid(unadjustedStaticEval)) unadjustedStaticEval = evaluate(pos); - else if (PvNode) - Eval::NNUE::hint_common_parent_position(pos, networks[numaAccessToken], refreshTable); ss->staticEval = eval = to_corrected_static_eval(unadjustedStaticEval, correctionValue);