jordanbray
Implement all other Pull Requests
Hello everyone curious enough to visit this unmaintained implementation of chess in rust. I have taken it upon myself to fork this repo and maintain it. If you would like to contribute, please create a pull request at https://github.com/Orkking2/chess/pulls, unlike jordanbray, I have not been inactive since 2022 😅. If you'd like to see what changes I have implemented, feel free to peruse this pull request, as it is not likely to be accepted.
Wow, this is a lot of code.
Let me know when it is ready for review/merge.
aside from some silly changes to the links in the toml file (given you're still alive) it should be ready 🫡
Yeah, I am alive. I had a baby, which prevented me from working on this. I always intended to start it back up, but family comes first.
That said, I'm past the tiny baby years, and have more time to actually spend on hobbies like this now.
I've glanced at this, and I saw a few minor nitpicks. I'm not convinced that STARTPOS constant is the best way, I need to think on that. The hash being hardcoded looks like a terrifying bug in the future. I also saw the loop in the bitboard from_array function, which should be a for loop. (Pro tip: you can avoid the if i == 8 branch by doing the shift first, then the or.)
And, most importantly, I want to benchmark it.
STARTPOS was ripped from another pr, so I can't take credit for the idea or the buggy implementation (same with BitBoard::from_array, which originally used const_for which is literally just a macro that generates loops). I suspect that it can be generated with a LazyLock, like I've done with rook_directions and bishop_directions in magic_helpers.rs, to avoid the const requirements while still only requiring it to be generated once. This would also allow for the complete removal of the BitBoard::from_array function, which exists only to generate STARTPOS.
Note that you cannot use for loops in const functions, as they rely on iterators.
P.S. Congrats on the baby! I am in college so I've got plenty of free time on my hands to work on these things 😝
P.P.S. I wanted to benchmark it too but I don't have much experience, being a 20 yr old in college, so I haven't.
This is a lot of work for a college student! It all looks great. It looks like the community has really stepped up to the plate to make this library great, and you have put in a lot of work to merge all their ideas together. I greatly appreciate all of that effort, by you and by everyone else.
You may be right about the loop now that you say that. (Honestly, given that's the case, should the loop just be completely unrolled? It's like 8 lines of code then.)
I am not a 20 year old, I am an old man now (36!), so what I lack in energy, I can make up for with experience working on large-scale rust projects in production codebases.
I have started the review process. I will allocate some time on Sunday to complete the review process, but these are some of my initial thoughts on what needs to be done.
If we're gonna bump the version to 4.0.0, let's do it right!
Coding is a hobby of mine and in my pursuit of re-creating AlphaZero I stumbled across this crate and decided it needed some loving.
I might be a little over-eager in my updating but I would appreciate any and all lessons you can teach me about writing pull requests and helping improve open-source projects !!
Hey, maybe #48 #78 and #84 could be reviewed and then rebase this if they get merged so the complexity of this PR is reduced? Not sure if the creators of these PRs are active to address feedback in any case
So, in this particular case, I prefer to review this as one giant change.
Under normal circumstances, you really want to minimize the lines of code. A feature should normally be written in the minimum number of lines that you can get away with.
This time, though, when the crate has been effectively abandoned, and a bunch of community members making their own versions, I think it's best to review all the changes together, because it lets me see how everyone's code interacts. And, you've already taken the time to put this together, so you've done 80% of the hard work for me.
I'll allocate some time to help clean this up too.
All that said, because it's such a big change, it takes time to review. It's easy to get sloppy when you have thousand line merges ("Ehh, it's fine. It's just one function"), and you have to resist the urge to accept things that you would reject if it were a smaller change.
I will not merge this before I think it's ready. I will merge this, though. On the surface, it looks like the community has done good work, and I've been a bad maintainer. I will fix that now that I have more free time, and now that you've already done a lot of the leg-work.
Ok, I've been running some benchmarks.
Commit 0b0f7f02d1bd9d9a4eda672bc61cd020543ba047 is faster than anything after. This appears to be when 3.2.0 was officially deployed.
I'm not 100% sure what the holdup is. But, this branch is also slower than main. When all is said and done, perft_01 runs in 3.9s on my machine, whereas the above commit runs in 2.9s.
This library is currently losing to cozychess. We are beating shakmaty and chessie in my preferred benchmark.
I am getting the chess_perft crate in a format where you can run it yourself. I will update this comment in a sec.
So, to run the benchmarks:
git checkout ssh://[email protected]/jordanbray/chess_perft.git
# make sure ../chess is your copy of `chess`, the Cargo.toml depends on the path for the moment. I'll fix that later.
cargo bench
I have also created a branch called bigolupdate which contains all of this data, as well as some of my own updates. That is to avoid clobbering your history.
I got the following error:
git checkout ssh://[email protected]/jordanbray/chess_perft.git
error: pathspec 'ssh://[email protected]/jordanbray/chess_perft.git' did not match any file(s) known to git
Running cargo bench on https://github.com/jordanbray/chess_perft/pull/3 demonstrates that this update is faster than the original (generally) when run with no_std.
I have created a new branch, called fastest, where I'm slowly merging many of the ideas here. The problem with this update is simply that it's slow, and I couldn't find out where the slowness is coming from.
However, the fastest branch is now "completely safe" (by that I mean, I don't think it's possible for a user to cause a safety violation without intentionally calling an unsafe function), and the fastest branch is the fastest move generator in the rust ecosystem.