-
Notifications
You must be signed in to change notification settings - Fork 2.3k
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
Implement accumulator refresh table #5183
Conversation
a02661a
to
0712cab
Compare
For future reference. |
// When we are refreshing the accumulator of the big net, | ||
// redirect to the version of refresh that uses the refresh table. | ||
// Using the cache for the small net is not beneficial. | ||
if constexpr (HalfDimensions == Eval::NNUE::TransformedFeatureDimensionsBig) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was thinking it might make more sense to decide whether to use the cache at the callsite, and here just check for nullptr. This part of the code shouldn't even know what a big net is
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yea I am not a fan of having different behaviour inside the feature transformer for different network sizes, but I'd keep that as is for now since we've been testing the current version on fishtest right now and I think it is somewhat time to get it into master. I can take a look at that later again.
For each thread, keep a list of
AccumulatorRefreshEntry
, one for each possible king square.An
AccumulatorRefreshEntry
contains the bitboards to represent the pieces of a position, and an accumulator.When the accumulator needs to be refreshed, instead of filling it with biases and adding every piece from scratch, we...
AccumulatorRefreshEntry
associated with the new king bucket(from differences between bitboards in the entry and bitboards of the actual position)
I believe the structure of this new feature isn't implemented very well and needs a refactoring.
Credits to Luecx/Finny, who originally came up with this idea.
Results at STC:
https://tests.stockfishchess.org/tests/live_elo/662301573fe04ce4cefc1386
No functional change