official-stockfish / Stockfish

A free and strong UCI chess engine
https://stockfishchess.org/
GNU General Public License v3.0
11.56k stars 2.27k forks source link

Improve dealing with the default net? #3030

Closed vondele closed 4 years ago

vondele commented 4 years ago

This is an issue to see if we should improve the current way we look for the default net. Currently, the default net will be found if it has been downloaded and copied to the working directory of the engine, or if the user specified the 'EvalFile' with the proper path if the file is located elsewhere. Bullet points are mixed pro/cons.

Several options exist:

While most things can be easily implemented, the question is is we need them, or if they become a source of confusion and mistakes.

gekkehenker commented 4 years ago

I think integrating a net is a nice thing for a SF12 (or 13, 14, etc.) official release. Releases tend to attract the people who just want it to work with a single click and not worry about settings and other "difficult" stuff. For them SF12 can be the one click install.

In other cases I think it's fine just the way it's currently is.

nguyenpham commented 4 years ago

I think one of the strong points of current SF NNUE is that there are so few networks to choose (or that is a very weak point ;) ) and there is always a suggested-network as the default value of EvalFile. Thus users get less confusion to find, select, download... In contrast, I have seen so many supports/answers from LC0's team were just about the need for networks, which/where to download, etc... Thus, I hove SF NNUE can continue that advantage: always have one good network fit for multi-purpose/hardware.

For chess GUIs, I think once they support downloading networks, they can do more work to manage them. For example, they can search networks from multi folders, sort to display, verify, delete, re-download them, let users select and auto do the rest (such as setting up paths). Basically, if we accept the help from chess GUIs, we have done on that issue since chess GUIs have almost no limits on libraries, interfaces to work with, users don't have to go to the engine level/folder to fix nn problems.

vondele commented 4 years ago

@gekkehenker We should not adopt a different method for our releases from the one we use during development. Only if we're able to use it ourselves all the time will it be good enough.

snicolet commented 4 years ago

I asked @DragonMist in the TCEC chat about the situation in Fritz, as it is the most used GUI (90% of chess users), here are his answers. At the end Aloril also stepped in, suggesting something similar to option 2.


Stephane_Nicolet: Anybody using Fritz GUI here? Leonardo?

DragonMist66: yup

Stephane_Nicolet: how difficult is it to setup the nnue files in Fritz? in other words: is the current installation procedure good enough? how could we improve on it?

DragonMist66: rather difficult and probably impossible for the average user

Stephane_Nicolet : can you send a screencopy of the parameters dialog?

DragonMist66: http://cassio.free.fr/divers/MCZrPqs.png MCZrPqs

DragonMist66: there is no Browse button in the parameters, I would have that first.

DragonMist66: At the moment, user must manualy enter full path, remember to add a | at the end, then copy and paste at the end also the full name of the file of nnue...

Stephane_Nicolet: Where did you choose to put the nnue file? with the stockfish exe or in a separate directory?

DragonMist66: with the Stockfish exe

Stephane_Nicolet: Do you have to do this for each new SF binary you download from Abrok?

DragonMist66: of course

Stephane_Nicolet: aie aie aie

DragonMist66: I think just adding a Browse button and feature near the EvalFile window would solve most of the problems for the average user. That is why I said on fishcooking and here that there are more astronauts in the world than people able to create SFNNUE uci functionally at this time. Some anti marketing geniuses advising me to install Arena, etc. Nonsense when 90% of the ordinary folks use Fritz GUI.

Stephane_Nicolet: Problem is that adding a browse button in the dialog is a Fritz decision, not ours. And they will not help us. We have to make our magic ourselves to make it as easy as possible for the astronauts.

Aloril42: Detect file named default EvalFile in same directory Stockfish is?

