-
Notifications
You must be signed in to change notification settings - Fork 68
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Movelist / Tree View now includes color highlights for inaccuracy/mistake/blunder #237
base: master
Are you sure you want to change the base?
Movelist / Tree View now includes color highlights for inaccuracy/mistake/blunder #237
Conversation
29ddcc8
to
ba64003
Compare
FYI I've tested this out quite a bit locally, and stomped out most of the bugs. Let me know if you have any other questions! |
Thanks for your work on this! I've read in #235 that it already works for you - so is it good to go to simply clone your branch and follow the install instructions (Windows 11)? And it supports Stockfish, correct? |
Yes.
I didn't touch anything Leela/Stockfish specific, so hopefully it does. Give it a try and let us know! |
Works great! Tested Stockfish. -- Few things:
|
Would you mind sharing the PGN file you're seeing issues with? I can take a look and see what's happening. I assume you're using this button For reference: Auto-eval is working for me, here's a screenshot run using https://www.dropbox.com/scl/fi/qpzh38axktnof6e3clup3/nibbler-2.4.dev-windows.zip?rlkey=wzd7i1fajocd54lx1mtehd0ro This is the PGN file from the screenshot
I have some ideas about how to do this, but for now let's focus on making sure auto-eval is working for you. We can revisit the one-side visualizations later. |
Sorry -- correct PGN I changed all settings to yours and now it works - the one at which it worked is 1M -> 10M nodes (it was also the last thing that didn't match yours, all else failed), so maybe because it's too fast (I set to 16 cores and my CPU is very vast, it passes several moves per second). Also,
(Side question: any docs on Nibbler? I can't tell what many of the options do) |
Oh, thanks for catching that! I probably need to add a line or two to nibbler/files/src/renderer/51_node.js Line 479 in 004e66b
Let me take a look at that.
Not sure, but @rooklift I'm happy to either start some docs or refresh anything that might already exist. Happy to contribute some cleanup here and there or expand any existing documentation if helpful. @OverLordGoldDragon in the meantime, if there are any specific options you'd like to learn more about I can probably read the source code and give you an answer 🙂 |
Well in our "quest" to mimic Lichess I think it crucial to replicate the evaluation graph - @ rooklift's given me a pointer but I've not had a chance to dig into it. I've also noticed @ rooklift's comment on another thread which I think belongs in README - i.e. if using classical engines, ensure |
…l analysis" As suggested by rooklift#237 (comment)
Discussion, continuing from #242 (comment) and building on #242 (comment)
ConclusionThe suggestions in #242 (comment) were to use 0.25 & 0.5, but they would probably be too small in practice.
Maybe:
within the stipulation that all evals larger than ±2.5 are to be considered virtually the same. |
… only `dom_from_scratch`)
instead. (It seems the node.table.eval is updated after this.tree.prev() / this.tree.next() is called when either forward or backward Auto-eval are running)
(yellow and orange are a bit too close for colorblind/accessibility)
…l analysis" As suggested by rooklift#237 (comment)
…m −1.0 to +1.0 (whereas we compare percentage ranging from 0.0 to 1.0) https://github.com/abebinder/lila/blob/77fe2205e30ab17ff8e028d949d2db83e41cc210/modules/analyse/src/main/Advice.scala#L57
35f7c68
to
bfe200b
Compare
Rebased onto latest master, and updated after #244 For posterity, here's how the other platforms handle it: chess.comNote that chess.com also has a "Missed Win" annotation that is more severe than "Blunder". LichessThe Lichess rules are slightly different, and are based instead on win percentage change (±0.1, ±0.2, and ±0.3/0.5 — NOTE: Lichess defines cpWinningChances ranging from −1.0 to +1.0). Longer discussion is here #242 (comment) |
bfe200b
to
7887349
Compare
* `table`.eval replaced with `SortedMoveInfoFromTable(table)[0].cp` * blunder/mistake/inaccuracy defined in terms of centipawns, with the stipulation that all evals larger than ±2.5 are to be considered virtually the same.
7887349
to
f7ca196
Compare
aa536db
to
ddcc292
Compare
…han ±2.5 are to be considered virtually the same"
September 25, 2023:
|
Help to reduce merge conflicts w/ rooklift#237
57a1f22
to
8941e30
Compare
Help to reduce merge conflicts w/ rooklift#237
8941e30
to
1342d2c
Compare
Reduce potential merge conflicts w/ rooklift#248
1342d2c
to
1b41cbf
Compare
Help to reduce merge conflicts w/ rooklift#237
678b9f2
to
c6129b1
Compare
(Also fix a couple bugs where blunders might not display, or inaccuracy would be shown for good moves)
c6129b1
to
c3a98bb
Compare
a49514c
to
f17dc81
Compare
Component
Movelist / Tree View
Implemented in this Pull Request
Some basic color coding for inaccuracies vs. mistakes vs. blunders.
This pull request defines inaccuracy/mistake/blunder based on #242 (comment) → #237 (comment) so that both Stockfish & Lc0 evals can be usable by default in Nibbler for analysis of human games.
Testing