-
-
Notifications
You must be signed in to change notification settings - Fork 12
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
Updated Huffman code #16
Draft
dpldgr
wants to merge
14
commits into
fairy-stockfish:tools
Choose a base branch
from
dpldgr:tools
base: tools
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Draft
Changes from 4 commits
Commits
Show all changes
14 commits
Select commit
Hold shift + click to select a range
1ba9e20
Updated Huffman code
dpldgr b3fbf71
Fixed off by one error.
dpldgr a821dbd
Updated name of header file that needs to be updated.
dpldgr 8003405
Update dataSize estimate.
dpldgr e8ff7d9
Updated estimate for maximum pieces on the board.
dpldgr a73604d
Merge branch 'fairy-stockfish:tools' into tools
dpldgr 6bd4bc1
Testing GitHub actions.
dpldgr 33f0a04
Changed huffman code to 6 bits
dpldgr 68f68d4
Automatically determine how many piece bits are needed in the trainin…
dpldgr f846997
Fixed read_board_piece_from_stream
dpldgr 0e1c156
Fixed.
dpldgr 21d717c
Try again.
dpldgr eeeb272
3rd attempt.
dpldgr 88967a3
Updated piece type bits.
dpldgr File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
these are the bits for the pocket counts, see https://github.com/fairy-stockfish/variant-nnue-pytorch/wiki/Technical-details#training-data-format, the
v->nnueMaxPieces * 5
is the one for the pieces on the board. It could e.g. bev->nnueMaxPieces * (typeCount > 15 ? 9 : 5)
. I think using worst case estimates here (all pieces are of the last type) is reasonable in order to be safe.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.
Sorry, I should have picked that one up, I broke a long-standing rule to not code just before bed! 😴
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.
thanks. The other change should be reverted though, since the 5 bit for the pockets are counts per type (5 bit = [0, 31]), i.e., they are fixed size and not related to the huffman encoding. However, it is using 7 bit when using the bigger data size, see
variant-nnue-tools/src/tools/sfen_packer.cpp
Lines 200 to 202 in cc4094b
but that does not need to be considered here. It could just be added as an additional criterion, but this isn't really related to this PR, so no need to add it here unless you would like to.
Edit: I just realized this formula requires tons of knowledge about the code base to decipher, so I should have added a comment explaining each term. I might add that later.
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.
Sorry if there might have been a misunderstanding. The changes regarding the bits for the pieces in hand still needs to be reverted. The comments on this formula I will add once this PR is merged to avoid merge conflicts.
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.
Hi, there was a misunderstanding it seems, but I have also been busy preparing to move house so haven't gotten around to fixing this yet. I'll make it a draft again for the moment until I have some time to revert the pieces in hand.
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.
Ah, ok. There is no rush of course, so take your time. Or if you want I could also push the changes to the PR from my side.