search

aricneto / TwistyTimer

Twisty Timer is a material design twisty puzzle timer for Android. It uses the TNoodle library to generate scramble sequences for all current official speedsolving puzzles.
GNU General Public License v3.0
242 stars 54 forks source link

Ao50/Ao100 Calculation Problems #107

Closed XuLx closed 5 years ago

XuLx commented 7 years ago

I have a lot of DNFs for 3BLD, but the timer still shows that Ao50 and Ao100 are not DNFs. It seems that the calculations didn't take DNFs into account.

screenshot_2017-09-28-18-32-21 screenshot_2017-09-28-18-32-14

Combinacijus commented 7 years ago

It always was this way. At least in previous version ao50 calculation only calculated last 50 times excluding DNF. How about now I don't know

xaviershay commented 7 years ago

reference for correct behaviour: https://www.speedsolving.com/wiki/index.php/DNF

Note that a DNF is considered to be a worse result than any time or amount of moves, so in an average of 5 it will be removed along with the best solve, but if there are two or more, then the average becomes a DNF as well.

xaviershay commented 7 years ago

Leaving notes since I might try to fix this:

Test file: https://github.com/aricneto/TwistyTimer/blob/master/app/src/test/java/com/aricneto/twistytimer/stats/AverageCalculatorTestCase.java

Averages are tests for 1, 3 and 5 solves. This covers

  • the trivial case (1), the case where a simple arithmetic mean is used (3) and the case where a
  • truncated arithmetic mean is used (5). Values above 5 should be no different.

Great comment in AverageCalculator suggests this may be a deliberate choice for performance reasons: https://github.com/aricneto/TwistyTimer/blob/0e37b419b6b375173dddc6c02596f436fd2ce87f/app/src/main/java/com/aricneto/twistytimer/stats/AverageCalculator.java#L20 ... I'm very beginner at Android dev but I'm surprised that for such considerations are needed for what I'd naively consider a small N.

https://github.com/aricneto/TwistyTimer/blob/0e37b419b6b375173dddc6c02596f436fd2ce87f/app/src/main/java/com/aricneto/twistytimer/stats/Statistics.java#L83 appears that this was done deliberately, since it's calling the average method and instructing it to exclude DNFs (see https://github.com/aricneto/TwistyTimer/blob/0e37b419b6b375173dddc6c02596f436fd2ce87f/app/src/main/java/com/aricneto/twistytimer/stats/Statistics.java#L216 for method signature).

Suggested next steps:

aricneto commented 5 years ago

This is intended behavior as @xaviershay pointed out. AFAIK, there are no standards on how many DNFs should disqualify an Ao50+, so putting one would be kinda arbitrary.

I'm closing this issue, but if you manage to find any resources pointing to how many DNFs should disqualify these bigger averages, feel free to open it again.