Stephane_Nicolet: There is no easy way in C++17 to detect the directory where the executable is :-(

Viceroy-Sam commented 4 years ago

DragonMist66: At the moment, user must manualy enter full path, remember to add a | at the end, then copy and paste at the end also the full name of the file of nnue...

A SyzygyPath is used for the directory of the Tablebases. I don't see that on the above screenshot in the Fritz GUI or how that is dealt with in the Fritz GUI.

An EvalPath is likely required so user doesn't have to use '|' pipe in Evalfile entry.

noobpwnftw commented 4 years ago

Weighting in the possibility of net architecture change and/or bug fixes, it is more preferred if the net is embedded in the official binary releases. Develop versions are of course in another story.

DragonMist commented 4 years ago

I misstyped, one has to enter full path, then a \, and paste nnue file name at the end. So not | but .

ned, 23. kol 2020. 00:58 Sam notifications@github.com je napisao:

DragonMist66: At the moment, user must manualy enter full path, remember to add a | at the end, then copy and paste at the end also the full name of the file of nnue...

A SyzygyPath is used for the directory of the Tablebases. I don't see that on the above screenshot in the Fritz GUI or how that is dealt with in the Fritz GUI.

An EvalPath is likely required so user doesn't have to use '|' pipe in Evalfile entry.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/official-stockfish/Stockfish/issues/3030#issuecomment-678705702, or unsubscribe https://github.com/notifications/unsubscribe-auth/AGEFTDTH4VRNPJKLTMBKH33SCBEP5ANCNFSM4QFEXACA .

the-real-greco commented 4 years ago

I think integrating a net is a nice thing for a SF12 (or 13, 14, etc.) official release. Releases tend to attract the people who just want it to work with a single click and not worry about settings and other "difficult" stuff. For them SF12 can be the one click install.

@gekkehenker We should not adopt a different method for our releases from the one we use during development. Only if we're able to use it ourselves all the time will it be good enough.

I've come here to beg of you, please, PLEASE make using a net as stupid-easy as you can.

Keep in mind that a lot of people using Stockfish will be 10 years old, and a lot of them will be like 70. Not to say that you can't be both old and technically proficient, but some fraction will have no idea how to set a path. Or know what GUI means other than "Chessbase". Or be able to navigate to a forum, Discord, etc. where they can find help.

Trust me. I'm in CCC chat a lot. I love our fans but (some of their) their technical knowledge is close to zero. And those are the people who are actually interested in this stuff.

"This is easy, users will figure this out" is never true. It just isn't.

snicolet commented 4 years ago

For reference and maybe inspiration, here is the Leela code: https://github.com/LeelaChessZero/lc0/tree/master/src/utils

They merged a commit a few weeks ago to change the default directory: https://github.com/LeelaChessZero/lc0/commit/d9679beadada77420a3ee6cb3ca394c193073385

noobpwnftw commented 4 years ago

I'll just break the sugar coating: giving an average user the ability to fiddle with what net to load is simply useless, they almost always heard some of those hearsay than using the default net. Expert users who has any clue about what they are doing would have absolute no trouble with having the path set correctly, or figure out whether the net and binary are compatible.

I just don't like the idea that it should have any options available to the user that may change the engine internal workings, unless absolutely necessary.

vdbergh commented 4 years ago

Note: the posted code in Option 2 does not actually work on Linux. More precisely, in the most common use case where we invoke stockfish simply as stockfish where " stockfish " is somewhere in a directory in the PATH variable (say /usr/local/bin) then std::filesystem::path(argv[0]).remove_filename().string() returns the empty path.

In other words std::filesystem doesn't perform any magic. We may just as well take the part of argv[0] before the last path separator (/ or \) and use the result if it is sensible. This eliminates the dependency on std::filesystem.

snicolet commented 4 years ago

We may just as well take the part of argv[0] before the last path separator (/ or \) and use it if the result is sensible. This eliminates the dependency on std::filesystem.

Agreed, this is also the way Lc0 does it.

Edit: implemented in https://github.com/official-stockfish/Stockfish/pull/3054

nguyenpham commented 4 years ago

I have been continuing to work on how chess GUIs can help users dealing with SF networks. The NNUE download dialog now includes network management. It works as below logic: 1) Popup

2) Download

3) All existent networks

In most cases, users just need to click on the button "Download" only and all are done.

Screen Shot 2020-08-23 at 6 13 23 pm

snicolet commented 4 years ago

As a funny subtext for the current discussion, please watch the small video of https://github.com/official-stockfish/Stockfish/issues/3052 .

Obviously this "Samer707" chess fan is so fast that @the-real-greco is right, he will never read a Readme nor navigate to a forum, a Discord, etc... What he wants is download a Stockfish version, point his GUI at the executable, and run analysis.

Isn't interested in nnue files, no Readme, no parameter dialog, no nothing...

download exe, point, run.

It must work out of the box for 99% of users :-)

vondele commented 4 years ago

which will not necessarily work by having SF look for a net close to the binary. Just two typical cases, are linux distros which will put those files in different locations, and users that will download the files as they always did, and move the exe to where they store their other exes, separating them from the network. If we really think users are not smart enough, the only option I see is to embed it in the binary.

vdbergh commented 4 years ago

