Skip to content

Commit fc27d15

Browse files
syzygy1vondele
syzygy1
authored and
vondele
committed
Bug fix in do_null_move() and NNUE simplification.
This fixes #3108 and removes some NNUE code that is currently not used. At the moment, do_null_move() copies the accumulator from the previous state into the new state, which is correct. It then clears the "computed_score" flag because the side to move has changed, and with the other side to move NNUE will return a completely different evaluation (normally with changed sign but also with different NNUE-internal tempo bonus). The problem is that do_null_move() clears the wrong flag. It clears the computed_score flag of the old state, not of the new state. It turns out that this almost never affects the search. For example, fixing it does not change the current bench (but it does change the previous bench). This is because the search code usually avoids calling evaluate() after a null move. This PR corrects do_null_move() by removing the computed_score flag altogether. The flag is not needed because nnue_evaluate() is never called twice on a position. This PR also removes some unnecessary {}s and inserts a few blank lines in the modified NNUE files in line with SF coding style. Resulf ot STC non-regression test: LLR: 2.95 (-2.94,2.94) {-1.25,0.25} Total: 26328 W: 3118 L: 3012 D: 20198 Ptnml(0-2): 126, 2208, 8397, 2300, 133 https://tests.stockfishchess.org/tests/view/5f553ccc2d02727c56b36db1 closes #3109 bench: 4109324
1 parent d539da1 commit fc27d15

File tree

4 files changed

+29
-71
lines changed

4 files changed

+29
-71
lines changed

‎src/nnue/evaluate_nnue.cpp‎

Lines changed: 4 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -115,31 +115,16 @@ namespace Eval::NNUE {
115115
return stream && stream.peek() == std::ios::traits_type::eof();
116116
}
117117

118-
// Proceed with the difference calculation if possible
119-
static void UpdateAccumulatorIfPossible(const Position& pos) {
120-
121-
feature_transformer->UpdateAccumulatorIfPossible(pos);
122-
}
123-
124-
// Calculate the evaluation value
125-
static Value ComputeScore(const Position& pos, bool refresh) {
126-
127-
auto& accumulator = pos.state()->accumulator;
128-
if (!refresh && accumulator.computed_score) {
129-
return accumulator.score;
130-
}
118+
// Evaluation function. Perform differential calculation.
119+
Value evaluate(const Position& pos) {
131120

132121
alignas(kCacheLineSize) TransformedFeatureType
133122
transformed_features[FeatureTransformer::kBufferSize];
134-
feature_transformer->Transform(pos, transformed_features, refresh);
123+
feature_transformer->Transform(pos, transformed_features);
135124
alignas(kCacheLineSize) char buffer[Network::kBufferSize];
136125
const auto output = network->Propagate(transformed_features, buffer);
137126

138-
auto score = static_cast<Value>(output[0] / FV_SCALE);
139-
140-
accumulator.score = score;
141-
accumulator.computed_score = true;
142-
return accumulator.score;
127+
return static_cast<Value>(output[0] / FV_SCALE);
143128
}
144129

145130
// Load eval, from a file stream or a memory stream
@@ -150,19 +135,4 @@ namespace Eval::NNUE {
150135
return ReadParameters(stream);
151136
}
152137

153-
// Evaluation function. Perform differential calculation.
154-
Value evaluate(const Position& pos) {
155-
return ComputeScore(pos, false);
156-
}
157-
158-
// Evaluation function. Perform full calculation.
159-
Value compute_eval(const Position& pos) {
160-
return ComputeScore(pos, true);
161-
}
162-
163-
// Proceed with the difference calculation if possible
164-
void update_eval(const Position& pos) {
165-
UpdateAccumulatorIfPossible(pos);
166-
}
167-
168138
} // namespace Eval::NNUE

‎src/nnue/nnue_accumulator.h‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -29,9 +29,7 @@ namespace Eval::NNUE {
2929
struct alignas(kCacheLineSize) Accumulator {
3030
std::int16_t
3131
accumulation[2][kRefreshTriggers.size()][kTransformedFeatureDimensions];
32-
Value score;
3332
bool computed_accumulation;
34-
bool computed_score;
3533
};
3634

3735
} // namespace Eval::NNUE

