Stockfish icon indicating copy to clipboard operation
Stockfish copied to clipboard

1st step to build SF as shared library

Open ab-chesspad opened this issue 1 year ago • 23 comments

I would still want to be able to build SF as a shared library. It gives certain benefits to the user-developer, e.g. easy usage, flexibility, etc. To achieve this I need to make three main changes to SF:

  1. Entry points to invoke SF: launch/initialize quit execute any SF function read SF output read SF error output
  2. Read commands and write output using std:cin and std:cout only in the standalone mode. Provide async mechanizm for input/output in the shared library mode.
  3. 'Non-stop' execution. SF must not crash and stop only when getting 'quit' command. In case of an error SF reports it and rolls back to the previous state.

And of course, all SF functionality and performance must be preserved. I have implemented all these goals in my SF fork, but after a glitch during sync, I lost all the info. So now I am starting from scratch, using my local repo, this time in stages, so that people can easily review and analyze my submissions. This is the first part of step 1: Create the entry point to execute any SF command for the future shared library. Thank you Alex Bootman

ab-chesspad avatar Jun 15 '23 16:06 ab-chesspad

I'm not sure opening pull requests, something meant to merge this changes into the main StockFish repository, makes sense. You are better off working on your own Repository.

PGG106 avatar Jun 15 '23 16:06 PGG106

Stockfish is a UCI chess engine. I don't see there are major reasons to introduce shared-library-compatible features. People can fork the repository and do their stuffs individually, and I believe this one also falls in such category.

MinetaS avatar Jun 15 '23 16:06 MinetaS

I have implemented all these goals in my SF fork, but after a glitch during sync, I lost all the info.

I don't know if it helps in your case but if you've committed your changes, you should still find them via git reflog.

UniQP avatar Jun 15 '23 17:06 UniQP

thank you, I decided to redo it from scratch anyway. Thank you Alex Bootman

On Thu, Jun 15, 2023 at 10:02 AM Sebastian Buchwald < @.***> wrote:

I have implemented all these goals in my SF fork, but after a glitch during sync, I lost all the info.

I don't know if it helps in your case but if you've commited your changes, you should still find them via git reflog.

— Reply to this email directly, view it on GitHub https://github.com/official-stockfish/Stockfish/pull/4626#issuecomment-1593434456, or unsubscribe https://github.com/notifications/unsubscribe-auth/AF4K74KMN5BG3HE52KNAZATXLM523ANCNFSM6AAAAAAZIDPILM . You are receiving this because you authored the thread.Message ID: @.***>

ab-chesspad avatar Jun 15 '23 20:06 ab-chesspad

of course Stockfish is a UCI engine and I am planning to keep it this way. But why cannot UCI engine be implemented as a shared library? Thank you Alex Bootman

On Thu, Jun 15, 2023 at 9:56 AM Syine Mineta @.***> wrote:

Stockfish is a UCI chess engine. I don't see there are major reasons to introduce shared-library-compatible features. People can fork the repository and do their stuffs individually, and I believe this one also falls in such category.

— Reply to this email directly, view it on GitHub https://github.com/official-stockfish/Stockfish/pull/4626#issuecomment-1593420850, or unsubscribe https://github.com/notifications/unsubscribe-auth/AF4K74LQQYOBJWPWJS64OZLXLM5EPANCNFSM6AAAAAAZIDPILM . You are receiving this because you authored the thread.Message ID: @.***>

ab-chesspad avatar Jun 15 '23 20:06 ab-chesspad

you can implement whatever you want in whatever way you want, it just makes no sense to try to PR said changes into SF

PGG106 avatar Jun 15 '23 21:06 PGG106

If it makes sense to allow building SF for various platforms from the same repo, then consider this shared library version as an option to build SF for Android. Starting with Android 10 it's not allowed to use a program as a separate process. I already saw this opposition when I tried to suggest it two years ago. I really do not understand why you people are so against making SF more flexible and convenient?

ab-chesspad avatar Jun 15 '23 21:06 ab-chesspad

one thing is running a CI to build a binary, another is PR-ing a code change, you are really just better off working on this on your own repo, that being said i'm not the maintainer so you might want to wait for him to respond (but i think this is a pretty clear case)

Edit: and even if you want to get merged you didn't do your due diligence, is this change functional? did run an sprt on fishtest? just opening a PR isn't enough

PGG106 avatar Jun 15 '23 21:06 PGG106

Just FYI, the code already contains lots of #if defined(_WIN32) and similar. Thank you Alex Bootman

On Thu, Jun 15, 2023 at 2:10 PM PGG106 @.***> wrote:

one thing is running a CI to build a binary, another is PR-ing a code change, you are really just better off working on this on your own repo, that being said i'm not the maintainer so you might want to wait for him to respond (but i think this is a pretty clear case)

