Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Halfmove count wraps at 63 #75

Closed
mbabigian opened this issue Aug 6, 2020 · 3 comments · Fixed by #182
Closed

Halfmove count wraps at 63 #75

mbabigian opened this issue Aug 6, 2020 · 3 comments · Fixed by #182

Comments

@mbabigian
Copy link

To reproduce import fens using "learn convert_bin" then rename the output file and reverse with "learn convert_plain" Check the output file. Look at fens that had a halfmove count greater than 63. 64 becomes 0, 65 becomes 1, etc.

Looks like an overflow.

Adding this issue to document the behavior.

@nodchip
Copy link
Owner

nodchip commented Aug 6, 2020

Thank you for reporting. I will take a look it after NNUE is integrated into the official Stockfish branch.

@gcp
Copy link

gcp commented Aug 20, 2020

The bug is here: https://github.com/nodchip/Stockfish/blob/master/src/extra/sfen_packer.cpp#L198

It's written as 6 bits, but the count is in halfmoves so it can go up to 100.

To keep data files compatible it will probably have to be clamped.

@Sopel97
Copy link
Collaborator

Sopel97 commented Oct 11, 2020

Okay, I'm looking at this now and trying to find a way to fix this without breaking backwards compatibility. It looks like it can be fixed by adding one more bit to the bit stream at the end. This would be completely backwards compatibile.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants