-
Notifications
You must be signed in to change notification settings - Fork 21
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
Pet Priority #91
Pet Priority #91
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Overall looks good to me. I will checkout the new numpy random generators when I have more time later this week.
The performance issue I noticed with previous RandomState was team creation. This is obviously an issue because if one wants to run millions of battles or games. An issue can be created to update to the newer Numpy implementation. Then performance can be re-checked against the notes in MockRandomState.
Lines 4 to 14 in ec22d83
class MockRandomState: | |
""" | |
Numpy RandomState is actually extremely slow, requiring about 300 microseconds | |
for any operation involving state. Therefore, when reproducibility is not | |
necessary, this class should be used to immensly improve efficiency. | |
Tests were run for Player: | |
%timeit _ = Player.from_state(pstate) | |
# ORIGINAL: 26.8 ms ± 788 µs per loop | |
# MockRandomState: 1.15 ms ± 107 µs per loop | |
Use of MockRandomState improved the performance by 10x. This is very important. |
…apai into feature/refactor-battle
Refactor update_pet_priority into calculate_pet_priority and rewrite to no longer prioritise health (#89)
The calculate_pet_priority seed uses the new numpy random generators which should improve performance.
Might be worth checking if the performance increase gets it anywhere close to
MockRandomState
's performance.