‎src/nnue/nnue_feature_transformer.h‎

Lines changed: 25 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -50,11 +50,13 @@ namespace Eval::NNUE {
5050

5151
// Hash value embedded in the evaluation file
5252
static constexpr std::uint32_t GetHashValue() {
53+
5354
return RawFeatures::kHashValue ^ kOutputDimensions;
5455
}
5556

5657
// Read network parameters
5758
bool ReadParameters(std::istream& stream) {
59+
5860
for (std::size_t i = 0; i < kHalfDimensions; ++i)
5961
biases_[i] = read_little_endian<BiasType>(stream);
6062
for (std::size_t i = 0; i < kHalfDimensions * kInputDimensions; ++i)
@@ -64,23 +66,26 @@ namespace Eval::NNUE {
6466

6567
// Proceed with the difference calculation if possible
6668
bool UpdateAccumulatorIfPossible(const Position& pos) const {
69+
6770
const auto now = pos.state();
68-
if (now->accumulator.computed_accumulation) {
71+
if (now->accumulator.computed_accumulation)
6972
return true;
70-
}
73+
7174
const auto prev = now->previous;
7275
if (prev && prev->accumulator.computed_accumulation) {
7376
UpdateAccumulator(pos);
7477
return true;
7578
}
79+
7680
return false;
7781
}
7882

7983
// Convert input features
80-
void Transform(const Position& pos, OutputType* output, bool refresh) const {
81-
if (refresh || !UpdateAccumulatorIfPossible(pos)) {
84+
void Transform(const Position& pos, OutputType* output) const {
85+
86+
if (!UpdateAccumulatorIfPossible(pos))
8287
RefreshAccumulator(pos);
83-
}
88+
8489
const auto& accumulation = pos.state()->accumulator.accumulation;
8590

8691
#if defined(USE_AVX2)
@@ -177,6 +182,7 @@ namespace Eval::NNUE {
177182
private:
178183
// Calculate cumulative value without using difference calculation
179184
void RefreshAccumulator(const Position& pos) const {
185+
180186
auto& accumulator = pos.state()->accumulator;
181187
IndexType i = 0;
182188
Features::IndexList active_indices[2];
@@ -216,9 +222,8 @@ namespace Eval::NNUE {
216222
&accumulator.accumulation[perspective][i][0]);
217223
auto column = reinterpret_cast<const __m64*>(&weights_[offset]);
218224
constexpr IndexType kNumChunks = kHalfDimensions / (kSimdWidth / 2);
219-
for (IndexType j = 0; j < kNumChunks; ++j) {
225+
for (IndexType j = 0; j < kNumChunks; ++j)
220226
accumulation[j] = _mm_add_pi16(accumulation[j], column[j]);
221-
}
222227

223228
#elif defined(USE_NEON)
224229
auto accumulation = reinterpret_cast<int16x8_t*>(
@@ -240,11 +245,11 @@ namespace Eval::NNUE {
240245
#endif
241246

242247
accumulator.computed_accumulation = true;
243-
accumulator.computed_score = false;
244248
}
245249

246250
// Calculate cumulative value using difference calculation
247251
void UpdateAccumulator(const Position& pos) const {
252+
248253
const auto prev_accumulator = pos.state()->previous->accumulator;
249254
auto& accumulator = pos.state()->accumulator;
250255
IndexType i = 0;
@@ -288,33 +293,27 @@ namespace Eval::NNUE {
288293

289294
#if defined(USE_AVX2)
290295
auto column = reinterpret_cast<const __m256i*>(&weights_[offset]);
291-
for (IndexType j = 0; j < kNumChunks; ++j) {
296+
for (IndexType j = 0; j < kNumChunks; ++j)
292297
accumulation[j] = _mm256_sub_epi16(accumulation[j], column[j]);
293-
}
294298

295299
#elif defined(USE_SSE2)
296300
auto column = reinterpret_cast<const __m128i*>(&weights_[offset]);
297-
for (IndexType j = 0; j < kNumChunks; ++j) {
301+
for (IndexType j = 0; j < kNumChunks; ++j)
298302
accumulation[j] = _mm_sub_epi16(accumulation[j], column[j]);
299-
}
300303

301304
#elif defined(USE_MMX)
302305
auto column = reinterpret_cast<const __m64*>(&weights_[offset]);
303-
for (IndexType j = 0; j < kNumChunks; ++j) {
306+
for (IndexType j = 0; j < kNumChunks; ++j)
304307
accumulation[j] = _mm_sub_pi16(accumulation[j], column[j]);
305-
}
306308

307309
#elif defined(USE_NEON)
308310
auto column = reinterpret_cast<const int16x8_t*>(&weights_[offset]);
309-
for (IndexType j = 0; j < kNumChunks; ++j) {
311+
for (IndexType j = 0; j < kNumChunks; ++j)
310312
accumulation[j] = vsubq_s16(accumulation[j], column[j]);
311-
}
312313

313314
#else
314-
for (IndexType j = 0; j < kHalfDimensions; ++j) {
315-
accumulator.accumulation[perspective][i][j] -=
316-
weights_[offset + j];
317-
}
315+
for (IndexType j = 0; j < kHalfDimensions; ++j)
316+
accumulator.accumulation[perspective][i][j] -= weights_[offset + j];
318317
#endif
319318

320319
}
@@ -325,33 +324,27 @@ namespace Eval::NNUE {
325324

326325
#if defined(USE_AVX2)
327326
auto column = reinterpret_cast<const __m256i*>(&weights_[offset]);
328-
for (IndexType j = 0; j < kNumChunks; ++j) {
327+
for (IndexType j = 0; j < kNumChunks; ++j)
329328
accumulation[j] = _mm256_add_epi16(accumulation[j], column[j]);
330-
}
331329

332330
#elif defined(USE_SSE2)
333331
auto column = reinterpret_cast<const __m128i*>(&weights_[offset]);
334-
for (IndexType j = 0; j < kNumChunks; ++j) {
332+
for (IndexType j = 0; j < kNumChunks; ++j)
335333
accumulation[j] = _mm_add_epi16(accumulation[j], column[j]);
336-
}
337334

338335
#elif defined(USE_MMX)
339336
auto column = reinterpret_cast<const __m64*>(&weights_[offset]);
340-
for (IndexType j = 0; j < kNumChunks; ++j) {
337+
for (IndexType j = 0; j < kNumChunks; ++j)
341338
accumulation[j] = _mm_add_pi16(accumulation[j], column[j]);
342-
}
343339

344340
#elif defined(USE_NEON)
345341
auto column = reinterpret_cast<const int16x8_t*>(&weights_[offset]);
346-
for (IndexType j = 0; j < kNumChunks; ++j) {
342+
for (IndexType j = 0; j < kNumChunks; ++j)
347343
accumulation[j] = vaddq_s16(accumulation[j], column[j]);
348-
}
349344

350345
#else
351-
for (IndexType j = 0; j < kHalfDimensions; ++j) {
352-
accumulator.accumulation[perspective][i][j] +=
353-
weights_[offset + j];
354-
}
346+
for (IndexType j = 0; j < kHalfDimensions; ++j)
347+
accumulator.accumulation[perspective][i][j] += weights_[offset + j];
355348
#endif
356349

357350
}
@@ -362,7 +355,6 @@ namespace Eval::NNUE {
362355
#endif
363356

364357
accumulator.computed_accumulation = true;
365-
accumulator.computed_score = false;
366358
}
367359

368360
using BiasType = std::int16_t;

‎src/position.cpp‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -704,7 +704,6 @@ void Position::do_move(Move m, StateInfo& newSt, bool givesCheck) {
704704

705705
// Used by NNUE
706706
st->accumulator.computed_accumulation = false;
707-
st->accumulator.computed_score = false;
708707
auto& dp = st->dirtyPiece;
709708
dp.dirty_num = 1;
710709

@@ -1000,7 +999,6 @@ void Position::do_null_move(StateInfo& newSt) {
1000999
if (Eval::useNNUE)
10011000
{
10021001
std::memcpy(&newSt, st, sizeof(StateInfo));
1003-
st->accumulator.computed_score = false;
10041002
}
10051003
else
10061004
std::memcpy(&newSt, st, offsetof(StateInfo, accumulator));

0 commit comments

Comments
 (0)