-
-
Notifications
You must be signed in to change notification settings - Fork 2.3k
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
Add UR estimation to the osu!mania ruleset #22613
Open
Natelytle
wants to merge
98
commits into
ppy:pp-dev
Choose a base branch
from
Natelytle:maniastatacc
base: pp-dev
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
+538
−12
Open
Changes from all commits
Commits
Show all changes
98 commits
Select commit
Hold shift + click to select a range
7882423
add deviation estimation to mania's perfcalc
Natelytle 4af6c26
add UR attribute, change initial min to 300UR
Natelytle 6ea6ba2
bound deviation estimate to numbers greater than 0
Natelytle 3d80168
fix hitwindows
Natelytle f4c7e9d
account for converts, add note counts
Natelytle dfd386c
use onlineid for contingency
Natelytle 5296fb5
fix convert hit windows
Natelytle 562954a
account for HR/EZ in deviation calculation
Natelytle 49495db
use erfc approximation
Natelytle dd1754f
Add rudimentary difficulty scaling curve
Natelytle 79b3541
Change curve
Natelytle 3a51c53
Don't give PP to jittered plays
Natelytle fb7cdc4
merge master and change hit window formulas
Natelytle c5233c7
change hit window formulas
Natelytle 8e78adb
fix judgement
Natelytle 4a5b14f
lazer deviation is finally accurate woo
Natelytle 82bf3e7
increase totalhits by 1
Natelytle 522271b
quick legacy LN idea
Natelytle 3a78b9e
Fix formatting
Natelytle 6f5e03c
change legacy LN calculation
Natelytle c331058
switch to totalvalue
Natelytle bc7fd8d
add comments
Natelytle 80128d2
rewrite everything with erf for simplicity
Natelytle 91fe6a5
change isLegacy toggle
Natelytle 8fb84c2
whop
Natelytle 68cea94
readability bump
Natelytle 4535b29
rename method
Natelytle 7196325
Merge remote-tracking branch 'master/master' into maniastatacc
Natelytle b0cd729
fix convert OD with lazer judgements
Natelytle 2b5ab41
Revert unrelated files changed in formatting commit
Natelytle fde20e1
Add desmos links to likelihood functions
Natelytle 5a0aa24
Code quality overhaul
Natelytle 9ff4c1b
fix const name
Natelytle 01dbce6
More formatting
Natelytle db0fda0
Remove pointless erf approximation, fix lazer return statement
Natelytle 2d7c99b
Massively improve estimation accuracy using a folded distribution
Natelytle 33a73e3
update desmos, remove redundant parenthesis
Natelytle 97da8cc
Reformatting, increase precision a lot by rewriting in terms of the l…
Natelytle 08ceb02
Rename "Judgements" to "HitWindows"
Natelytle 35119bc
Address reviews
Natelytle 06a2358
account for mania tests
Natelytle 11f6fd0
Fix issue with high misscount plays returning 115 UR
Natelytle 15ca535
Fix incorrect logDiff function, make lazer tail multiplier a const
Natelytle 1e272f1
Address abraker's review, add approximation for legacy LN tails
Natelytle bafb8f6
Address abraker's review, add approximation for legacy LN tails
Natelytle 1f76d48
Merge remote-tracking branch 'origin/maniastatacc' into maniastatacc
Natelytle 4a8d679
Fix NaNs (whoops)
Natelytle 3a1c479
Avoid repeated code using structs and methods
Natelytle 66529f6
Add comments
Natelytle bfc642e
Change types, remove pointless duplicate likelihood function
Natelytle ab8d19e
Fix erfc approximation
Natelytle 6399b9b
expose hit windows publicly for testing purposes
Natelytle 32a3878
Add tests
Natelytle d0d0aae
Edit comments
Natelytle 34c2cc0
Add comments and fail messages to UR estimation tests
Natelytle 4affdc6
Change UR scaling curve
Natelytle a10dccf
Revert to live scaling
Natelytle d44d893
Rename to IsConvert, add handling for converts below od4
Natelytle 9de2585
Move comment
Natelytle 6ace77a
Revert back to previous curve, add more tests
Natelytle ec19983
Update desmos
Natelytle d8d4cc7
Change curve to clamp PP to <700ur
Natelytle 18cef8b
Change hit window test to not require a beatmap, test multiple ODs
Natelytle 69b99f6
Change curve (again) to fix jittering edge cases
Natelytle 6e3e522
Merge remote-tracking branch 'osumaster/master' into maniastatacc
Natelytle 0a72493
Band-aid math error on stable LNs, add tail deviation multiplier
Natelytle a58470d
Fix stable LN logic
Natelytle f60317c
Nest Ternary into Math.Log
Eve-ning 951861a
Shorten property fetching code
Eve-ning 20de476
Fix incorrect function name logPNote
Eve-ning 7ed9141
Fix incorrect legacyLogPHold function name
Eve-ning c20e150
Merge master
Natelytle 0ed0915
Collapse boilerplate code to parse judgements
Eve-ning e9b02c2
Remove resource dependency on test
Eve-ning b131ac8
Move test case code closer to test callables
Eve-ning 1d7dd8b
Expose getHitWindows for tests
Eve-ning 515ff55
Narrow scope of hitWindows fns
Eve-ning d00d7fd
Narrow scope of computeEstimatedUr
Eve-ning 3dea273
Update changed values
Eve-ning b2ad8ad
Add remark on null & exception throwable
Eve-ning 35f6ffe
Improve testing
Eve-ning 5864d87
Add comment justifying args on TestEdge
Eve-ning 104eea3
Delete ur-estimation-test.osu
Eve-ning da36c2a
Remove unresolvable cref
Eve-ning dd4a3ec
Add test for More Max Less UR
Eve-ning 9cc6a3a
Switch to using deviation on notes and heads for multiplier scaling
Natelytle 95c7237
Remove IsConvert difficulty attribute
smoogipoo a2197e2
Store difficulty attribute values
smoogipoo a05c3b8
Update ManiaUnstableRateEstimationTest.cs
Eve-ning 4eb307b
Merge pull request #1 from Eve-ning/maniastatacc-improve-test
Natelytle 1e80f79
Fix non-existent attribute call
Natelytle 52799d5
Merge remote-tracking branch 'osumaster/master' into maniastatacc
Natelytle 75c295c
Remove hit windows scaling by rate, adjust tests
Natelytle 5de7e42
Merge branch 'master' into maniastatacc
smoogipoo aa8b9cd
Merge master
Natelytle ad818a4
Increase readability
Natelytle 4f2496f
Merge branch 'ppy:master' into maniastatacc
Natelytle 0f47f46
Merge branch 'master' into maniastatacc
smoogipoo File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
224 changes: 224 additions & 0 deletions
224
osu.Game.Rulesets.Mania.Tests/ManiaUnstableRateEstimationTest.cs
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,224 @@ | ||
// Copyright (c) ppy Pty Ltd <[email protected]>. Licensed under the MIT Licence. | ||
// See the LICENCE file in the repository root for full licence text. | ||
|
||
#nullable disable | ||
|
||
using NUnit.Framework; | ||
using osu.Game.Rulesets.Difficulty; | ||
using osu.Game.Rulesets.Mania.Difficulty; | ||
using osu.Game.Rulesets.Mania.Mods; | ||
using osu.Game.Scoring; | ||
using System.Collections.Generic; | ||
using System.Linq; | ||
using osu.Game.Rulesets.Mania.Scoring; | ||
using osu.Game.Rulesets.Mods; | ||
using osu.Game.Rulesets.Scoring; | ||
|
||
namespace osu.Game.Rulesets.Mania.Tests | ||
{ | ||
/// <summary> | ||
/// This test suite tests ManiaPerformanceCalculator.computeEstimatedUr | ||
/// <remarks> | ||
/// This suite focuses on the objective aspects of the calculation, not the accuracy of the calculation. | ||
/// </remarks> | ||
/// </summary> | ||
public class ManiaUnstableRateEstimationTest | ||
{ | ||
public enum SpeedMod | ||
{ | ||
DoubleTime, | ||
NormalTime, | ||
HalfTime | ||
} | ||
|
||
public static IEnumerable<TestCaseData> TestCaseSourceData() | ||
{ | ||
yield return new TestCaseData(1037.4609375d, new[] { 3, 3, 3, 3, 3, 3 }, SpeedMod.DoubleTime); | ||
yield return new TestCaseData(1037.4609375d, new[] { 3, 3, 3, 3, 3, 3 }, SpeedMod.NormalTime); | ||
yield return new TestCaseData(1037.4609375d, new[] { 3, 3, 3, 3, 3, 3 }, SpeedMod.HalfTime); | ||
} | ||
|
||
/// <summary> | ||
/// A catch-all hardcoded regression test, inclusive of rate changing. | ||
/// </summary> | ||
[TestCaseSource(nameof(TestCaseSourceData))] | ||
public void RegressionTest(double expectedUr, int[] judgementCounts, SpeedMod speedMod) | ||
{ | ||
double? estimatedUr = computeUnstableRate(judgementCounts, speedMod: speedMod); | ||
// Platform-dependent math functions (Pow, Cbrt, Exp, etc) and advanced math functions (Erf, FindMinimum) may result in slight differences. | ||
Assert.That( | ||
estimatedUr, Is.EqualTo(expectedUr).Within(0.001), | ||
$"The estimated mania UR {estimatedUr} differed from the expected value {expectedUr}." | ||
); | ||
} | ||
|
||
/// <summary> | ||
/// Test anomalous judgement counts where NULLs can occur. | ||
/// </summary> | ||
[TestCase(false, new[] { 1, 0, 0, 0, 0, 0 })] | ||
[TestCase(true, new[] { 0, 0, 0, 0, 0, 1 })] | ||
[TestCase(true, new[] { 0, 0, 0, 0, 0, 0 })] | ||
public void TestNull(bool expectedIsNull, int[] judgementCounts) | ||
{ | ||
double? estimatedUr = computeUnstableRate(judgementCounts); | ||
bool isNull = estimatedUr == null; | ||
|
||
Assert.That(isNull, Is.EqualTo(expectedIsNull), $"Estimated mania UR {estimatedUr} was/wasn't null."); | ||
} | ||
|
||
/// <summary> | ||
/// Ensure that the worst case scenarios don't result in unbounded URs. | ||
/// <remarks>Given Int.MaxValue judgements, it can result in | ||
/// <see cref="MathNet.Numerics.Optimization.MaximumIterationsException"/>. | ||
/// However, we'll only test realistic scenarios.</remarks> | ||
/// </summary> | ||
[Test, Combinatorial] | ||
public void TestEdge( | ||
[Values(100_000, 1, 0)] int judgeMax, // We're only interested in the edge judgements. | ||
[Values(100_000, 1, 0)] int judge50, | ||
[Values(100_000, 1, 0)] int judge0, | ||
[Values(SpeedMod.DoubleTime, SpeedMod.HalfTime, SpeedMod.NormalTime)] | ||
SpeedMod speedMod, | ||
[Values(true, false)] bool isHoldsLegacy, | ||
[Values(true, false)] bool isAllHolds, // This will determine if we use all holds or all notes. | ||
[Values(10, 5, 0)] double od | ||
) | ||
{ | ||
// This is tested in TestNull. | ||
if (judgeMax + judge50 == 0) Assert.Ignore(); | ||
|
||
int noteCount = isAllHolds ? 0 : judgeMax + judge50 + judge0; | ||
int holdCount = isAllHolds ? judgeMax + judge50 + judge0 : 0; | ||
|
||
double? estimatedUr = computeUnstableRate( | ||
new[] { judgeMax, 0, 0, 0, judge50, judge0 }, | ||
noteCount, | ||
holdCount, | ||
od, | ||
speedMod, | ||
isHoldsLegacy | ||
); | ||
Assert.That( | ||
estimatedUr, Is.AtMost(1_000_000_000), | ||
$"The estimated mania UR {estimatedUr} returned too high for a single note." | ||
); | ||
} | ||
|
||
/// <summary> | ||
/// This tests if the UR gets smaller, given more MAX judgements. | ||
/// This follows the logic that: | ||
/// - More MAX judgements implies stronger evidence of smaller UR, as the probability of hitting a MAX judgement is higher. | ||
/// <remarks> | ||
/// It's not necessary, nor logical to test other behaviors. | ||
/// </remarks> | ||
/// </summary> | ||
[Test] | ||
public void TestMoreMaxJudgementsSmallerUr( | ||
[Values(1, 10, 1000)] int count, | ||
[Values(1, 10, 1000)] int step | ||
) | ||
{ | ||
int[] judgementCountsLess = { count, 0, 0, 0, 0, 0 }; | ||
int[] judgementCountsMore = { count + step, 0, 0, 0, 0, 0 }; | ||
double? estimatedUrLessJudgements = computeUnstableRate(judgementCountsLess); | ||
double? estimatedUrMoreJudgements = computeUnstableRate(judgementCountsMore); | ||
|
||
// Assert that More Judgements results in a smaller UR. | ||
Assert.That( | ||
estimatedUrMoreJudgements, Is.LessThan(estimatedUrLessJudgements), | ||
$"UR {estimatedUrMoreJudgements} with More Judgements {string.Join(",", judgementCountsMore)} >= " | ||
+ $"UR {estimatedUrLessJudgements} than Less Judgements {string.Join(",", judgementCountsLess)} " | ||
); | ||
} | ||
|
||
/// <summary> | ||
/// Evaluates the Unstable Rate | ||
/// </summary> | ||
/// <param name="judgementCounts">Size-6 Int List of Judgements, starting from MAX</param> | ||
/// <param name="noteCount">Number of notes</param> | ||
/// <param name="holdCount">Number of holds</param> | ||
/// <param name="od">Overall Difficulty</param> | ||
/// <param name="speedMod">Speed Mod, <see cref="SpeedMod"/></param> | ||
/// <param name="isHoldsLegacy">Whether to append ClassicMod to simulate Legacy Holds</param> | ||
private double? computeUnstableRate( | ||
IReadOnlyList<int> judgementCounts, | ||
int? noteCount = null, | ||
int holdCount = 0, | ||
double od = 5, | ||
SpeedMod speedMod = SpeedMod.NormalTime, | ||
bool isHoldsLegacy = false) | ||
{ | ||
var judgements = new Dictionary<HitResult, int> | ||
{ | ||
{ HitResult.Perfect, judgementCounts[0] }, | ||
{ HitResult.Great, judgementCounts[1] }, | ||
{ HitResult.Good, judgementCounts[2] }, | ||
{ HitResult.Ok, judgementCounts[3] }, | ||
{ HitResult.Meh, judgementCounts[4] }, | ||
{ HitResult.Miss, judgementCounts[5] } | ||
}; | ||
noteCount ??= judgements.Sum(kvp => kvp.Value); | ||
|
||
var mods = new Mod[] { }; | ||
|
||
if (isHoldsLegacy) mods = mods.Append(new ManiaModClassic()).ToArray(); | ||
|
||
switch (speedMod) | ||
{ | ||
case SpeedMod.DoubleTime: | ||
mods = mods.Append(new ManiaModDoubleTime()).ToArray(); | ||
break; | ||
|
||
case SpeedMod.HalfTime: | ||
mods = mods.Append(new ManiaModHalfTime()).ToArray(); | ||
break; | ||
} | ||
|
||
ManiaPerformanceAttributes perfAttributes = new ManiaPerformanceCalculator().Calculate( | ||
new ScoreInfo | ||
{ | ||
Mods = mods, | ||
Statistics = judgements | ||
}, | ||
new ManiaDifficultyAttributes | ||
{ | ||
NoteCount = (int)noteCount, | ||
HoldNoteCount = holdCount, | ||
OverallDifficulty = od, | ||
Mods = mods | ||
} | ||
); | ||
|
||
return perfAttributes.EstimatedUr; | ||
} | ||
|
||
/// <summary> | ||
/// This ensures that external changes of hit windows don't break the ur calculator. | ||
/// This includes all ODs. | ||
/// </summary> | ||
[Test] | ||
public void RegressionTestHitWindows( | ||
[Range(0, 10, 0.5)] double overallDifficulty | ||
) | ||
{ | ||
DifficultyAttributes attributes = new ManiaDifficultyAttributes { OverallDifficulty = overallDifficulty }; | ||
|
||
var hitWindows = new ManiaHitWindows(); | ||
hitWindows.SetDifficulty(overallDifficulty); | ||
|
||
double[] trueHitWindows = | ||
{ | ||
hitWindows.WindowFor(HitResult.Perfect), | ||
hitWindows.WindowFor(HitResult.Great), | ||
hitWindows.WindowFor(HitResult.Good), | ||
hitWindows.WindowFor(HitResult.Ok), | ||
hitWindows.WindowFor(HitResult.Meh) | ||
}; | ||
|
||
ManiaPerformanceAttributes perfAttributes = new ManiaPerformanceCalculator().Calculate(new ScoreInfo(), attributes); | ||
|
||
// Platform-dependent math functions (Pow, Cbrt, Exp, etc) may result in minute differences. | ||
Assert.That(perfAttributes.HitWindows, Is.EqualTo(trueHitWindows).Within(0.000001), "The true mania hit windows are different to the ones calculated in ManiaPerformanceCalculator."); | ||
} | ||
} | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -12,6 +12,12 @@ public class ManiaPerformanceAttributes : PerformanceAttributes | |
[JsonProperty("difficulty")] | ||
public double Difficulty { get; set; } | ||
|
||
[JsonProperty("estimated_ur")] | ||
public double? EstimatedUr { get; set; } | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same as #20963, would prefer this to be called There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Sure. |
||
|
||
[JsonProperty("hit_windows")] | ||
public double[] HitWindows { get; set; } = null!; | ||
|
||
public override IEnumerable<PerformanceDisplayAttribute> GetAttributesForDisplay() | ||
{ | ||
foreach (var attribute in base.GetAttributesForDisplay()) | ||
|
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
I was going to ask why these are not just being read from the
IBeatmapOnlineInfo
param here and skipped entirely in.ToDatabaseAttributes()
, but then realised that wouldn't work because of keymods. I'm not sure how intentional this was but yeah.This is going to be something that we need to be wary of going forward unless realtime calculation becomes a thing.
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.
This part in particular was added by smoogi to calculate a sheet for the rework