From 97300a16bfe7a14e8eb16a97a35814b968194d4b Mon Sep 17 00:00:00 2001 From: Damien Elmes Date: Mon, 22 Feb 2021 19:46:57 +1000 Subject: [PATCH] implement fuzzing Notes: - The fuzz seed is now derived from the card id and # of reps, so if a card is undone and done again, the same fuzz will be used. - The intervals shown on the answer buttons now include the fuzz, instead of hiding it from the user. This will prevent questions about due dates being different to what was shown on the buttons, but will create questions about due dates being different for cards with the same interval, and some people may find it distracting for learning cards. The new approach is easier to reason about, but time will tell whether it's a net gain or not. - The env var we were using to shift the clock away from rollover for unit tests has been repurposed to also disable fuzzing, which simplifies the tests. - Cards in filtered decks without scheduling now have the preview delay fuzzed. - Sub-day learning cards are mostly fuzzed like before, but will apply the up-to-5-minutes of fuzz regardless of the time of day. - The answer buttons now round minute values, as the fuzz on short intervals is distracting. --- pylib/tests/run_pytest.py | 2 +- rslib/src/sched/answering.rs | 17 ++++++- rslib/src/sched/states/filtered.rs | 2 +- rslib/src/sched/states/learning.rs | 12 ++--- rslib/src/sched/states/mod.rs | 61 ++++++++++++++++++++++++ rslib/src/sched/states/preview_filter.rs | 10 ++-- rslib/src/sched/states/relearning.rs | 12 ++--- rslib/src/sched/states/review.rs | 60 ++++++++--------------- rslib/src/sched/timespan.rs | 19 ++++++-- rslib/src/timestamp.rs | 2 +- 10 files changed, 136 insertions(+), 61 deletions(-) diff --git a/pylib/tests/run_pytest.py b/pylib/tests/run_pytest.py index 5884588bf..6489ec747 100644 --- a/pylib/tests/run_pytest.py +++ b/pylib/tests/run_pytest.py @@ -3,7 +3,7 @@ import sys import pytest -os.environ["SHIFT_CLOCK_HACK"] = "1" +os.environ["ANKI_TEST_MODE"] = "1" if __name__ == "__main__": folder = os.path.join(os.path.dirname(__file__), "..", "tests") diff --git a/rslib/src/sched/answering.rs b/rslib/src/sched/answering.rs index 7360968b0..f3b9d5a0d 100644 --- a/rslib/src/sched/answering.rs +++ b/rslib/src/sched/answering.rs @@ -67,6 +67,11 @@ impl AnswerContext { lapse_multiplier: self.config.inner.lapse_multiplier, minimum_lapse_interval: self.config.inner.minimum_lapse_interval, in_filtered_deck: self.deck.is_filtered(), + preview_step: if let DeckKind::Filtered(deck) = &self.deck.kind { + deck.preview_delay + } else { + 0 + }, } } @@ -540,7 +545,7 @@ impl Collection { config: self.deck_config_for_card(card)?, timing, now: TimestampSecs::now(), - fuzz_seed: None, + fuzz_seed: get_fuzz_seed(card), }) } @@ -552,3 +557,13 @@ impl Collection { Ok(current.next_states(&state_ctx)) } } + +/// Return a consistent seed for a given card at a given number of reps. +/// If in test environment, disable fuzzing. +fn get_fuzz_seed(card: &Card) -> Option { + if *crate::timestamp::TESTING { + None + } else { + Some((card.id.0 as u64).wrapping_add(card.reps as u64)) + } +} diff --git a/rslib/src/sched/states/filtered.rs b/rslib/src/sched/states/filtered.rs index 717552d58..448b08528 100644 --- a/rslib/src/sched/states/filtered.rs +++ b/rslib/src/sched/states/filtered.rs @@ -28,7 +28,7 @@ impl FilteredState { pub(crate) fn next_states(self, ctx: &StateContext) -> NextCardStates { match self { - FilteredState::Preview(state) => state.next_states(), + FilteredState::Preview(state) => state.next_states(ctx), FilteredState::Rescheduling(state) => state.next_states(ctx), } } diff --git a/rslib/src/sched/states/learning.rs b/rslib/src/sched/states/learning.rs index 139b4d2f5..e142dad4f 100644 --- a/rslib/src/sched/states/learning.rs +++ b/rslib/src/sched/states/learning.rs @@ -33,20 +33,20 @@ impl LearnState { fn answer_again(self, ctx: &StateContext) -> LearnState { LearnState { remaining_steps: ctx.steps.remaining_for_failed(), - scheduled_secs: ctx.steps.again_delay_secs_learn(), + scheduled_secs: ctx.with_learning_fuzz(ctx.steps.again_delay_secs_learn()), } } fn answer_hard(self, ctx: &StateContext) -> CardState { if let Some(hard_delay) = ctx.steps.hard_delay_secs(self.remaining_steps) { LearnState { - scheduled_secs: hard_delay, + scheduled_secs: ctx.with_learning_fuzz(hard_delay), ..self } .into() } else { ReviewState { - scheduled_days: ctx.graduating_interval_good, + scheduled_days: ctx.fuzzed_graduating_interval_good(), ..Default::default() } .into() @@ -57,12 +57,12 @@ impl LearnState { if let Some(good_delay) = ctx.steps.good_delay_secs(self.remaining_steps) { LearnState { remaining_steps: ctx.steps.remaining_for_good(self.remaining_steps), - scheduled_secs: good_delay, + scheduled_secs: ctx.with_learning_fuzz(good_delay), } .into() } else { ReviewState { - scheduled_days: ctx.graduating_interval_good, + scheduled_days: ctx.fuzzed_graduating_interval_good(), ..Default::default() } .into() @@ -71,7 +71,7 @@ impl LearnState { fn answer_easy(self, ctx: &StateContext) -> ReviewState { ReviewState { - scheduled_days: ctx.graduating_interval_easy, + scheduled_days: ctx.fuzzed_graduating_interval_easy(), ..Default::default() } } diff --git a/rslib/src/sched/states/mod.rs b/rslib/src/sched/states/mod.rs index fd28d5fd8..fcd64fdd1 100644 --- a/rslib/src/sched/states/mod.rs +++ b/rslib/src/sched/states/mod.rs @@ -12,6 +12,9 @@ pub(crate) mod rescheduling_filter; pub(crate) mod review; pub(crate) mod steps; +use rand::prelude::*; +use rand::rngs::StdRng; + pub use { filtered::FilteredState, learning::LearnState, new::NewState, normal::NormalState, preview_filter::PreviewState, relearning::RelearnState, @@ -75,6 +78,64 @@ pub(crate) struct StateContext<'a> { // filtered pub in_filtered_deck: bool, + pub preview_step: u32, +} + +impl<'a> StateContext<'a> { + pub(crate) fn with_review_fuzz(&self, interval: f32) -> u32 { + // fixme: floor() is to match python + let interval = interval.floor(); + if let Some(seed) = self.fuzz_seed { + let mut rng = StdRng::seed_from_u64(seed); + let (lower, upper) = if interval < 2.0 { + (1.0, 1.0) + } else if interval < 3.0 { + (2.0, 3.0) + } else if interval < 7.0 { + fuzz_range(interval, 0.25, 0.0) + } else if interval < 30.0 { + fuzz_range(interval, 0.15, 2.0) + } else { + fuzz_range(interval, 0.05, 4.0) + }; + if lower >= upper { + lower + } else { + rng.gen_range(lower, upper) + } + } else { + interval + } + .round() as u32 + } + + /// Add up to 25% increase to seconds, but no more than 5 minutes. + pub(crate) fn with_learning_fuzz(&self, secs: u32) -> u32 { + if let Some(seed) = self.fuzz_seed { + let mut rng = StdRng::seed_from_u64(seed); + let upper_exclusive = secs + ((secs as f32) * 0.25).min(300.0).floor() as u32; + if secs >= upper_exclusive { + secs + } else { + rng.gen_range(secs, upper_exclusive) + } + } else { + secs + } + } + + pub(crate) fn fuzzed_graduating_interval_good(&self) -> u32 { + self.with_review_fuzz(self.graduating_interval_good as f32) + } + + pub(crate) fn fuzzed_graduating_interval_easy(&self) -> u32 { + self.with_review_fuzz(self.graduating_interval_easy as f32) + } +} + +fn fuzz_range(interval: f32, factor: f32, minimum: f32) -> (f32, f32) { + let delta = (interval * factor).max(minimum).max(1.0); + (interval - delta, interval + delta + 1.0) } #[derive(Debug)] diff --git a/rslib/src/sched/states/preview_filter.rs b/rslib/src/sched/states/preview_filter.rs index d49979f02..97173211c 100644 --- a/rslib/src/sched/states/preview_filter.rs +++ b/rslib/src/sched/states/preview_filter.rs @@ -1,7 +1,7 @@ // Copyright: Ankitects Pty Ltd and contributors // License: GNU AGPL, version 3 or later; http://www.gnu.org/licenses/agpl.html -use super::{IntervalKind, NextCardStates, NormalState}; +use super::{IntervalKind, NextCardStates, NormalState, StateContext}; #[derive(Debug, Clone, Copy, PartialEq)] pub struct PreviewState { @@ -14,10 +14,14 @@ impl PreviewState { IntervalKind::InSecs(self.scheduled_secs) } - pub(crate) fn next_states(self) -> NextCardStates { + pub(crate) fn next_states(self, ctx: &StateContext) -> NextCardStates { NextCardStates { current: self.into(), - again: self.into(), + again: PreviewState { + scheduled_secs: ctx.with_learning_fuzz(ctx.preview_step * 60), + ..self + } + .into(), hard: self.original_state.into(), good: self.original_state.into(), easy: self.original_state.into(), diff --git a/rslib/src/sched/states/relearning.rs b/rslib/src/sched/states/relearning.rs index da5dec23e..ae635ed51 100644 --- a/rslib/src/sched/states/relearning.rs +++ b/rslib/src/sched/states/relearning.rs @@ -33,11 +33,11 @@ impl RelearnState { } fn answer_again(self, ctx: &StateContext) -> CardState { - if let Some(learn_interval) = ctx.relearn_steps.again_delay_secs_relearn() { + if let Some(again_delay) = ctx.relearn_steps.again_delay_secs_relearn() { RelearnState { learning: LearnState { remaining_steps: ctx.relearn_steps.remaining_for_failed(), - scheduled_secs: learn_interval, + scheduled_secs: ctx.with_learning_fuzz(again_delay), }, review: ReviewState { scheduled_days: self.review.failing_review_interval(ctx), @@ -52,13 +52,13 @@ impl RelearnState { } fn answer_hard(self, ctx: &StateContext) -> CardState { - if let Some(learn_interval) = ctx + if let Some(hard_delay) = ctx .relearn_steps .hard_delay_secs(self.learning.remaining_steps) { RelearnState { learning: LearnState { - scheduled_secs: learn_interval, + scheduled_secs: ctx.with_learning_fuzz(hard_delay), ..self.learning }, review: ReviewState { @@ -73,13 +73,13 @@ impl RelearnState { } fn answer_good(self, ctx: &StateContext) -> CardState { - if let Some(learn_interval) = ctx + if let Some(good_delay) = ctx .relearn_steps .good_delay_secs(self.learning.remaining_steps) { RelearnState { learning: LearnState { - scheduled_secs: learn_interval, + scheduled_secs: ctx.with_learning_fuzz(good_delay), remaining_steps: ctx .relearn_steps .remaining_for_good(self.learning.remaining_steps), diff --git a/rslib/src/sched/states/review.rs b/rslib/src/sched/states/review.rs index 2c5cd137a..f145931d7 100644 --- a/rslib/src/sched/states/review.rs +++ b/rslib/src/sched/states/review.rs @@ -1,9 +1,6 @@ // Copyright: Ankitects Pty Ltd and contributors // License: GNU AGPL, version 3 or later; http://www.gnu.org/licenses/agpl.html -use rand::prelude::*; -use rand::rngs::StdRng; - use crate::revlog::RevlogReviewKind; use super::{ @@ -81,11 +78,11 @@ impl ReviewState { lapses: self.lapses + 1, }; - if let Some(learn_interval) = ctx.relearn_steps.again_delay_secs_relearn() { + if let Some(again_delay) = ctx.relearn_steps.again_delay_secs_relearn() { RelearnState { learning: LearnState { remaining_steps: ctx.relearn_steps.remaining_for_failed(), - scheduled_secs: learn_interval, + scheduled_secs: ctx.with_learning_fuzz(again_delay), }, review: again_review, } @@ -143,16 +140,18 @@ impl ReviewState { // fixme: floor() is to match python let hard_interval = - constrain_passing_interval(ctx, current_interval * hard_factor, hard_minimum); + constrain_passing_interval(ctx, current_interval * hard_factor, hard_minimum, true); let good_interval = constrain_passing_interval( ctx, (current_interval + (days_late / 2.0).floor()) * self.ease_factor, hard_interval + 1, + true, ); let easy_interval = constrain_passing_interval( ctx, (current_interval + days_late) * self.ease_factor * ctx.easy_multiplier, good_interval + 1, + true, ); (hard_interval, good_interval, easy_interval) @@ -169,11 +168,16 @@ impl ReviewState { let hard_interval = { let factor = ctx.hard_multiplier; let half_usual = factor / 2.0; - constrain_passing_interval(ctx, (elapsed * factor).max(scheduled * half_usual), 0) + constrain_passing_interval( + ctx, + (elapsed * factor).max(scheduled * half_usual), + 0, + false, + ) }; let good_interval = - constrain_passing_interval(ctx, (elapsed * self.ease_factor).max(scheduled), 0); + constrain_passing_interval(ctx, (elapsed * self.ease_factor).max(scheduled), 0, false); let easy_interval = { // currently flooring() f64s to match python output @@ -184,6 +188,7 @@ impl ReviewState { ((elapsed as f64 * self.ease_factor as f64).max(scheduled as f64) * reduced_bonus) .floor() as f32, 0, + false, ) }; @@ -191,44 +196,21 @@ impl ReviewState { } } -fn fuzz_range(interval: f32, factor: f32, minimum: f32) -> (f32, f32) { - let delta = (interval * factor).max(minimum).max(1.0); - (interval - delta, interval + delta) -} - /// Transform the provided hard/good/easy interval. /// - Apply configured interval multiplier. /// - Apply fuzz. /// - Ensure it is at least `minimum`, and at least 1. /// - Ensure it is at or below the configured maximum interval. -fn constrain_passing_interval(ctx: &StateContext, interval: f32, minimum: u32) -> u32 { +fn constrain_passing_interval(ctx: &StateContext, interval: f32, minimum: u32, fuzz: bool) -> u32 { // fixme: floor is to match python - let interval = interval.floor(); - with_review_fuzz(ctx.fuzz_seed, interval * ctx.interval_multiplier) + let interval = interval.floor() * ctx.interval_multiplier; + let interval = if fuzz { + ctx.with_review_fuzz(interval) + } else { + interval.floor() as u32 + }; + interval .max(minimum) .min(ctx.maximum_review_interval) .max(1) } - -fn with_review_fuzz(seed: Option, interval: f32) -> u32 { - // fixme: floor() is to match python - let interval = interval.floor(); - if let Some(seed) = seed { - let mut rng = StdRng::seed_from_u64(seed); - let (lower, upper) = if interval < 2.0 { - (1.0, 1.0) - } else if interval < 3.0 { - (2.0, 3.0) - } else if interval < 7.0 { - fuzz_range(interval, 0.25, 0.0) - } else if interval < 30.0 { - fuzz_range(interval, 0.15, 2.0) - } else { - fuzz_range(interval, 0.05, 4.0) - }; - rng.gen_range(lower, upper + 1.0) - } else { - interval - } - .round() as u32 -} diff --git a/rslib/src/sched/timespan.rs b/rslib/src/sched/timespan.rs index 2c8ce3ff4..9aefaea11 100644 --- a/rslib/src/sched/timespan.rs +++ b/rslib/src/sched/timespan.rs @@ -6,7 +6,7 @@ use crate::i18n::{tr_args, I18n, TR}; /// Short string like '4d' to place above answer buttons. pub fn answer_button_time(seconds: f32, i18n: &I18n) -> String { let span = Timespan::from_secs(seconds).natural_span(); - let args = tr_args!["amount" => span.as_rounded_unit()]; + let args = tr_args!["amount" => span.as_rounded_unit_for_answer_buttons()]; let key = match span.unit() { TimespanUnit::Seconds => TR::SchedulingAnswerButtonTimeSeconds, TimespanUnit::Minutes => TR::SchedulingAnswerButtonTimeMinutes, @@ -114,13 +114,26 @@ impl Timespan { /// truncates to one decimal place. pub fn as_rounded_unit(self) -> f32 { match self.unit { - // seconds/days as integer + // seconds/minutes/days as integer TimespanUnit::Seconds | TimespanUnit::Days => self.as_unit().round(), // other values shown to 1 decimal place _ => (self.as_unit() * 10.0).round() / 10.0, } } + /// Round seconds, minutes and days to integers, otherwise + /// truncates to one decimal place. + pub fn as_rounded_unit_for_answer_buttons(self) -> f32 { + match self.unit { + // seconds/minutes/days as integer + TimespanUnit::Seconds | TimespanUnit::Minutes | TimespanUnit::Days => { + self.as_unit().round() + } + // other values shown to 1 decimal place + _ => (self.as_unit() * 10.0).round() / 10.0, + } + } + pub fn unit(self) -> TimespanUnit { self.unit } @@ -161,7 +174,7 @@ mod test { let log = log::terminal(); let i18n = I18n::new(&["zz"], "", log); assert_eq!(answer_button_time(30.0, &i18n), "30s"); - assert_eq!(answer_button_time(70.0, &i18n), "1.2m"); + assert_eq!(answer_button_time(70.0, &i18n), "1m"); assert_eq!(answer_button_time(1.1 * MONTH, &i18n), "1.1mo"); } diff --git a/rslib/src/timestamp.rs b/rslib/src/timestamp.rs index 0abcd3872..ebadf757e 100644 --- a/rslib/src/timestamp.rs +++ b/rslib/src/timestamp.rs @@ -52,7 +52,7 @@ impl TimestampMillis { } lazy_static! { - static ref TESTING: bool = env::var("SHIFT_CLOCK_HACK").is_ok(); + pub(crate) static ref TESTING: bool = env::var("ANKI_TEST_MODE").is_ok(); } fn elapsed() -> time::Duration {