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

Fix wrong assert. #2547

Closed
wants to merge 1 commit into from
Closed

Conversation

vondele
Copy link
Member

@vondele vondele commented Feb 7, 2020

can trigger an abort when compiling with debug=yes, and using 7men TB.
The assert should check that less than 8 pieces are in the key for each
side. Also take explicitly care of a 'v' character in material strings.

Fixes an issue reported in the forum:
https://groups.google.com/d/msg/fishcooking/yoVC7etIpz0/7mS7ntZMBAAJ

No functional change.

can trigger an abort when compiling with debug=yes, and using 7men TB.
The assert should check that less than 8 pieces are in the key for each
side. Also take explicitly care of a 'v' character in material strings.

Fixes an issue reported in the forum:
https://groups.google.com/d/msg/fishcooking/yoVC7etIpz0/7mS7ntZMBAAJ

No functional change.
@mrobaczyk
Copy link

Can't we just throw away the "v" character and leave the assert as it was? Less than 8 pieces for each side means assert will be true for KPPPPPPKPPPPPPP.

@vondele
Copy link
Member Author

vondele commented Feb 7, 2020

I don't think so, the original assert is limiting the total number of pieces to 7, while there is no reason for that as far as I can tell. It would trigger as soon as the first 8men TB files become available (or if we have 8men specialized endgame routines).

@joergoster
Copy link
Contributor

joergoster commented Feb 7, 2020

@vondele However, 8-man TB files seem too far away in the future to care about imho. I would eventually add a note in the comment, but only deal with what we currently have (max. 7-man tables/endgames).

@vondele
Copy link
Member Author

vondele commented Feb 10, 2020

@joergoster I agree that complete 8men TB are not so likely, but one could generate a few. More importantly, the asserts now reflect the assumptions of the implementation. So, I'll go ahead and merge this version.

@vondele vondele closed this in 0aee1ec Feb 10, 2020
joergoster pushed a commit to joergoster/Stockfish that referenced this pull request Sep 11, 2022
can trigger an abort when compiling with debug=yes, and using 7men TB.
The assert should check that less than 8 pieces are in the key for each
side, matching the assumption that underlies the FEN string construction.
Also take explicitly care of a 'v' character in material strings.

Fixes an issue reported in the forum:
https://groups.google.com/d/msg/fishcooking/yoVC7etIpz0/7mS7ntZMBAAJ

closes official-stockfish#2547

No functional change.
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 this pull request may close these issues.

None yet

3 participants