Updated Huffman code

[dpldgr]

Updated to support 26 piece types.

[ianfab]

Thanks. It would be good to also update the dataSize estimation in on_variant_change as well, to consider 9 bit instead of 5 per piece when piece types >= 16 since this should likely consider the worst case to be safe. Also it would be nice to have a corresponding PR for the parsing in the trainer, although that isn't that urgent considering it is almost backwards-compatible.

[dpldgr]

Thanks. It would be good to also update the dataSize estimation in on_variant_change as well, to consider 9 bit instead of 5 per piece when piece types >= 16 since this should likely consider the worst case to be safe. Also it would be nice to have a corresponding PR for the parsing in the trainer, although that isn't that urgent considering it is almost backwards-compatible.

Ahhh yes, it presumes that it's always 5 bits. If someone specifies something like customPiece1 and then customPiece20 in variants.ini, would that use 9 bits for customPiece20, or does the parser skip empty slots?

I am planning to do a PR for the trainer too, I just need to check the end loop condition won't be an issue as it references an enum value rather than using a literal to terminate.

[ianfab]

The NNUE piece index only depends on the order, not the value of the enum of the pieces types (and the trainer therefore can't even know which types these actually are), see

variant-nnue-tools/src/variant.cpp

Lines 1969 to 1976 in cc4094b

	for (PieceSet ps = pieceTypes; ps;)
	{
	// Make sure that the nnueKing type gets the last index, since the NNUE architecture relies on that
	PieceType pt = lsb(ps != piece_set(nnueKing) ? ps & ~piece_set(nnueKing) : ps);
	ps ^= pt;
	assert(pt != nnueKing || !ps);
	pieceIndex[pt] = i;

So only the count of used types is what matters, and the 16th type and later will consume 9 bit then, i.e., it might even apply for built-in types if you use enough of them.

[ianfab]

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. be v->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.

[dpldgr]

Sorry, I should have picked that one up, I broke a long-standing rule to not code just before bed! 😴

[ianfab]

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

	for(auto c: Colors)
	for (PieceSet ps = pos.piece_types(); ps;)
	stream.write_n_bit(pos.count_in_hand(c, pop_lsb(ps)), DATA_SIZE > 512 ? 7 : 5);

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.

[ianfab]

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.

[dpldgr]

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.

[ianfab]

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.