WIP - initial compiling version. Needs to be thoroughly tested

lichess-org / fishnet

Distributed Stockfish analysis for lichess.org

https://lichess.org/get-fishnet

GNU General Public License v3.0

738 stars 102 forks source link

WIP - initial compiling version. Needs to be thoroughly tested #224

Closed lakinwecker closed 1 year ago

lakinwecker commented 2 years ago

There is still a lot to be done here, I just wanted to make the PR right away. It shows the breadth of the sort of stuff that needs to be done. However, I have lots left to do, like:

[x] Testing with our code base and making sure everything still works for chess related fishnet tasks
[x] Cleaning up the code base (I want to stress this one, I didn't spend a lot of time trying to make it "nice")
[ ] Fuzz testing the rsffish API
[ ] Make sure it's compiling on windows.
[ ] Any feedback you might have after this.

lakinwecker commented 2 years ago

Another thing I'll point out as I work through this more, there are numerous places where fairystockfish uses std::cerr directly and we can't really stop it from messing up the output. I'm currently defaulting to use the shakmaty code paths first, so this won't be a common issue, but thought I would mention it.

lakinwecker commented 2 years ago

Ok, there are some obvious defects in this current pull request. But otherwise, I'm ready for you to take an initial high level look at it and give me feedback. It "works" for us, but I suspect it's not nearly ready to merge.

lakinwecker commented 2 years ago

In addition to using the master branch of FairyStockfish (because flipello is broken in the release that fishnet currently uses), we also do a few other things like just arbitrarily up the timeout for analysis, which needs to be fixed and a number of other things.

Assuming I was to fix all of those and we waited until the FairyStockfish made a new release, and I finished the rest of my TODOs, would this be a candidate for a merge?

lakinwecker commented 2 years ago

FWIW, there is no rush on this. I'm unlikely to have time to do a bunch of extra work on this in the next month or so, so if you're busy then feel free to ignore it for a bit.

I will be picking it back up at some point though.

lakinwecker commented 1 year ago

Closing this because it's way out of date and has diverged significantly from its original intention. If I get to a point where it makes sense to consider merging again, I will re-open a new PR