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

Improve FRC correction. #3730

Closed
wants to merge 1 commit into from
Closed

Conversation

vondele
Copy link
Member

@vondele vondele commented Oct 5, 2021

As FRC can not be tested on fishtest, locally tuned and tested:

100k games at STC:
Elo: 2.36 +- 1.07
LOS: 0.999992

Bench: 5714575

As FRC can not be tested on fishtest, locally tuned and tested:

Elo: 2.36 +- 1.07
LOS: 0.999992

Bench: 5714575
@snicolet
Copy link
Member

snicolet commented Oct 5, 2021

It may also be possible to do the same for lines 482-483 of evaluation.cpp

if (   pos.is_chess960()
   && (s == relative_square(Us, SQ_A1) || s == relative_square(Us, SQ_H1)))
    {
        Direction d = pawn_push(Us) + (file_of(s) == FILE_A ? EAST : WEST);
        if (pos.piece_on(s + d) == make_piece(Us, PAWN))
           score -= !pos.empty(s + d + pawn_push(Us)) ? 4 * make_score(CorneredBishop, CorneredBishop)
                                                      : 3 * make_score(CorneredBishop, CorneredBishop);
   }

Since this code is only used in Chess960 and when the classical eval is called, this is really a corner case :-) and we could even commit such a change directly, as the result in your test is enough to build confidence, IMHO.

@NightlyKing
Copy link
Contributor

Wow, a FRC patch out of the blue. A surprise to be sure - but a welcome one.
Congratulations!

@vondele
Copy link
Member Author

vondele commented Oct 6, 2021

@snicolet I prefer not to change classical eval at this point. It definitely would need testing since the correction should depend on the 'base evaluation', and I think it is not worth resources to do that. I think we should essentially consider the classical eval frozen for now.

@snicolet snicolet added the to be merged Will be merged shortly label Oct 6, 2021
@snicolet snicolet closed this in 329bdbd Oct 6, 2021
@snicolet
Copy link
Member

snicolet commented Oct 6, 2021

Merged via 329bdbd, congrats :-)

proukornew pushed a commit to proukornew/Stockfish that referenced this pull request Oct 8, 2021
As Chess960 patches can not be tested on fishtest, this was locally tuned
and tested:

Elo: 2.36 +- 1.07
LOS: 0.999992

closes official-stockfish#3730

Bench: 5714575
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.

3 participants