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

Small cleanup of nnue_feature_transformer.h #5745

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

xu-shawn
Copy link
Contributor

@xu-shawn xu-shawn commented Dec 29, 2024

Passed Non-regression STC:
LLR: 2.94 (-2.94,2.94) <-1.75,0.25>
Total: 285760 W: 73716 L: 73768 D: 138276
Ptnml(0-2): 777, 30775, 79851, 30677, 800
https://tests.stockfishchess.org/tests/view/676f78681a2f267f205485aa

bench 1294909

@mstembera
Copy link
Contributor

I noticed that sizeof(std::uint_fast8_t) is 1 byte which usually performs worse than int on modern 32 or 64 bit machines. What about std::uint_fast32_t or just leaving it as is?

@xu-shawn xu-shawn force-pushed the cleanup_feature_transformer branch from 1a96f92 to 86b7ee6 Compare December 30, 2024 08:02
@xu-shawn
Copy link
Contributor Author

done. would a re-test be needed?

@mstembera
Copy link
Contributor

Still on lines 773 & 782.

@xu-shawn xu-shawn force-pushed the cleanup_feature_transformer branch from 86b7ee6 to b1eb8ce Compare December 30, 2024 08:52
@Disservin
Copy link
Member

i don't see a particular need to introduce a new type for loop variables uint_fast32_t

@xu-shawn xu-shawn force-pushed the cleanup_feature_transformer branch from b1eb8ce to 3377f14 Compare December 31, 2024 06:24
bench 1294909
@xu-shawn xu-shawn force-pushed the cleanup_feature_transformer branch from 3377f14 to 2f2c973 Compare December 31, 2024 07:18
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.

3 participants