Post-NNUE testing

[ianfab]

This is to discuss testing SF versions after the merge of NNUE evaluation code. One critical change is that the compiler needs to support c++17 instead of c++11 now, which raises the requirement to GCC >=7. In order to avoid crashing workers, please check the compiler versions you are using and upgrade if required @notruck @ddugovic @nordlandia @ppigazzini. As soon as we are clear that all workers are ready, we can run a first test with MV-SF.

[ppigazzini]

@ianfab workers OK :)

[ddugovic]

Thanks. I'm also working on verifying with Travis a new flag -DUSE_NNUE in case we need a way to remove/disable NNUE dependencies other than the forced compiler upgrade.

[notruck]

Sorry for the late reply. Shouldn't be a problem for me either.

[ddugovic]

FYI official-stockfish recently merged https://github.com/official-stockfish/Stockfish/pull/3091 which seemingly breaks my Travis CI valgrind for use-nnue and use-nnue-no branches https://travis-ci.org/github/ddugovic/Stockfish/branches until https://github.com/official-stockfish/Stockfish/pull/3063 is merged. See comment for details: https://github.com/official-stockfish/Stockfish/pull/3091#issuecomment-684567021

At the same time they introduced that bug, they also froze SF 12. I am not happy about this and I am fixing https://github.com/ddugovic/Stockfish/issues/582 while unable to verify in Travis CI that building without -DUSE_NNUE (effectively commenting out all the NNUE dependencies other than c++17) works.

[ianfab]

Thanks all for confirming, so c++17 code should compile fine on all workers.

@ddugovic I do not see how the PR you mentioned should cause undefined behavior, it just seems like a few useless instructions that could be skipped, but that is more of a cosmetic change. The CI error also seems to point to somewhere else.

I think that excluding the NNUE code should not be required for testing, since it does not add any external dependencies (unless I missed something?), just some more internal ones. However, if the NNUE code is included, bench and search of course must not use NNUE by default and NNUE_EMBEDDING_OFF must be set by default in order to make sure that the net is not used in any step of compilation, bench, and play.

[ddugovic]

Indeed, thanks to all for confirming!

Thanks Fabian, I think you are correct that the few useless instructions are unrelated to the CI error. I've managed to locally reproduce the valgrind error produced by Travis...

It's possible that both the current or future src/nnue/ implementations might not add external dependencies. I'll look further into all of this...

[ddugovic]

I have submitted a test of Multi-Variant SF not including NNUE support (it does not embed the NN weights): http://www.variantfishtest.org:6543/tests/view/5f50d5a56e23db221d9e905b

Official SF 12 enables NNUE by default... I'll maintain two branches (one which functions the same as upstream, another which is compatible with this fishtest server) although this will cause some confusion.

[gekkehenker]

Hope NNUE hasn't complicated matters too much. But maybe in the future it will also bring great oppertunities to elevate variants to the next level.

Anyway, hope my core injection in mv-Fishtest will be able to help development.

[ianfab]

@gekkehenker Thanks a lot for your contribution! The NNUE code should not complicate matters too much for testing, at least as long as we do not want to test nets for variants on fishtest. For Fairy-Stockfish I anyway still have to catch up with upstream merges, I am still 35 commits away from the NNUE merge, but if I manage to make it compatible for all major variants, having NNUE for variants like Xiangqi, Janggi, Makruk, S-Chess, etc. would be awesome and might give a huge performance boost, since the handcrafted evaluation for variants is of course far worse than the one for standard chess.