Increase performance by saving the result of pos.value(move) instead of computing it multiple times

[Recursing]

pos.value(move) gets called with the same arguments three times on lines 274, 276 and 278
By just making making pos.gen_moves() return a list of tuples of the form (value, move) and making pos.move take move_value as an argument I think I obtained roughly a 35% speed improvement (with NODES_SEARCHED = 1e5, on pypy on my computer it goes from 2m24.644s to 1m35.306s for 4 moves)

I don't think this is the best way to do it, but the current implementation seems very inefficient.

Also, are there plans to make the position mutable, or do you think it would increase complexity too much?

I understand that the main focus of this project is to keep it simple and short, but it looks like it would play reasonably well with just a ~10x performance improvement, that seems reachable (at least, I think so)

[thomasahle]

This sounds like an interesting improvement, that probably wouldnt increase complexity too much. If you want to, you can also test the improvement by letting the new and old versions play each other. The test.py script allows you to do this easily, with the arena option.

There are no immediate plans to make position mutable. At least I dont know a way to do this, that wouldnt also require an unmove method, which would easily take 10-15 lines. Also Im wondering how much it would help, since the expensive operations (string copying) are done in C rather than Python, whereas with mutable code everything would be in Python. Its worth trying though. Say it wins enough that we can remove the value method and just reevaluate the board after each move, then perhaps it wins the lines back.

[thomasahle]

@Recursing Did you have any luck with either of these improvements? As mentioned, it is easy to test whether a change is an improvement or not, by saving the new version as sunfish_new.py and running python3 test.py arena --games=4000 --seconds=60 --plus=0 sunfish sunfish_new.

[Recursing]

Hi! I didn't look into it in the past year, but if that function is still called so many times with the same arguments I'm sure there's still room for simple improvement there. I just didn't know the best way to do it. Maybe in the next few days I'll be able to look into it

…

On Aug 25, 2017 14:27, "Thomas Dybdahl Ahle" ***@***.***> wrote: @Recursing <https://github.com/recursing> Did you have any luck with either of these improvements? As mentioned, it is easy to test whether a change is an improvement or not, by saving the new version as sunfish_new.py and running python3 test.py arena --games=4000 --seconds=60 --plus=0 sunfish sunfish_new. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#39 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AGp6mF-Yb4VABpDTn2c6j_ov-wlHLPfsks5sbr28gaJpZM4Jj5pw> .

[thomasahle]

Something like

for score, move in sorted((pos.value(m), m) for m in pos.gen_moves(), reverse=True):
    if depth > 0 or score >= QS_LIMIT:
        yield move, -self.bound(pos.move(move, score), 1-gamma, depth-1, root=False)

could work (where pos.move is modified to take an optional pre-computed score value.)

A more involved approach would be to have gen_moves also generate the score. That might be able to save some lines as well, by removing the entire value method.

[thomasahle]

This is now implemented.
Except that pos.move still calls pos.value again.
I'll leave it open in case I try that optimization too.