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

Simplify evaluate condition on search #3746

Closed
wants to merge 1 commit into from

Conversation

BM123499
Copy link
Contributor

Remove condition for MOVE_NULL on search.

STC:
LLR: 2.94 (-2.94,2.94) <-2.50,0.50>
Total: 47544 W: 11968 L: 11864 D: 23712
Ptnml(0-2): 150, 5535, 12318, 5599, 170
https://tests.stockfishchess.org/tests/view/616e37143799eb91f1f071ee

LTC:
LLR: 2.93 (-2.94,2.94) <-2.50,0.50>
Total: 67472 W: 16938 L: 16870 D: 33664
Ptnml(0-2): 49, 7119, 19331, 7189, 48
https://tests.stockfishchess.org/tests/view/616e3fab3799eb91f1f071f1

bench: 5255771

bench: 5255771
@vondele vondele added the to be merged Will be merged shortly label Oct 19, 2021
@vondele vondele closed this in b37054c Oct 19, 2021
@mstembera
Copy link
Contributor

I wish this was left open at least 24 hours so people had time to comment..

Removing this make things inconsistent with how the same thing is handled in qsearch. If it could be removed there as well that would be nice but it failed so IMO it would be better to leave it alone in both places. https://tests.stockfishchess.org/tests/view/616e856c942d40685e32376a

@BM123499
Copy link
Contributor Author

Well, I believe the use of evaluate(pos) is more correct than using -(ss-1)->staticEval. it would be excellent if we could use in both places, but since qsearch is not a perfect, I don't see a problem in having a cheap way in qsearch and a more correct in search.

@vondele
Copy link
Member

vondele commented Oct 20, 2021

I was rather quick applying it since, I think this is a good direction, i.e. since the net has some knowledge of tempo now, the evaluation after a null move can be quite different from just negating. The call in qsearch might be a bit more sensitive to the computational cost of this.

@mstembera
Copy link
Contributor

Yes but this patch doesn't gain elo so the question isn't whether this new search/qsearch inconsistency is justified by elo but whether it's justified by only removing the if statement.

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

3 participants