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

Simplify hashfull() #5651

Closed
wants to merge 1 commit into from
Closed

Conversation

mstembera
Copy link
Contributor

@mstembera mstembera commented Oct 24, 2024

STC https://tests.stockfishchess.org/tests/view/6719696086d5ee47d953cce6
LLR: 2.96 (-2.94,2.94) <-1.75,0.25>
Total: 79200 W: 20537 L: 20366 D: 38297
Ptnml(0-2): 267, 8736, 21387, 8979, 231

No functional change
bench: 1283457

Copy link

github-actions bot commented Oct 24, 2024

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 11805141837 / attempt 1)

@mstembera
Copy link
Contributor Author

@Disservin Is there something this needs since it wasn't merged with the recent batch of patches?

@Disservin
Copy link
Member

i'd prefer to leave this unchanged for better consistency with previous versions unless there is a real performance hit

@mstembera
Copy link
Contributor Author

@Disservin Are you referring to the constant 1000 to 500 change or the code changes? The reason I'm confused about your consistency comment with previous versions is because the the current master version of hashfull() was only recently written in PR #5354 just to get something working for benchmark. It actually clumsily duplicates the existing functionality of TTEntry::relative_age() (which was nicely written by you in https://github.com/official-stockfish/Stockfish/pull/5061/files) instead of using it. Prior to #5354 hashfull() looked like (click link below)

int TranspositionTable::hashfull() const {
which is much more consistent with this PR. Could you please take one more look and clarify? I will do whatever you decide of course. Thanks.

@Disservin
Copy link
Member

the usage of the appropiate functions is good, I just mean the 500

@mstembera
Copy link
Contributor Author

mstembera commented Nov 12, 2024

Ok the 500 is now reverted.
[Edit] Actually let me run it as an elo gainer first https://tests.stockfishchess.org/tests/view/6733bee386d5ee47d953e948

No functional change
bench: 1281912
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
to be merged Will be merged shortly
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants