Skip to content
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

update star/PP calculations for Feb 2019 update #39

Open
llllllllll opened this issue Mar 25, 2019 · 5 comments
Open

update star/PP calculations for Feb 2019 update #39

llllllllll opened this issue Mar 25, 2019 · 5 comments
Labels
PP calculation Performance Points and Star calculation

Comments

@llllllllll
Copy link
Owner

The PP and star calculations were updated in February 2019. Slider needs to be updated to account for these changes.

source: https://osu.ppy.sh/home/news/2019-02-05-new-changes-to-star-rating-performance-points

@llllllllll llllllllll added the PP calculation Performance Points and Star calculation label Mar 25, 2019
@blimmo
Copy link
Collaborator

blimmo commented Mar 25, 2019

Would it perhaps be a good idea to use pyttanko here instead?

It seems to get updated after changes pretty quickly so would stop us having to write them into slider.

I'll happily write a PR for it if you think that's a good idea.

@llllllllll
Copy link
Owner Author

I hadn't seen that library. After giving it a quick glance, I would prefer to leave our implementation, but maybe add tests to compare the results with this. That code doesn't really appear to be designed for use as a library. There isn't a way to specify a beatmap as an in-memory object, you need to pass the beatmap file as a file-like object. Slider doesn't always have access to a beatmap file as text, for example when we download a new beatmap without save=True, so we couldn't call into pyttanko there.

The other main issue is that it is just a bunch of pure Python iterative code without any use of numpy. slider was designed to support https://github.com/llllllllll/lain and other medium to large scale processing tasks. While our parser is currently very simple and slow (and should be replaced), the PP calculations are pretty fast. Importantly, it is also fast/convenient to compute the performance points for many different results at the same time. For example https://github.com/llllllllll/lain/blob/master/lain/error_model.py#L699-L751 generates 1000 candidate plays by drawing from a probability distribution of results on each hit object. Then, each length 1000 vector of 300, 100, 50 counts is passed to the performance_points method which results in a length 1000 vector of the corresponding PP for each play. Importantly, this isn't just looping over the PP function 1000 times with different parameters, it will re-use any common work (like star calculation) or sub-computation that is constant with respect to the actual counts.

@blimmo
Copy link
Collaborator

blimmo commented Mar 25, 2019

Good point wrt the numpy usage but I think you could pass a list of line strings (so just the contents of the files .split('\n')ed) instead of a file object since it doesn't call open.

Also looking at its tests it looks like you can reuse beatmap data calculation like stars etc. by reusing the beatmap and stars objects so if we cached the pyttanko beatmap and stars objects in our beatmap object we'd only have to calculate that once.

The library is written kinda weirdly though (because the author mostly programs in C i think) so it's probably fair enough to try to keep the current pp implementation in slider especially if we can get it optimised with numpy.

@jxu
Copy link
Contributor

jxu commented Aug 26, 2019

It should be documented which version of PP update is used, since I can't keep track of all of them.
I support the idea of leaving PP calculation to a more frequently updated library but if it doesn't integrate well we should keep this one.

@jxu
Copy link
Contributor

jxu commented Oct 8, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
PP calculation Performance Points and Star calculation
Projects
None yet
Development

No branches or pull requests

3 participants