Remove 'si' StateInfo variable/parameter. #4444
Conversation
Since st is a member of position we don't need to pass it separately as parameter. Passed non-regression test https://tests.stockfishchess.org/tests/view/64098d562644b62c33942b35 LLR: 3.24 (-2.94,2.94) <-1.75,0.25> Total: 548960 W: 145834 L: 146134 D: 256992 Ptnml(0-2): 1617, 57652, 156261, 57314, 1636 While being there also remove some line in pos_is_ok, where a copy of StateInfo was made by using default copy constructor and then verified it's correctedness by doing a memcmp. There is no point in doing that. no functional change bench: 4747020
There is a call to set_state() before the memcmp. Since it's inside of pos_is_ok() which is used for debug verification purposes only it may just be there to confirm the current StateInfo is correct (self consistent with the current position). |
|
OK, missed that. |
|
I think the check can be removed, we have other ways of checking against using uninitialized data. |
no functional change bench: 4747020
Ups, I just pushed a restore-commit, just in these minutes. |
This reverts commit 8c5ba90.
I am indifferent to keeping it or removing it but there should be no need for any special implementation to keep it. If you simply change line 1330 in master from [Edit] @vondele Are you still in favor of removing it given that nothing special has to happen? |
|
I'm still fine with deleting the check. However, CI is repeatedly failing... I wonder what's going on? |
4776866 is indeed the current master bench, which is correctly obtained by the CI test. the reference bench is in the commits shown in this PR. I think simply rebasing this PR on master would suffice to fix the CI. |
Since st is a member of position we don't need to pass it separately as parameter. Passed non-regression test https://tests.stockfishchess.org/tests/view/64098d562644b62c33942b35 LLR: 3.24 (-2.94,2.94) <-1.75,0.25> Total: 548960 W: 145834 L: 146134 D: 256992 Ptnml(0-2): 1617, 57652, 156261, 57314, 1636 While being there also remove some line in pos_is_ok, where correct initialization of StateInfo is being verified. no functional change bench: 4776866
| set_check_info(st); | ||
| set_check_info(); | ||
|
|
||
| st->repetition = 0; |
There was a problem hiding this comment.
I wonder if this line st->repetition = 0; could be removed also, it generates the same bench for me, I would appreciate it if someone explained how though?
There was a problem hiding this comment.
I think even if we omit zeroing st->repetition on null move the bench remains stable because of the st->pliesFromNull guard which prevents to consider a move-sequence as 3-fold-repetition when there's a null-move in the chain.
P.S.: all related methods do_move, has_repeated, has_game_cycle take care of st->pliesFromNull
Since st is a member of position we don't need to pass it separately as parameter. While being there also remove some line in pos_is_ok, where a copy of StateInfo was made by using default copy constructor and then verified it's correctedness by doing a memcmp. There is no point in doing that. Passed non-regression test https://tests.stockfishchess.org/tests/view/64098d562644b62c33942b35 LLR: 3.24 (-2.94,2.94) <-1.75,0.25> Total: 548960 W: 145834 L: 146134 D: 256992 Ptnml(0-2): 1617, 57652, 156261, 57314, 1636 closes official-stockfish#4444 No functional change
5 participants
Since st is a member of position we don't need to pass it separately as parameter.
This allows us to use unique variable name 'st' for all occurences.
Passed non-regression test
https://tests.stockfishchess.org/tests/view/64098d562644b62c33942b35
LLR: 3.24 (-2.94,2.94) <-1.75,0.25>
Total: 548960 W: 145834 L: 146134 D: 256992
Ptnml(0-2): 1617, 57652, 156261, 57314, 1636
While being there also remove some line in pos_is_ok, where a copy of StateInfo was made by using default copy constructor and then verified it's correctedness by doing a memcmp. There is no point in doing that.
no functional change
bench: 4747020