I do not think we have to worry about linux packaging. The packagers will simply patch the source so that SF will find the net. Of course the tools used for generating binary packages are equipped to do this automatically.

IMHO: one sensible way to help linux packagers is to create a makefile argument with the default value for the NNUE path.

EDIT: Another way to help linux users is to provide a make install target. This will not help packagers since the directories will be wrong (e.g. /usr/local/bin instead of /usr/bin) but it will definitely help users who compile SF from source.

vondele commented 4 years ago

I think fundamentally we have two choices:

1) we provide a single file 2) we provide two files.

IMO:

1) will make it 'fool-proof'. If a user manages to download the exe, it will probably work. It will come with a number of downsides related to very large binaries (e.g. slow build, more difficult distribution (github/abrok), virus scanners going crazy, etc). Even if our nets today are 'small', this will likely change. One thing that is missing is a patch that implements this option.

2) users will always find a way to separate the two, and have any amount of 'smart logic' fail, so we'll patch it up, over and over.

gvreuls commented 4 years ago

How about this scheme:

This way you keep the ease of distributing development versions with the "argv[0] magic", while packagers are able to compile in another default path and net without drastically changing the code. GUIs and/or users who put their nets in yet another location are assumed to know what they're doing and must set EvalFile manually; there is no difference in doing this between home compiled versions, development distributed versions and Linux distro packaged versions, the logic is always the same.

So here is the logic: 1) If EvalFile is empty, use the compiled in default path and net. 2) If EvalFile isn't empty, try to load the net from EvalFile without applying any magic. 3) If 2. fails, prepend the argv[0] starting directory to EvalFile and try to load the net from the resulting location.

vondele commented 4 years ago

@snicolet patch contains this, we could easily make it like

std::vector<std::string> dirs = { "" , CommandLine::binaryDirectory };

distros

std::vector<std::string> dirs = { "" , CommandLine::binaryDirectory, "DEFAULT_NET_DISTROS" };

where DEFAULT_NET_DISTROS could be changed with a define at compile time. So, would search 3 directories instead of 2.

gvreuls commented 4 years ago

@vondele You need some preprocessor-foo to get a -D macro quoted: https://github.com/official-stockfish/Stockfish/blob/530fccbf272ffe424ae53a464b91db148cc968ae/src/misc.cpp#L165-L166 but basically:

#ifdef DEFAULT_NET_DISTROS
  std::vector<std::string> dirs = { "" , CommandLine::binaryDirectory, stringify(DEFAULT_NET_DISTROS) };
#else
  std::vector<std::string> dirs = { "" , CommandLine::binaryDirectory };
#endif

and a minor makefile change and we're in business (if you go for it).

Edit: the suggested makefile change is overkill unless we automate the clearing of EvalFile also.

syzygy1 commented 4 years ago

Perhaps there could be two kinds of builds: (i) a build with integrated network and without an EvalFile option, and (ii) a build without integrated network and with an EvalFile option.

The official builds for regular users would be of type (i). I guess this is the only way to make SF12 available to regular chess players without causing them headaches and frustration.

For advanced users wishing to experiment with different nets, builds of type (ii) might also be provided. Alternatively, they have to compile such a build themselves or obtain it from someone else.

I do think it is important to keep builds of type (ii) since SF development can benefit from users being able to test other networks. (Also, it would be annoying for sites like abrok to have to provide 20M+ development versions for download.)

If the network is (optionally) included in the SF binary, we might want to reconsider compressing the (integrated) network file, e.g. to save bandwidth.

Dantist commented 4 years ago

it would be annoying for sites like abrok to have to provide 20M+ development versions for download we might want to reconsider compressing the (integrated) network file

It can be 7MB: https://github.com/nodchip/Stockfish/issues/76 But things may change as the net architecture evolves.

syzygy1 commented 4 years ago

@Dantist

it would be annoying for sites like abrok to have to provide 20M+ development versions for download we might want to reconsider compressing the (integrated) network file

It can be 7MB: nodchip#76 But things may change as the net architecture evolves.

That proposal seems to be about some sort of special-purpose compression, which may well break (or become inefficient) if the network architecture changes. It probably makes more sense to use a general-purpose compressor like gzip/zlib. Of course the decompression library will have to be statically linked or we would just be replacing one dependency for another.

Hmm, I just realise that the argument against including decompression code for "external" network files is just as valid for integrated network files. Instead of letting SF decompress the network weights, just provide the SF binary as a zip file...

daylen commented 4 years ago

When we distribute Stockfish 12, it will be in zip files. So I don't think size is a huge concern for a main release.

