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

Add comments to uncommented parts of code. #3250

Closed

Conversation

Vizvezdenec
Copy link
Contributor

A lot of latest patches are not commented properly which doesn't help in understanding what parts of code actually do (mine included).
So add/clarify code in evaluate and search.
No functional change.
bench 3561701

bench 3561701
@Vizvezdenec
Copy link
Contributor Author

https://github.com/official-stockfish/Stockfish/pull/3250/files#diff-da923b7afa45cab7add143c4705b54142e46b2afe9a2627d5fa3b3474bdc8aecR1729
// Increase stats for the best move in case it wa a capture move
I knew I wouldn have a typo somewhere.
Also grammatical cleanup from native english speakers will be appreciated.

src/evaluate.cpp Outdated
else
sf = 22 + 3 * pos.count<ALL_PIECES>(strongSide);
}
// For rook endgames with strong side not having overwhelming pawn number advantage
// and it pawns being on one flank
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

and its pawns being on one flank

src/evaluate.cpp Outdated
else
sf = 22 + 3 * pos.count<ALL_PIECES>(strongSide);
}
// For rook endgames with strong side not having overwhelming pawn number advantage
// and it pawns being on one flank
// and weak side protecting it pieces with a king
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

and weak side protecting its pieces with a king

src/search.cpp Outdated
@@ -1248,6 +1261,7 @@ namespace {
{
value = -search<NonPV>(pos, ss+1, -(alpha+1), -alpha, newDepth, !cutNode);

// If the move passed LMR update it stats
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If the move passed LMR update its stats

src/search.cpp Outdated
@@ -1333,6 +1346,7 @@ namespace {
}
}

// If move is worse than some previously searched move remember it to update it stats later
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If move is worse than some previously searched move remember it to update its stats later

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually, a comma and a the are missing, too.

Should be: If the move is worse than some previously searched move, remember it to update its stats later

src/search.cpp Outdated
for (int i = 0; i < quietCount; ++i)
{
thisThread->mainHistory[us][from_to(quietsSearched[i])] << -bonus2;
update_continuation_histories(ss, pos.moved_piece(quietsSearched[i]), to_sq(quietsSearched[i]), -bonus2);
}
}
else
// Increase stats for the best move in case it wa a capture move
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Increase stats for the best move in case it was a capture move

@Vizvezdenec
Copy link
Contributor Author

Vizvezdenec commented Dec 1, 2020

completely forgot to comment this line 1744/1767 of search, should be smth like
// update killer heuristic

@vondele
Copy link
Member

vondele commented Dec 4, 2020

@Vizvezdenec please update the patch with the review comments, so I can merge it in a next round

@Vizvezdenec
Copy link
Contributor Author

Done.

@vondele vondele added the to be merged Will be merged shortly label Dec 5, 2020
vondele pushed a commit to vondele/Stockfish that referenced this pull request Dec 5, 2020
@vondele
Copy link
Member

vondele commented Dec 5, 2020

thanks, also @crossbr for the review

@vondele vondele closed this Dec 5, 2020
@Vizvezdenec Vizvezdenec deleted the commentsAdd branch December 5, 2020 22:16
Dantist pushed a commit to Dantist/Stockfish that referenced this pull request Dec 22, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
to be merged Will be merged shortly
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants