osuAkatsuki / bancho.py

An osu! server for the generic public, optimized for maintainability in modern python
https://akatsuki.gg
MIT License
212 stars 127 forks source link

[BUG] Overall weighted acc calculation returning wrong values #299

Closed def750 closed 7 months ago

def750 commented 1 year ago

Since some time we've been experiencing weird issues with weighted acc calculation, we've had some users that were aiming for profile acc of 100%. But after some time acc went down even though they had only SS ranks. Here's one of our users, his scores and output of acc calculation https://pastebin.com/yYEG1H1Y

I used bancho.py's acc calculation method and fetched scores where status=2 in mode=4

def750 commented 1 year ago

I also have no idea if there's simillar or same issue with pp calculation

minisbett commented 1 year ago

I don't know if this is related and how it is handled on bpy but a lot of people think that only the top 100 plays are taken for the account acc and overall pp when actually it takes all your passes ever submitted, the weight just gets insanely small

def750 commented 1 year ago

After further testing I've found the issue

I don't know if this is related and how it is handled on bpy but a lot of people think that only the top 100 plays are taken for the account acc and overall pp when actually it takes all your passes ever submitted, the weight just gets insanely small

Yeah I was about to write that, after further investigation I found that it's taking only top 100 scores into the calculation but it's also taking length of all scores where status=2

For example user x has 380 bests on approveds and rankeds, Server while calculating new pp and acc, takes his top 100 scores into calculation, so the total weight (i have no idea how to call it) is 100, but instead of putting value <=100 it's taking len(scores) of user which most likely will be longer than that I've tried switching it to len(top_100pp) and it worked perfectly

Here's the line of code I'm talking about https://github.com/osuAkatsuki/bancho.py/blob/master/app/api/domains/osu.py#L1049

def750 commented 1 year ago

If I'm wrong then it's still weird because like I mentioned, user above literally has no bests where accuracy is lower than 100%, and it still was 99.42

minisbett commented 1 year ago

So there two separate things going on, first being a somehow wrong acc calculation and second the fact that bpy does not weight the plays properly?

def750 commented 1 year ago

So there two separate things going on, first being a somehow wrong acc calculation and second the fact that bpy does not weight the plays properly?

I think that the calculation is ok, but it just doesn't weight properly, I'm literally guessing but after changing it it seemed to work

tsunyoku commented 1 year ago

see https://github.com/ppy/osu-queue-mania-key-rank-processor/blob/master/osu.Server.Queues.ManiaKeyRankingProcessor/ManiaKeyRankingProcessor.cs#L97 (scores are fetched @ https://github.com/ppy/osu-queue-mania-key-rank-processor/blob/master/osu.Server.Queues.ManiaKeyRankingProcessor/ManiaKeyRankingProcessor.cs#L46, there is no limit in the query)

this is how osu! does it; they do not limit to the top 100 scores. unless the calculations itself are wrong, the reason you are not getting the 100% you expect would be due to non-100% accuracy scores outside of their top 100.

def750 commented 1 year ago

see https://github.com/ppy/osu-queue-mania-key-rank-processor/blob/master/osu.Server.Queues.ManiaKeyRankingProcessor/ManiaKeyRankingProcessor.cs#L97

this is how osu! does it; they do not limit to the top 100 scores. unless the calculations itself are wrong, the reason you are not getting the 100% you expect would be due to non-100% accuracy scores outside of their top 100.

I've literally fetched all of their scores with no limit 🤨 image image and those are in any mode

def750 commented 1 year ago

it got truncated for some reason image

tsunyoku commented 1 year ago

I've literally fetched all of their scores with no limit 🤨

you said:

I've tried switching it to len(top_100pp) and it worked perfectly

it's not supposed to only fetch their top 100/check the length of that. it is meant to be use the length of your entire score list. you're conflicting yourself

def750 commented 1 year ago

I've literally fetched all of their scores with no limit 🤨

you said:

I've tried switching it to len(top_100pp) and it worked perfectly

it's not supposed to only fetch their top 100/check the length of that. it is meant to be use the length of your entire score list. you're conflicting yourself

I did that later, first I've tried fetching scores of few user with that problem, then I switched to that and it worked for some reason, I shouldn't be saying that Ii'm doing something better but it just started working as supposed, because well, Having profile acc under 100% with only ss ranks isn't expected behaviour

7ez commented 1 year ago

it got truncated for some reason image

i hope you do realise that mode 4 is rx!std and 3 is vn!mania

def750 commented 1 year ago

it got truncated for some reason image

i hope you do realise that mode 4 is rx!std and 3 is vn!mania

Yes I realized that

tsunyoku commented 1 year ago

i wonder if https://github.com/osuAkatsuki/bancho.py/blob/master/app/api/domains/osu.py#L1053 using len(top_100_pp) while https://github.com/osuAkatsuki/bancho.py/blob/master/app/api/domains/osu.py#L1056 uses len(total_scores) is an issue. if you change line 1053 to use len(total_scores) instead, does this issue still occur?

def750 commented 1 year ago

i wonder if https://github.com/osuAkatsuki/bancho.py/blob/master/app/api/domains/osu.py#L1053 using len(top_100_pp) while https://github.com/osuAkatsuki/bancho.py/blob/master/app/api/domains/osu.py#L1056 uses len(total_scores) is an issue. if you change line 1053 to use len(total_scores) instead, does this issue still occur?

After changing that to top_100_pp output acc was exactly 99.99999999999996

tsunyoku commented 1 year ago

After changing that to top_100_pp output acc was exactly 99.99999999999996

i want you to do the opposite. i want you to use len(total_scores) for both acc calculations (since one uses top 100 while the other doesn't, currently) since that is what osu does. this should hopefully get us closer if not 100% accurate.

def750 commented 1 year ago

After changing that to top_100_pp output acc was exactly 99.99999999999996

i want you to do the opposite. i want you to use len(total_scores) for both acc calculations (since one uses top 100 while the other doesn't, currently) since that is what osu does. this should hopefully get us closer if not 100% accurate.

Here's the result image

And here's the result of my ver image

tsunyoku commented 1 year ago

can you send me a json with all of their {pp: float, acc: float} scores alongside a score count? i will see what i can find

def750 commented 1 year ago

can you send me a json with all of their {pp: float, acc: float} scores alongside a score count? i will see what i can find

{'pp': 118.492, 'acc': 100.0} {'pp': 101.229, 'acc': 100.0} {'pp': 97.136, 'acc': 100.0} {'pp': 93.595, 'acc': 100.0} {'pp': 91.544, 'acc': 100.0} {'pp': 83.594, 'acc': 100.0} {'pp': 81.067, 'acc': 100.0} {'pp': 80.687, 'acc': 100.0} {'pp': 80.396, 'acc': 100.0} {'pp': 80.103, 'acc': 100.0} {'pp': 79.76, 'acc': 100.0} {'pp': 78.65, 'acc': 100.0} {'pp': 77.692, 'acc': 100.0} {'pp': 75.713, 'acc': 100.0} {'pp': 74.259, 'acc': 100.0} {'pp': 73.791, 'acc': 100.0} {'pp': 73.459, 'acc': 100.0} {'pp': 73.336, 'acc': 100.0} {'pp': 72.062, 'acc': 100.0} {'pp': 71.875, 'acc': 100.0} {'pp': 71.528, 'acc': 100.0} {'pp': 70.38, 'acc': 100.0} {'pp': 68.588, 'acc': 100.0} {'pp': 67.727, 'acc': 100.0} {'pp': 66.573, 'acc': 100.0} {'pp': 66.358, 'acc': 100.0} {'pp': 65.295, 'acc': 100.0} {'pp': 65.225, 'acc': 100.0} {'pp': 64.835, 'acc': 100.0} {'pp': 64.161, 'acc': 100.0} {'pp': 62.967, 'acc': 100.0} {'pp': 62.9, 'acc': 100.0} {'pp': 62.689, 'acc': 100.0} {'pp': 62.521, 'acc': 100.0} {'pp': 62.35, 'acc': 100.0} {'pp': 61.539, 'acc': 100.0} {'pp': 61.182, 'acc': 100.0} {'pp': 61.138, 'acc': 100.0} {'pp': 60.619, 'acc': 100.0} {'pp': 60.288, 'acc': 100.0} {'pp': 60.126, 'acc': 100.0} {'pp': 60.079, 'acc': 100.0} {'pp': 59.798, 'acc': 100.0} {'pp': 59.533, 'acc': 100.0} {'pp': 59.181, 'acc': 100.0} {'pp': 57.492, 'acc': 100.0} {'pp': 57.457, 'acc': 100.0} {'pp': 57.011, 'acc': 100.0} {'pp': 56.847, 'acc': 100.0} {'pp': 56.089, 'acc': 100.0} {'pp': 55.951, 'acc': 100.0} {'pp': 55.941, 'acc': 100.0} {'pp': 55.288, 'acc': 100.0} {'pp': 54.511, 'acc': 100.0} {'pp': 54.352, 'acc': 100.0} {'pp': 53.946, 'acc': 100.0} {'pp': 53.932, 'acc': 100.0} {'pp': 52.516, 'acc': 100.0} {'pp': 52.387, 'acc': 100.0} {'pp': 52.374, 'acc': 100.0} {'pp': 52.261, 'acc': 100.0} {'pp': 51.627, 'acc': 100.0} {'pp': 51.347, 'acc': 100.0} {'pp': 51.275, 'acc': 100.0} {'pp': 51.261, 'acc': 100.0} {'pp': 50.528, 'acc': 100.0} {'pp': 49.703, 'acc': 100.0} {'pp': 49.604, 'acc': 100.0} {'pp': 49.373, 'acc': 100.0} {'pp': 49.018, 'acc': 100.0} {'pp': 48.59, 'acc': 100.0} {'pp': 48.411, 'acc': 100.0} {'pp': 47.478, 'acc': 100.0} {'pp': 47.4, 'acc': 100.0} {'pp': 47.053, 'acc': 100.0} {'pp': 46.686, 'acc': 100.0} {'pp': 46.489, 'acc': 100.0} {'pp': 46.486, 'acc': 100.0} {'pp': 45.894, 'acc': 100.0} {'pp': 45.706, 'acc': 100.0} {'pp': 45.286, 'acc': 100.0} {'pp': 44.364, 'acc': 100.0} {'pp': 44.279, 'acc': 100.0} {'pp': 43.786, 'acc': 100.0} {'pp': 43.598, 'acc': 100.0} {'pp': 43.479, 'acc': 100.0} {'pp': 42.539, 'acc': 100.0} {'pp': 42.366, 'acc': 100.0} {'pp': 42.25, 'acc': 100.0} {'pp': 42.174, 'acc': 100.0} {'pp': 41.874, 'acc': 100.0} {'pp': 41.401, 'acc': 100.0} {'pp': 40.43, 'acc': 100.0} {'pp': 38.93, 'acc': 100.0} {'pp': 38.59, 'acc': 100.0} {'pp': 38.248, 'acc': 100.0} {'pp': 38.234, 'acc': 100.0} {'pp': 37.126, 'acc': 100.0} {'pp': 35.887, 'acc': 100.0} {'pp': 35.778, 'acc': 100.0} {'pp': 35.397, 'acc': 100.0} {'pp': 35.247, 'acc': 100.0} {'pp': 35.154, 'acc': 100.0} {'pp': 35.11, 'acc': 100.0} {'pp': 34.781, 'acc': 100.0} {'pp': 33.91, 'acc': 100.0} {'pp': 33.881, 'acc': 100.0} {'pp': 33.667, 'acc': 100.0} {'pp': 33.201, 'acc': 100.0} {'pp': 33.03, 'acc': 100.0} {'pp': 32.865, 'acc': 100.0} {'pp': 31.91, 'acc': 100.0} {'pp': 31.259, 'acc': 100.0} {'pp': 31.243, 'acc': 100.0} {'pp': 30.985, 'acc': 100.0} {'pp': 30.965, 'acc': 100.0} {'pp': 30.755, 'acc': 100.0} {'pp': 30.726, 'acc': 100.0} {'pp': 30.318, 'acc': 100.0} {'pp': 30.072, 'acc': 100.0} {'pp': 29.658, 'acc': 100.0} {'pp': 29.636, 'acc': 100.0} {'pp': 29.263, 'acc': 100.0} {'pp': 29.009, 'acc': 100.0} {'pp': 28.51, 'acc': 100.0} {'pp': 28.438, 'acc': 100.0} {'pp': 28.355, 'acc': 100.0} {'pp': 28.333, 'acc': 100.0} {'pp': 28.215, 'acc': 100.0} {'pp': 27.997, 'acc': 100.0} {'pp': 27.626, 'acc': 100.0} {'pp': 26.77, 'acc': 100.0} {'pp': 26.289, 'acc': 100.0} {'pp': 26.18, 'acc': 100.0} {'pp': 26.117, 'acc': 100.0} {'pp': 25.827, 'acc': 100.0} {'pp': 25.6, 'acc': 100.0} {'pp': 25.598, 'acc': 100.0} {'pp': 25.393, 'acc': 100.0} {'pp': 25.297, 'acc': 100.0} {'pp': 25.253, 'acc': 100.0} {'pp': 24.012, 'acc': 100.0} {'pp': 23.781, 'acc': 100.0} {'pp': 23.567, 'acc': 100.0} {'pp': 23.0, 'acc': 100.0} {'pp': 22.954, 'acc': 100.0} {'pp': 22.008, 'acc': 100.0} {'pp': 21.936, 'acc': 100.0} {'pp': 21.719, 'acc': 100.0} {'pp': 21.415, 'acc': 100.0} {'pp': 21.413, 'acc': 100.0} {'pp': 20.748, 'acc': 100.0} {'pp': 20.687, 'acc': 100.0} {'pp': 20.087, 'acc': 100.0} {'pp': 19.779, 'acc': 100.0} {'pp': 19.174, 'acc': 100.0} {'pp': 19.164, 'acc': 100.0} {'pp': 18.97, 'acc': 100.0} {'pp': 18.289, 'acc': 100.0} {'pp': 17.999, 'acc': 100.0} {'pp': 17.054, 'acc': 100.0} {'pp': 16.991, 'acc': 100.0} {'pp': 16.412, 'acc': 100.0} {'pp': 16.227, 'acc': 100.0} {'pp': 14.877, 'acc': 100.0} {'pp': 14.498, 'acc': 100.0} {'pp': 13.955, 'acc': 100.0} {'pp': 12.372, 'acc': 100.0} {'pp': 12.16, 'acc': 100.0} {'pp': 6.777, 'acc': 100.0} {'pp': 6.343, 'acc': 100.0} {'pp': 0.097, 'acc': 100.0} image with 172 scores

minisbett commented 1 year ago

https://github.com/osuAkatsuki/bancho.py/blob/master/app/api/domains/osu.py#L1050

pretty sure this line has to be removed so it no longer only includes the top 100 plays

tsunyoku commented 1 year ago

image

my suspicions were correct:

i want you to use len(total_scores) for both acc calculations (since one uses top 100 while the other doesn't, currently) since that is what osu does. this should hopefully get us closer if not 100% accurate.

fixes it.

cmyui commented 1 year ago

Hey @tsunyoku @def750 do you know if we fixed this and forgot to close this issue?

cmyui commented 1 year ago

looking at git blame, I don't see any recent updates, so I believe not https://github.com/osuAkatsuki/bancho.py/blame/master/app/api/domains/osu.py#L1074-L1083

tsunyoku commented 1 year ago

i'll pr the fix today, it's a 1 line change

skrungly commented 7 months ago

it looks like tools/recalc.py didn't get updated to account for this:

https://github.com/osuAkatsuki/bancho.py/blob/eb62357535be6c33d4aec1bafc9a3339456497c4/tools/recalc.py#L136-L148

i'll submit a quick fix!