Skip to content

Bug and fix in do_null_move #3108

@syzygy1

Description

@syzygy1

I'm not really sure what the correct procedure is for bug fixes. The testing guidelines tell me to discuss it in the forum, but the forum doesn't seem to be the right place anymore.

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).

If others agree that this is a bug, I can start a non-regression test on the following fix:
syzygy1/Stockfish@503fa01

It would also be possible to simplify the bug away by removing the computed_score flag. SF never uses it (or at least never in a correct manner). Where SF needs the evaluation of the current position multiple times, it already keeps it in a local variable.

Lastly, it should be possible to speed up do_null_move() a little by copying the previous accumulator only when it is useful (when computed_accumulator is set). I tried this a while ago but (to my surprise) could not detect a clear speed up, so I am not convinced that it would pass as an improvement (I did not try it in fishtest). I could combine this small probable improvement with the bug fix in a non-regression test, but that would be kind of cheating. (Any ideas?)

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions