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

Incorrect comment on bit order in class BitBoard? #825

Open
rhalbersma opened this issue Mar 30, 2019 · 5 comments
Open

Incorrect comment on bit order in class BitBoard? #825

rhalbersma opened this issue Mar 30, 2019 · 5 comments
Labels
good first issue Good for newcomers

Comments

@rhalbersma
Copy link

In bitboard.h, for class BoardSquare the comment on line 43/44 states:

// As a single number, 0 to 63, bottom to top, left to right.
// 0 is a1, 8 is a2, 63 is h8.

In contrast, for the class BitBoard the comment on line 88/89 states:

// Bit enumeration goes from bottom to top, from left to right:
// Square a1 is bit 0, square a8 is bit 7, square b1 is bit 8.

However, the subsequent BitBoard member functions set, reset etc. take a BoardSquare parameter. So in fact BitBoard has the same ordering as BoardSquare.

Should this comment in class BitBoard be updated?

@mooskagh
Copy link
Member

Yeah initially it was wrong in both places, but then fixed in one but apparently not another.

@rhalbersma
Copy link
Author

There are some other improvements possible in bitboard.h. E.g. BitBoard::Mirror() could use the compiler intrinsics _byteswap_uint64 (MSVC) or __builtin_bswap64 (gcc/clang). This should reduce the current 15 instructions to 1 on modern architectures.

And ReverseBitsInBytes from mcts/node.cc could be moved to be a member function of BitBoard, and renamed MirrorVertical e.g. (and Mirror be renamed to MirrorHorizontal).

I also don’t quite understand the rationale to vertically mirror the inputs for training, could you explain?

I could send a PR later. What do you think?

@mooskagh
Copy link
Member

Those are good suggestsions, PR will be appreciated!

@TesseractA
Copy link
Contributor

Still Relevant?

@mooskagh
Copy link
Member

Yes, I've just check, still wrong (the comments issue in the very first comment). :)

@mooskagh mooskagh added the good first issue Good for newcomers label Apr 22, 2020
KarlKfoury added a commit to KarlKfoury/lc0 that referenced this issue Aug 24, 2024
KarlKfoury added a commit to KarlKfoury/lc0 that referenced this issue Aug 24, 2024
mooskagh pushed a commit that referenced this issue Aug 25, 2024
* Update bitboard.h

adressing issue #825

* Update bitboard.h

adressing issue #825
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Good for newcomers
Projects
None yet
Development

No branches or pull requests

3 participants