Multiple errors, contact approach

[tuxmania87]

Hi,

I am using your library since a few days to create a search algorithm. I succeeded with it reaching around 1800 elo with relative simple means.

However there are many many bugs in your code and I fix them one by one as I approach them.

are you interested knowing them?
I could create a pull request for most of them.

I am not sure 100% that your code style or your idea of how you want to code complies with it and there are still some bugs that I dont care because they are not important for the search engine but still are tecnical wrong chess.

Please answer so we can discuss.

Here a short list out of memory whats wrong:

• fen loading into position is treating en passant square wrong (condition that square is between '3' and '6' is wrong
• Polyglot key calculation of material is wrong, i replaced it by my own and it generates the correct key
• polyglot lookup is wrong. the bit keys are reerted, instead of using the byte array you need to read the bytes and revert the array before casting it into u64 u32 or u16. this leads to no entries found in polyglot opening books
• in general the UCI representation of caste is wrong, "0-0" is no valid uci, it should be e1g1 simply and the engine should realice that this is a castling move simply because the king moves 2 squares
• related to the previous the UCI notation parsing is also not correct
• after e2e4 the en passant square is set to e3 (correctly) but when black tries to capture, the MakeMove already copied the state to the new state thus the en passant square gets cleared before makeMove() can check if e3 is ep square
• the isolated pawn bitmasks are wrongly calculated. it forgerts the direct neighbours of the pawn thus all pawns in start position are counted as isolated which is not true. I created an bitboard array "adjacentFiles" to calculate it correctly
• Suggestion to create a MakeMove that only receives Position and implicitly calles new State() since I couldnt reall find out why we need to pass a state

These are the most crucial ones I remember right now, If wished I can compile a full commented pull request

thanks for your work :) the ciricism seems to be rude but in fact i am very happy the whole bitboard approach is working very well.

[rudzen]

Let me try and break down a few things about what you listed.

Hi,

I am using your library since a few days to create a search algorithm. I succeeded with it reaching around 1800 elo with relative simple means.

However there are many many bugs in your code and I fix them one by one as I approach them.

are you interested knowing them?
I could create a pull request for most of them.

I am not sure 100% that your code style or your idea of how you want to code complies with it and there are still some bugs that I dont care because they are not important for the search engine but still are tecnical wrong chess.

Please answer so we can discuss.

Here a short list out of memory whats wrong:

• fen loading into position is treating en passant square wrong (condition that square is between '3' and '6' is wrong

An en-passant square can never be on the first two ranks from either side. It is using a simple calculation to determine if it's between 3 and 6 (inclusive). If verification is activated it should catch if it's rank 4 or 5.
Another simple approach would simple just to OR the square with valid rank bitboards - which is something I'm currently consider doing.

• Polyglot key calculation of material is wrong, i replaced it by my own and it generates the correct key

I'm aware of this - It's actually quite horrible atm.

• polyglot lookup is wrong. the bit keys are reerted, instead of using the byte array you need to read the bytes and revert the array before casting it into u64 u32 or u16. this leads to no entries found in polyglot opening books

Yes, the current polyglot code is not working as it should. It does not consider any possible endian conversions.

• in general the UCI representation of caste is wrong, "0-0" is no valid uci, it should be e1g1 simply and the engine should realice that this is a castling move simply because the king moves 2 squares

Correct, something I haven't had time to correct yet.

• related to the previous the UCI notation parsing is also not correct

This is related to previously mentioned issue.

• after e2e4 the en passant square is set to e3 (correctly) but when black tries to capture, the MakeMove already copied the state to the new state thus the en passant square gets cleared before makeMove() can check if e3 is ep square

If this didn't work - the Perft calculations would catch errors like this.

• the isolated pawn bitmasks are wrongly calculated. it forgerts the direct neighbours of the pawn thus all pawns in start position are counted as isolated which is not true. I created an bitboard array "adjacentFiles" to calculate it correctly

This is a case of lacking documentation. In this case an isolated pawn is a pawn which has no opposition on adjacent files. If there are pawns on adjacent files the pawn is able to be captured by the opposing pawn.
This should be made more clear what the intention is.

• Suggestion to create a MakeMove that only receives Position and implicitly calles new State() since I couldnt reall find out why we need to pass a state

Feel free to add a PR for this - my idea with this is to keep the states pre-made to reduce GC activity over time.

These are the most crucial ones I remember right now, If wished I can compile a full commented pull request

thanks for your work :) the ciricism seems to be rude but in fact i am very happy the whole bitboard approach is working very well.

Thanks for the feedback.
Please feel free to create more specific issues on what you have found so it would be easier to tie fix PR's to them.

[tuxmania87]

Refering to your en passant formula, to be more specific.

your code is:

State.EnPassantSquare = chunk.Length == 1 || chunk[0] == '-' || !chunk[0].InBetween('a', 'h') ? Square.None : chunk[1].InBetween('3', '6') ? Square.None : new Square(chunk[1] - '1', chunk[0] - 'a').Value;

Translated it means.

This is correct:
chunk.Length == 1 || chunk[0] == '-' || !chunk[0].InBetween('a', 'h') ? Square.None

This part is not
chunk[1].InBetween('3', '6') ? Square.None

It implied that when the chunk1 number is 3,4,5 or 6 we have no en passant.
but in fact en passant squares are always 3 or 6 . thats why this code never sets the en passant square when loading via fen. I tried it out and if you just play out games normally it pops up.

Its however rare since it only happens when you load a FEN having a en passant square flagged within the fen.

[tuxmania87]

• after e2e4 the en passant square is set to e3 (correctly) but when black tries to capture, the MakeMove already copied the state to the new state thus the en passant square gets cleared before makeMove() can check if e3 is ep square

If this didn't work - the Perft calculations would catch errors like this.

I will look at the perft code since it is in fact not working properly. Maybe its only sometimes but definetly the issue is that the en passant square gets clared before the movement if in the makemove can even check.

[rudzen]

Thanks for the input, I have merged fixes for the various issues you pointed out.