— Reply to this email directly, view it on GitHub https://github.com/official-stockfish/Stockfish/pull/4626#issuecomment-1593728832, or unsubscribe https://github.com/notifications/unsubscribe-auth/AF4K74KT4VH53JDI7KZE4PLXLN25ZANCNFSM6AAAAAAZIDPILM . You are receiving this because you authored the thread.Message ID: @.***>

ab-chesspad avatar Jun 15 '23 21:06 ab-chesspad

not relevant to anything anyone said to you up to now. Good luck with your future endeavors

PGG106 avatar Jun 15 '23 21:06 PGG106

Can you explain more why the current implementation of Stockfish will become incompatible with later versions of Android OS? You'll have a better pitch for updating the implementation of Stockfish to not branch the community of third party applications which rely on Stockfish for Android users.

In my experience, when I've seen users think they need a shared library to make use of Stockfish, they instead settle on wrappers which provide a Stockfish API like https://pypi.python.org/pypi/stockfish or https://github.com/cutechess/cutechess.

RogerThiede avatar Jun 15 '23 22:06 RogerThiede

As I said, you cannot launch a separate process from your own Android app. You may want to look at the older DroidFish build, it shows an example of using SF as a separate process. They abandoned SF support just for that reason. Thank you Alex Bootman

On Thu, Jun 15, 2023 at 3:00 PM Roger Thiede @.***> wrote:

Can you explain more why the current implementation of Stockfish will become incompatible with later versions of Android OS? You'll have a better pitch for updating the implementation of Stockfish to not branch the community of third party applications which rely on Stockfish for Android users.

In my experience, when I've seen users think they need a shared library to make use of Stockfish, they instead settle on wrappers which provide a Stockfish API like https://pypi.python.org/pypi/stockfish or https://github.com/cutechess/cutechess.

— Reply to this email directly, view it on GitHub https://github.com/official-stockfish/Stockfish/pull/4626#issuecomment-1593773207, or unsubscribe https://github.com/notifications/unsubscribe-auth/AF4K74ID4QV7BWX7KFEMMNTXLOAY7ANCNFSM6AAAAAAZIDPILM . You are receiving this because you authored the thread.Message ID: @.***>

ab-chesspad avatar Jun 15 '23 22:06 ab-chesspad

Correction. Seems like SF support in DroidFish is back, but is is removed from Playstore, because it uses SF as a separate process and as I said, starting with Android API 30 it's not legal anymore. It requires permissions that Google Play store does not allow.

On Thu, Jun 15, 2023 at 3:00 PM Roger Thiede @.***> wrote:

Can you explain more why the current implementation of Stockfish will become incompatible with later versions of Android OS? You'll have a better pitch for updating the implementation of Stockfish to not branch the community of third party applications which rely on Stockfish for Android users.

In my experience, when I've seen users think they need a shared library to make use of Stockfish, they instead settle on wrappers which provide a Stockfish API like https://pypi.python.org/pypi/stockfish or https://github.com/cutechess/cutechess.

— Reply to this email directly, view it on GitHub https://github.com/official-stockfish/Stockfish/pull/4626#issuecomment-1593773207, or unsubscribe https://github.com/notifications/unsubscribe-auth/AF4K74ID4QV7BWX7KFEMMNTXLOAY7ANCNFSM6AAAAAAZIDPILM . You are receiving this because you authored the thread.Message ID: @.***>

ab-chesspad avatar Jun 15 '23 23:06 ab-chesspad

so, my 2cents on this discussion. It might be needed to provide SF as a shared library if we want to continue to be available on e.g. the app stores as part of GUIs. There might be other advantages, e.g. in data generation where the UCI overhead is not small, etc.

One could imagine to rewrite SF such that the main + uci functionality is a light wrapper around a library. This is a major project. Technically, I haven't looked at the current PR code. So, I don't know if it goes in the right or the wrong direction. Designing a proper interface for a shared library is something that needs care and thought, and possibly some input by experienced SF devs.

I do not intend to merge something like this in the current development cycle, but having a proposal worked out for the next cycle might be a plan. A decision to integrate can only be made once complete code is available, and as usual will be based on an understanding of the advantages and disadvantages of the actual implementation.

vondele avatar Jun 16 '23 09:06 vondele

Interesting. When two years ago I suggested the whole complete code, the consensus was that my changes are invasive, hard to evaluate and this job should be done in small steps. Now it's just the opposite? Regards Alexander Bootman

On Fri, Jun 16, 2023 at 2:06 AM Joost VandeVondele @.***> wrote:

so, my 2cents on this discussion. It might be needed to provide SF as a shared library if we want to continue to be available on e.g. the app stores as part of GUIs. There might be other advantages, e.g. in data generation where the UCI overhead is not small, etc.

One could imagine to rewrite SF such that the main + uci functionality is a light wrapper around a library. This is a major project. Technically, I haven't looked at the current PR code. So, I don't know if it goes in the right or the wrong direction. Designing a proper interface for a shared library is something that needs care and thought, and possibly some input by experienced SF devs.

