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 readability of TTEntry occupancy check #5394

Closed
wants to merge 1 commit into from

Conversation

dubslow
Copy link
Contributor

@dubslow dubslow commented Jun 14, 2024

I say "fix" due #5263. That discussion was lengthy, and at least one comment appeared to not know what this code did. I wonder if it would have been submitted (or accepted) if that patch had read more like this one.

Also, setting aside the old PR, this is a clear readability gain anyways. Beginners can't understand it all, intermediates need to think a while to recall it, and even experts need a couple seconds per read to work out if it's an occupancy check or a genuine depth computation.

I think it just makes the code clearly easier to read and maintain.

Passed STC non-reg: https://tests.stockfishchess.org/tests/view/66695b6a602682471b064cfc
LLR: 2.93 (-2.94,2.94) <-1.75,0.25>
Total: 107520 W: 28138 L: 27998 D: 51384
Ptnml(0-2): 373, 12257, 28358, 12401, 371

no functional change

(and fix a stray comment)

I say "fix" due official-stockfish#5263 and its lengthy discussion. Beyond that, this is a clear readability and maintainbility improvement for beginners and experts alike.

Passed STC non-reg: https://tests.stockfishchess.org/tests/view/66695b6a602682471b064cfc
LLR: 2.93 (-2.94,2.94) <-1.75,0.25>
Total: 107520 W: 28138 L: 27998 D: 51384
Ptnml(0-2): 373, 12257, 28358, 12401, 371

no functional change
Copy link

clang-format 18 needs to be run on this PR.
If you do not have clang-format installed, the maintainer will run it when merging.
For the exact version please see https://packages.ubuntu.com/noble/clang-format-18.

(execution 9523455495 / attempt 1)

@vondele vondele added simplification A simplification patch to be merged Will be merged shortly labels Jun 15, 2024
@vondele vondele closed this in ff10f4a Jun 15, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
simplification A simplification patch to be merged Will be merged shortly
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants