Replace Edges and Corner arrays in endgames.cpp

[protonspring]

This is a functional simplification that removes the large arrays in endgames.cpp. I submit this version (ps_endgames9) because it performed best at LTC, but may regress a little at STC. In this patch, the values are closer to master, but are more expensive to calculate.

ps_endgames8 is faster, but less accurate. This seems to support better at STC performance, but worse at LTC. This may be just noise, I don't know. Is ps_endgames8 better?
http://tests.stockfishchess.org/tests/user/protonspring

STC
LLR: 2.94 (-2.94,2.94) {-1.50,0.50}
Total: 174724 W: 33325 L: 33404 D: 107995
Ptnml(0-2): 2503, 19607, 43181, 19608, 2463
http://tests.stockfishchess.org/tests/view/5e6448ffe42a5c3b3ca2e287

LTC
LLR: 2.95 (-2.94,2.94) {-1.50,0.50}
Total: 35640 W: 4679 L: 4621 D: 26340
Ptnml(0-2): 189, 2991, 11424, 3005, 211
http://tests.stockfishchess.org/tests/view/5e650b24e42a5c3b3ca2e2d8

Bench 5070301

[vondele]

merge into master, with the addition of a fen that should trigger endgames more often.

[MichaelB7]

This was not a simplification that one would expect with a simplification. It is a functional change which obfuscates the array that was being used which was quite easy for a reader to understand. In addition the alleged bug was not explained or detailed. If there was a bug created by a recent prior commit, the buggy commit should be reverted first and this simplification should be tested against the unbugged version. This commit being accepted as a simplification is dubious at best. In the big scheme of things , it probably matters very little, but it does set a precedent which may not be best practice. Just my $.02.

[mstembera]

IMO the arrays made it much easier to comprehend what the code was actually doing. The formulas while using less syntax are more obfuscated. Also I appreciate the fast commits of patches but it would be nice to leave things open for say 48 hours in order to allow people to have a chance to comment before committing.

[vondele]

I agree this one was a bit too quick. This was indeed because it contained a bug fix. That bug fix should have been presented as a separate PR.

[protonspring]

Let me know if you'd like me to do something to fix this.

…

On Tue, Mar 10, 2020 at 11:58 PM Joost VandeVondele < ***@***.***> wrote: I agree this one was a bit too quick. This was indeed because it contained a bug fix. That bug fix should have been presented as a separate PR. — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#2577?email_source=notifications&email_token=AHCWOSHQKQHOGU7N247SE4TRG4SAFA5CNFSM4LELCPDKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEOOHXQI#issuecomment-597457857>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AHCWOSEAKN2ZEIT67EOAF7TRG4SAFANCNFSM4LELCPDA> .

-- Michael T. Whiteley mobile: 801.707.6886

[vondele]

@protonspring I think there is nothing to be done right now. I'd suggest next time you notice a bug in an earlier commit, communicate it clearly and early.... bugs happen, we just need to deal with it effectively.

With the added fen, at least our testing improves a bit.... long ago I proposed to have some coverage testing as part of CI, and at that point, I observed that KBNK was not covered, unfortunately, that was not followed up.