@vondele is right, fundamentally the choice is between distributing 1 file or 2 files. I think that with enough "smart logic" the engine will be able to find the EvalFile most of the time…

Another idea is that rather than stop analysis when the EvalFile is not found with this error:

info string ERROR: NNUE evaluation used, but the network file nn-82215d0fd0df.nnue was not loaded successfully.
info string ERROR: The UCI option EvalFile might need to specify the full path, including the directory/folder name, to the file.
info string ERROR: The default net can be downloaded from: https://tests.stockfishchess.org/api/nn/nn-82215d0fd0df.nnue
info string ERROR: If the UCI option Use NNUE is set to true, network evaluation parameters compatible with the program must be available.
info string ERROR: The engine will be terminated now.

we fallback to the classical evaluation. This would make the engine "always work" but at the cost of sometimes not using NNUE. If the user is wondering which eval is actually being used, the engine does say which eval is in use with info string but I'm not sure if all GUIs display that string.

Dantist commented 4 years ago

It seems that as the engine develops, the classical search alone will become weaker and weaker, so the binary with strongest classical search - is the binary compiled just before the NNUE branch merge. If so, then the NNUE file should be an integral part of the engine and over time the stockfish without the NNUE file alongside, will simply be "too weak"..

nguyenpham commented 4 years ago

IMO, stopping when the network not found is better than the auto-switching to the classic one. Users must find and solve the problem instead of ignoring and assuming there is no problem. They may find and fix the problem much quicker. No point to continue since it is not what users want, according to the engine's settings.

Chess GUIs can show info strings, typically not in popups but just normal windows/panels (to avoid disturbing users too much). Users may turn those panels off or hide them under other panels thus they may easily ignore that info. Thus, the stop/crash of the engine may take much more attention.

vondele commented 4 years ago

The following is a link to a repo that deals with including binary data in the compiled binary/executable:

https://github.com/graphitemaster/incbin

It works great in clang/gcc and 10+ other compilers (i.e. essentially no added compile time), but MSVC support requires generating an additional source file, which needs about 1-2 min to compile. The same is true for the header generated via xxd -i nn-82215d0fd0df.nnue > nn.h.

There is also a proposal for std::embed http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2020/p1040r6.html but that has gone nowhere so far.

syzygy1 commented 4 years ago

I think what you mean is including a binary data file in the compiled binary/executable (not "including binaries in sources").

syzygy1 commented 4 years ago

(Not trying to nitpick, but I was about to question why the network file would have to be included in the SF sources.)

MJZ1977 commented 4 years ago

Personally, I find that "option 1" is working well and not complicated. But if there is some difficulties to use it, why not using a new parameter to default path (option 4?)

noobpwnftw commented 4 years ago

We are mainly dealing with 3 types of binary files:

  1. PE/COFF, Windows, the most trivial way is to add the net as overlay data at the end of the file on disk, very common practice for most self-extracting installers and Whatever2EXE(SWF2EXE, Perl2EXE and so on), antivirus may complain, but we can apply digital signatures(Authenticode) to silence most of them.
  2. ELF, Linux, the most trivial way is to add data as extra symbol/object with the linker, as described earlier.
  3. Mach-O, Mac, very similar to ELF and can probably do the same.

There are plenty of scenarios where such embedding mechanism is needed for various reasons, and there are well-established specifications within each executable format on how to accomplish this.

vondele commented 4 years ago

thinking a little more with it, using incbin seems like it could work. <internal> could be added as one more path to https://github.com/official-stockfish/Stockfish/pull/3054 and on those setups where incbin works (essentially all) we could first see if the net is part of the binary, falling back to the other mechanisms as needed (also e.g. for non-default networks).

Only MSVC users would compile their own binaries that would never find something under <internal> since it is not supported, they would continue to use the current mechanism. Since nobody distributes MSVC compiled binaries as far as I know, the magic would work for essentially all users that download binaries.

If somebody could come up with a prototype patch, that would be appreciated. I expect this will need an iteration or two, since it needs to smoothly work with fishtest, CI, etc.

snicolet commented 4 years ago

There is a prototype in this pull request: https://github.com/official-stockfish/Stockfish/pull/3070

vondele commented 4 years ago

So, IMO #3070 looks like an approach that covers the most important cases from the user perspective.

1) It embeds the default net in the binary, so a download of that binary will result in a working engine with the default net, no need to fiddle around with any options.