I do not intend to merge something like this in the current development cycle, but having a proposal worked out for the next cycle might be a plan. A decision to integrate can only be made once complete code is available, and as usual will be based on an understanding of the advantages and disadvantages of the actual implementation.

— Reply to this email directly, view it on GitHub https://github.com/official-stockfish/Stockfish/pull/4626#issuecomment-1594364947, or unsubscribe https://github.com/notifications/unsubscribe-auth/AF4K74I3H7NG4QCYIMRMO2DXLQOYFANCNFSM6AAAAAAZIDPILM . You are receiving this because you authored the thread.Message ID: @.***>

ab-chesspad avatar Jun 16 '23 14:06 ab-chesspad

BTW, this specific change is an attempt to separate "user interface" (reading the commands from cin) from the actual SF logic. I believe that regardless of other changes it is a good thing. Regards Alexander Bootman

On Fri, Jun 16, 2023 at 2:06 AM Joost VandeVondele @.***> wrote:

so, my 2cents on this discussion. It might be needed to provide SF as a shared library if we want to continue to be available on e.g. the app stores as part of GUIs. There might be other advantages, e.g. in data generation where the UCI overhead is not small, etc.

One could imagine to rewrite SF such that the main + uci functionality is a light wrapper around a library. This is a major project. Technically, I haven't looked at the current PR code. So, I don't know if it goes in the right or the wrong direction. Designing a proper interface for a shared library is something that needs care and thought, and possibly some input by experienced SF devs.

I do not intend to merge something like this in the current development cycle, but having a proposal worked out for the next cycle might be a plan. A decision to integrate can only be made once complete code is available, and as usual will be based on an understanding of the advantages and disadvantages of the actual implementation.

— Reply to this email directly, view it on GitHub https://github.com/official-stockfish/Stockfish/pull/4626#issuecomment-1594364947, or unsubscribe https://github.com/notifications/unsubscribe-auth/AF4K74I3H7NG4QCYIMRMO2DXLQOYFANCNFSM6AAAAAAZIDPILM . You are receiving this because you authored the thread.Message ID: @.***>

ab-chesspad avatar Jun 16 '23 14:06 ab-chesspad

I think it's an interesting idea to factor out the core Stockfish functionality as a library and provide the Stockfish binary as a thin wrapper around it, as @vondele alludes to. To do it "correctly" in order that other projects (including "Stockfish official" projects) can make use of it is no small feat. Some things that come to mind:

  1. we desparately need to move to a more professional build system (e.g. CMake), to make some stuff a lot easier and prevents lots of boilerplate code
  2. we'd better use semantic versioning for the library part (major.minor.patch) to make it clear if we break the API at some point.
  3. what functionality do we want to expose? I would guess not every single function/class in the Stockfish namespace. This would imply separating public/private headers.
  4. Do we provide a C compatible interface? If not, (e.g. C++11 only) life would be a lot easier I think.

I think 1. and 2. are a must, 3. and 4. "nice to haves" (e.g. just make all the headers files public for 3.)

ddobbelaere avatar Jun 16 '23 18:06 ddobbelaere

Just to leave a note. I think this approach is a good choice. It would keep the communication simple and standard. It shouldn't be problematic to use, and we can always add some higher level wrappers.

Ideally the interface should use C ABI though. They should also be explicitly exported, as on some platforms no functions are exported by default.

Sopel97 avatar Jun 21 '23 09:06 Sopel97

Please note that ABI will conflict with UCI. One of my goals is to maintain it.

ab-chesspad avatar Jun 30 '23 13:06 ab-chesspad

Why would adopting a C ABI conflict with using UCI? We could easily have an application that uses the shared library via C ABI and consumes UCI text via standard input, sockets, or other communication channel and outputs valid UCI text.

RogerThiede avatar Jul 01 '23 00:07 RogerThiede

I think it will be better if the library itself can be used with UCI without additional layers. Especially when its output is in the text form, so text in - text out.

ab-chesspad avatar Jul 02 '23 01:07 ab-chesspad

I think what @vondele suggest is excellent design. SF itself could be reduced to a library, without UCI. And UCI would be an independent program (which can be a simple python script, or even written in a different compiled language such as Rust). This opens the door to implement CECP (for those who care - I don't - but I know they still exist), without soiling the SF code base. And of course to interact more directly with SF ABI without paying for the cost of UCI (pipe I/O and task switching).

lucasart avatar Jul 08 '23 07:07 lucasart

I agree on the following two points:

  1. making SF a library will have enormous benefit to the project, for foreseeable applications (android or other mobile apps), and for unforeseeable applications. We never know what is lost by failing to be as flexible as a library is.

  2. UCI should not be part of the core library. The main SF project should of course include the UCI interface that uses the core library, but the library, the engine, should always be separate from any interfaces (including the standard UCI interface).

dubslow avatar Dec 10 '23 17:12 dubslow