2) It still allows non-default nets to be used, which will be looked for in up to three directories (working directory, location of the binary, a distro specific directory). This mechanism is also kept for those developers that use MSVC, the one compiler that doesn't have an easy mechanism for embedding data.

Would be good if #3070 could get some testing in the wild, since it uses some less standard code. In particular, testing on windows would be needed.

Comments on the approach, in particular overlooked aspects, would be welcome as well.

snicolet commented 4 years ago

One small caveat of #3070 is that (if the default net is not yet in the src directory) #3070 relies on the availability of an Internet connection to download the net before compiling: the API https://tests.stockfishchess.org/api/nn/ has to be available and running.

Today around 5:30 CST time, it so happened that fishtest went down, so I took the opportunity to rename the default net in my src directory and try to compile #3070, I got a make error for the implicit net target of the Makefile after one minute:

make clean make build make: *** [net] Error 7

Just to be clear, this is only relevant for compiling from sources, once the end users get the big exe with the embedded net on their hard disc they will be able to use it, internet connection or not.

hartmajr commented 4 years ago

Option 1 seems best to me. FWIW.

Alayan-stk-2 commented 4 years ago

I think embedding a net is a good solution for many end users. The less effort the user has to spend to make things run correctly, the less likely the user will mess up something.

Ebola-Chan-bot commented 4 years ago

After so many days of practice after deciding to embed, I am now firmly opposed to embedding. Look at my analysis of pros and cons:

Pros: 1

Cons: 2

BTW, please don't use Straw Poll on abrok.eu. This site is broken on its account login or register, so voters can't leave comments. ——Added 20200906—— However, why don't we suggest those, who have no interest in understanding the exe-net relationship, to simply download the official release version, which has a net embedded? That's where we actually want plain users to go, isn't it? I probably didn't make it clear before. What I mean is that, it should be only the release version, but never the development build, that has the net embedded in.

noobpwnftw commented 4 years ago

@Silver-Fang Your goal contradicts your approach, on one hand you want to save download time, storage space and bandwidth, on the other hand, you want to download a binary for every commit, decompress and archive them in uncompressed form. This outcome is decided by two factors: size of the binary and the frequency of commits, however you only see the former as a problem, for the latter you can simply download a compile once every week/month/year, which would easily meet your desired goal of saving download time, storage space and bandwidth.

Meanwhile, do people very often download, compile, or archive every Chrome, GCC, or any other projects' nightly build? If not, why would they do that to a chess engine in particular? It is exactly for those people who cannot understand the difference between a release and a nightly build, that we need to have everything embedded to save their day.

vondele commented 4 years ago

I'm very strongly in favor of keeping the default net embedded. Let's optimistically estimate that only 5% of the roughly 1M users of SF struggle only 10min to find the right solution. That's literally 1 year of human time lost. In reality it is much more. Even I only transferred the binary to my phone after the net was embedded, as I couldn't (care to) find out where to put the net and how to specify it...

Embedding only makes it easier for the majority of people. Most that whine don't even verify that you can still run any net you want. The only downside is the storage.... those that are capable of dealing with a non-embedded net are some hobbyists, and they should spent $50 on a 500GB HD that stores them 25000 exes ($0.002 per stored binary).

I would have preferred that the exes are not zipped on abrok, I do think it adds one click too many. But that's up to the owner of the site to decide.

syzygy1 commented 4 years ago

What the Stockfish source code offers as default compilation option shouldn't stop sites like abrok.eu from compiling SF without embedded net, should they so wish (up to them).

vondele commented 4 years ago

The problem is that most issues that users have with binaries downloaded on abrok come to us (github issues, fishtest, direct messages, etc), and we do suggest people to get their dev versions from abrok. So if that source would not be suitable anymore, we would presumably have to create an alternative, which seems like an unneeded effort.

Ebola-Chan-bot commented 4 years ago

@noobpwnftw @vondele However, why don't we suggest those, who have no interest in understanding the exe-net relationship, to simply download the official release version, which has a net embedded? That's where we actually want plain users to go, isn't it? I probably didn't make it clear before. What I mean is that, it should be only the release version, but never the development build, that has the net embedded in. If one wants to get the development build, he ought to learn more about the net than plain users; otherwise, he has to admit he is just one of the plain users that should go to the release version.

vondele commented 4 years ago

@Silver-Fang so far abrok was a site for users, you seem to want to limit it to a small subset of people. Why optimize for those who can't affort a 20MB download, instead of keeping the latest and strongest SF available to a wide user base? The former can compile that special version from sources. I'm really unable to see what use-case you have in mind that is blocked by a 20MB exe.