GitHub/Anki
1
0
Fork 0
mirror of https://github.com/ankitects/anki.git synced 2025-09-24 16:56:36 -04:00
Code Issues Projects Releases Packages 1 Wiki Activity Actions

drop binary heap in test scheduler

Browse source 
The original rationale was avoiding a possible O(n) insertion if
the learning card was due outside the cutoff, but the increased code
complexity doesn't seem worth it, given that learning cards will
rarely grow above 1000.

Also added a currently-disabled test that demonstrates the current undo
handling behaviour is yielding incorrect counts; that will be reworked
in the next commit, and this change will make that easier.
This commit is contained in:
Damien Elmes 2021-05-14 16:02:50 +10:00
parent c41d5ca4bf
commit 9990a10161
6 changed files with 326 additions and 258 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

38
rslib/src/scheduler/answering/mod.rs
View file

@ -7,7 +7,6 @@ mod preview;
mod relearning;
mod review;
mod revlog;
mod undo;
use revlog::RevlogEntryPartial;
@ -375,6 +374,43 @@ impl Collection {
}
}
// test helpers
#[cfg(test)]
impl Collection {
pub(crate) fn answer_again(&mut self) {
self.answer(|states| states.again, Rating::Again).unwrap()
}
#[allow(dead_code)]
pub(crate) fn answer_hard(&mut self) {
self.answer(|states| states.hard, Rating::Hard).unwrap()
}
pub(crate) fn answer_good(&mut self) {
self.answer(|states| states.good, Rating::Good).unwrap()
}
pub(crate) fn answer_easy(&mut self) {
self.answer(|states| states.easy, Rating::Easy).unwrap()
}
fn answer<F>(&mut self, get_state: F, rating: Rating) -> Result<()>
where
F: FnOnce(&NextCardStates) -> CardState,
{
let queued = self.next_card()?.unwrap();
self.answer_card(&CardAnswer {
card_id: queued.card.id,
current_state: queued.next_states.current,
new_state: get_state(&queued.next_states),
rating,
answered_at: TimestampMillis::now(),
milliseconds_taken: 0,
})?;
Ok(())
}
}
/// 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<u64> {

145
rslib/src/scheduler/answering/undo.rs
View file

@ -1,145 +0,0 @@
// Copyright: Ankitects Pty Ltd and contributors
// License: GNU AGPL, version 3 or later; http://www.gnu.org/licenses/agpl.html
#[cfg(test)]
mod test {
use crate::{
card::{CardQueue, CardType},
collection::open_test_collection,
deckconfig::LeechAction,
prelude::*,
scheduler::answering::{CardAnswer, Rating},
};
#[test]
fn undo() -> Result<()> {
// add a note
let mut col = open_test_collection();
let nt = col
.get_notetype_by_name("Basic (and reversed card)")?
.unwrap();
let mut note = nt.new_note();
note.set_field(0, "one")?;
note.set_field(1, "two")?;
col.add_note(&mut note, DeckId(1))?;
// turn burying and leech suspension on
let mut conf = col.storage.get_deck_config(DeckConfigId(1))?.unwrap();
conf.inner.bury_new = true;
conf.inner.leech_action = LeechAction::Suspend as i32;
col.storage.update_deck_conf(&conf)?;
// get the first card
let queued = col.next_card()?.unwrap();
let nid = note.id;
let cid = queued.card.id;
let sibling_cid = col.storage.all_card_ids_of_note_in_order(nid)?[1];
let assert_initial_state = |col: &mut Collection| -> Result<()> {
let first = col.storage.get_card(cid)?.unwrap();
assert_eq!(first.queue, CardQueue::New);
let sibling = col.storage.get_card(sibling_cid)?.unwrap();
assert_eq!(sibling.queue, CardQueue::New);
Ok(())
};
assert_initial_state(&mut col)?;
// immediately graduate the first card
col.answer_card(&CardAnswer {
card_id: queued.card.id,
current_state: queued.next_states.current,
new_state: queued.next_states.easy,
rating: Rating::Easy,
answered_at: TimestampMillis::now(),
milliseconds_taken: 0,
})?;
// the sibling will be buried
let sibling = col.storage.get_card(sibling_cid)?.unwrap();
assert_eq!(sibling.queue, CardQueue::SchedBuried);
// make it due now, with 7 lapses. we use the storage layer directly,
// bypassing undo
let mut card = col.storage.get_card(cid)?.unwrap();
assert_eq!(card.ctype, CardType::Review);
card.lapses = 7;
card.due = 0;
col.storage.update_card(&card)?;
// fail it, which should cause it to be marked as a leech
col.clear_study_queues();
let queued = col.next_card()?.unwrap();
dbg!(&queued);
col.answer_card(&CardAnswer {
card_id: queued.card.id,
current_state: queued.next_states.current,
new_state: queued.next_states.again,
rating: Rating::Again,
answered_at: TimestampMillis::now(),
milliseconds_taken: 0,
})?;
let assert_post_review_state = |col: &mut Collection| -> Result<()> {
let card = col.storage.get_card(cid)?.unwrap();
assert_eq!(card.interval, 1);
assert_eq!(card.lapses, 8);
assert_eq!(
col.storage.get_all_revlog_entries(TimestampSecs(0))?.len(),
2
);
let note = col.storage.get_note(nid)?.unwrap();
assert_eq!(note.tags, vec!["leech".to_string()]);
assert_eq!(col.storage.all_tags()?.is_empty(), false);
let deck = col.get_deck(DeckId(1))?.unwrap();
assert_eq!(deck.common.review_studied, 1);
dbg!(&col.next_card()?);
assert_eq!(col.next_card()?.is_some(), false);
Ok(())
};
let assert_pre_review_state = |col: &mut Collection| -> Result<()> {
// the card should have its old state, but a new mtime (which we can't
// easily test without waiting)
let card = col.storage.get_card(cid)?.unwrap();
assert_eq!(card.interval, 4);
assert_eq!(card.lapses, 7);
// the revlog entry should have been removed
assert_eq!(
col.storage.get_all_revlog_entries(TimestampSecs(0))?.len(),
1
);
// the note should no longer be tagged as a leech
let note = col.storage.get_note(nid)?.unwrap();
assert_eq!(note.tags.is_empty(), true);
assert_eq!(col.storage.all_tags()?.is_empty(), true);
let deck = col.get_deck(DeckId(1))?.unwrap();
assert_eq!(deck.common.review_studied, 0);
assert_eq!(col.next_card()?.is_some(), true);
Ok(())
};
// ensure everything is restored on undo/redo
assert_post_review_state(&mut col)?;
col.undo()?;
assert_pre_review_state(&mut col)?;
col.redo()?;
assert_post_review_state(&mut col)?;
col.undo()?;
assert_pre_review_state(&mut col)?;
col.undo()?;
assert_initial_state(&mut col)?;
Ok(())
}
}

53
rslib/src/scheduler/queue/builder/mod.rs
View file

@ -6,10 +6,7 @@ pub(crate) mod intersperser;
pub(crate) mod sized_chain;
mod sorting;
use std::{
cmp::Reverse,
collections::{BinaryHeap, HashMap, VecDeque},
};
use std::collections::{HashMap, VecDeque};
use intersperser::Intersperser;
use sized_chain::SizedChain;
@ -114,19 +111,19 @@ impl QueueBuilder {
pub(super) fn build(
mut self,
top_deck_limits: RemainingLimits,
learn_ahead_secs: u32,
learn_ahead_secs: i64,
selected_deck: DeckId,
current_day: u32,
) -> CardQueues {
self.sort_new();
self.sort_reviews(current_day);
// split and sort learning
let learn_ahead_secs = learn_ahead_secs as i64;
let (due_learning, later_learning) = split_learning(self.learning, learn_ahead_secs);
let learn_count = due_learning.len();
// intraday learning
let learning = sort_learning(self.learning);
let cutoff = TimestampSecs::now().adding_secs(learn_ahead_secs);
let learn_count = learning.iter().take_while(|e| e.due <= cutoff).count();
// merge day learning in, and cap to parent review count
// merge interday learning into main, and cap to parent review count
let main_iter = merge_day_learning(
self.review,
self.day_learning,
@ -148,8 +145,7 @@ impl QueueBuilder {
},
undo: Vec::new(),
main: main_iter.collect(),
due_learning,
later_learning,
learning,
learn_ahead_secs,
selected_deck,
current_day,
@ -186,34 +182,9 @@ fn merge_new(
}
}
/// Split the learning queue into cards due within limit, and cards due later
/// today. Learning does not need to be sorted in advance, as the sorting is
/// done as the heaps/dequeues are built.
fn split_learning(
learning: Vec<DueCard>,
learn_ahead_secs: i64,
) -> (
VecDeque<LearningQueueEntry>,
BinaryHeap<Reverse<LearningQueueEntry>>,
) {
let cutoff = TimestampSecs(TimestampSecs::now().0 + learn_ahead_secs);
// split learning into now and later
let (mut now, later): (Vec<_>, Vec<_>) = learning
.into_iter()
.map(LearningQueueEntry::from)
.partition(|c| c.due <= cutoff);
// sort due items in ascending order, as we pop the deque from the front
now.sort_unstable_by(|a, b| a.due.cmp(&b.due));
// partition() requires both outputs to be the same, so we need to create the deque
// separately
let now = VecDeque::from(now);
// build the binary min heap
let later: BinaryHeap<_> = later.into_iter().map(Reverse).collect();
(now, later)
fn sort_learning(mut learning: Vec<DueCard>) -> VecDeque<LearningQueueEntry> {
learning.sort_unstable_by(|a, b| a.due.cmp(&b.due));
learning.into_iter().map(LearningQueueEntry::from).collect()
}
impl Collection {
@ -293,7 +264,7 @@ impl Collection {
let queues = queues.build(
selected_deck_limits,
self.learn_ahead_secs(),
self.learn_ahead_secs() as i64,
deck_id,
timing.days_elapsed,
);

110
rslib/src/scheduler/queue/learning.rs
View file

@ -1,10 +1,7 @@
// Copyright: Ankitects Pty Ltd and contributors
// License: GNU AGPL, version 3 or later; http://www.gnu.org/licenses/agpl.html
use std::{
cmp::{Ordering, Reverse},
collections::VecDeque,
};
use std::{cmp::Ordering, collections::VecDeque};
use super::CardQueues;
use crate::{prelude::*, scheduler::timing::SchedTimingToday};
@ -34,24 +31,16 @@ impl CardQueues {
/// Does not check for newly due cards, as that is already done by
/// next_learning_entry_due_before_now()
pub(super) fn next_learning_entry_learning_ahead(&self) -> Option<LearningQueueEntry> {
self.due_learning.front().copied()
self.learning.front().copied()
}
pub(super) fn pop_learning_entry(&mut self, id: CardId) -> Option<LearningQueueEntry> {
if let Some(top) = self.due_learning.front() {
if let Some(top) = self.learning.front() {
if top.id == id {
self.counts.learning -= 1;
return self.due_learning.pop_front();
}
}
// fixme: remove this in the future
// the current python unit tests answer learning cards before they're due,
// so for now we also check the head of the later_due queue
if let Some(top) = self.later_learning.peek() {
if top.0.id == id {
// self.counts.learning -= 1;
return self.later_learning.pop().map(|c| c.0);
// under normal circumstances this should not go below 0, but currently
// the Python unit tests answer learning cards before they're due
self.counts.learning = self.counts.learning.saturating_sub(1);
return self.learning.pop_front();
}
}
@ -85,31 +74,28 @@ impl CardQueues {
mut entry: LearningQueueEntry,
timing: SchedTimingToday,
) -> LearningQueueEntry {
let learn_ahead_limit = timing.now.adding_secs(self.learn_ahead_secs);
if entry.due < learn_ahead_limit {
if self.learning_collapsed() {
if let Some(next) = self.due_learning.front() {
let cutoff = timing.now.adding_secs(self.learn_ahead_secs);
if entry.due <= cutoff && self.learning_collapsed() {
if let Some(next) = self.learning.front() {
// ensure the card is scheduled after the next due card
if next.due >= entry.due {
// the earliest due card is due later than this one; make this one
// due after that one
entry.due = next.due.adding_secs(1);
}
self.push_due_learning_card(entry);
if next.due < cutoff {
entry.due = next.due.adding_secs(1)
} else {
// nothing else waiting to review; make this due in a minute
entry.due = learn_ahead_limit.adding_secs(60);
self.later_learning.push(Reverse(entry));
// or outside the cutoff, in cases where the next due
// card is due later than the cutoff
entry.due = cutoff.adding_secs(60);
}
}
} else {
// not collapsed; can add normally
self.push_due_learning_card(entry);
// nothing else waiting to review; make this due a minute after
// cutoff
entry.due = cutoff.adding_secs(60);
}
} else {
// due outside current learn ahead limit, but later today
self.later_learning.push(Reverse(entry));
}
self.push_learning_card(entry);
entry
}
@ -117,41 +103,39 @@ impl CardQueues {
self.main.is_empty()
}
/// Adds card, maintaining correct sort order, and increments learning count.
pub(super) fn push_due_learning_card(&mut self, entry: LearningQueueEntry) {
self.counts.learning += 1;
/// Adds card, maintaining correct sort order, and increments learning count if
/// card is due within cutoff.
pub(super) fn push_learning_card(&mut self, entry: LearningQueueEntry) {
let target_idx =
binary_search_by(&self.due_learning, |e| e.due.cmp(&entry.due)).unwrap_or_else(|e| e);
self.due_learning.insert(target_idx, entry);
binary_search_by(&self.learning, |e| e.due.cmp(&entry.due)).unwrap_or_else(|e| e);
if target_idx < self.counts.learning {
self.counts.learning += 1;
}
self.learning.insert(target_idx, entry);
}
fn check_for_newly_due_learning_cards(&mut self, cutoff: TimestampSecs) {
while let Some(earliest) = self.later_learning.peek() {
if earliest.0.due > cutoff {
break;
}
let entry = self.later_learning.pop().unwrap().0;
self.push_due_learning_card(entry);
}
self.counts.learning += self
.learning
.iter()
.skip(self.counts.learning)
.take_while(|e| e.due <= cutoff)
.count();
}
pub(super) fn remove_requeued_learning_card_after_undo(&mut self, id: CardId) {
let due_idx = self
.due_learning
.iter()
.enumerate()
.find_map(|(idx, entry)| if entry.id == id { Some(idx) } else { None });
if let Some(idx) = due_idx {
self.counts.learning -= 1;
self.due_learning.remove(idx);
let due_idx = self.learning.iter().enumerate().find_map(|(idx, entry)| {
if entry.id == id {
Some(idx)
} else {
// card may be in the later_learning binary heap - we can't remove
// it in place, so we have to rebuild it
self.later_learning = self
.later_learning
.drain()
.filter(|e| e.0.id != id)
.collect();
None
}
});
if let Some(idx) = due_idx {
// FIXME: if we remove the saturating_sub from pop_learning(), maybe
// this can go too?
self.counts.learning = self.counts.learning.saturating_sub(1);
self.learning.remove(idx);
}
}
}

21
rslib/src/scheduler/queue/mod.rs
View file

@ -8,10 +8,7 @@ mod limits;
mod main;
pub(crate) mod undo;
use std::{
cmp::Reverse,
collections::{BinaryHeap, VecDeque},
};
use std::collections::VecDeque;
pub(crate) use builder::{DueCard, NewCard};
pub(crate) use entry::{QueueEntry, QueueEntryKind};
@ -28,8 +25,7 @@ pub(crate) struct CardQueues {
/// Any undone items take precedence.
undo: Vec<QueueEntry>,
main: VecDeque<MainQueueEntry>,
due_learning: VecDeque<LearningQueueEntry>,
later_learning: BinaryHeap<Reverse<LearningQueueEntry>>,
learning: VecDeque<LearningQueueEntry>,
selected_deck: DeckId,
current_day: u32,
learn_ahead_secs: i64,
@ -208,11 +204,24 @@ impl Collection {
}))
}
}
}
// test helpers
#[cfg(test)]
impl Collection {
pub(crate) fn next_card(&mut self) -> Result<Option<QueuedCard>> {
Ok(self
.next_cards(1, false)?
.map(|mut resp| resp.cards.pop().unwrap()))
}
pub(crate) fn get_queue_single(&mut self) -> Result<QueuedCards> {
self.next_cards(1, false)?.ok_or(AnkiError::NotFound)
}
pub(crate) fn counts(&mut self) -> [usize; 3] {
self.get_queue_single()
.map(|q| [q.new_count, q.learning_count, q.review_count])
.unwrap_or([0; 3])
}
}

213
rslib/src/scheduler/queue/undo.rs
View file

@ -77,3 +77,216 @@ impl CardQueues {
self.undo.push(entry);
}
}
#[cfg(test)]
mod test {
use crate::{
card::{CardQueue, CardType},
collection::open_test_collection,
deckconfig::LeechAction,
prelude::*,
};
fn add_note(col: &mut Collection, with_reverse: bool) -> Result<NoteId> {
let nt = col
.get_notetype_by_name("Basic (and reversed card)")?
.unwrap();
let mut note = nt.new_note();
note.set_field(0, "one")?;
if with_reverse {
note.set_field(1, "two")?;
}
col.add_note(&mut note, DeckId(1))?;
Ok(note.id)
}
#[test]
fn undo() -> Result<()> {
// add a note
let mut col = open_test_collection();
let nid = add_note(&mut col, true)?;
// turn burying and leech suspension on
let mut conf = col.storage.get_deck_config(DeckConfigId(1))?.unwrap();
conf.inner.bury_new = true;
conf.inner.leech_action = LeechAction::Suspend as i32;
col.storage.update_deck_conf(&conf)?;
// get the first card
let queued = col.next_card()?.unwrap();
let cid = queued.card.id;
let sibling_cid = col.storage.all_card_ids_of_note_in_order(nid)?[1];
let assert_initial_state = |col: &mut Collection| -> Result<()> {
let first = col.storage.get_card(cid)?.unwrap();
assert_eq!(first.queue, CardQueue::New);
let sibling = col.storage.get_card(sibling_cid)?.unwrap();
assert_eq!(sibling.queue, CardQueue::New);
Ok(())
};
assert_initial_state(&mut col)?;
// immediately graduate the first card
col.answer_easy();
// the sibling will be buried
let sibling = col.storage.get_card(sibling_cid)?.unwrap();
assert_eq!(sibling.queue, CardQueue::SchedBuried);
// make it due now, with 7 lapses. we use the storage layer directly,
// bypassing undo
let mut card = col.storage.get_card(cid)?.unwrap();
assert_eq!(card.ctype, CardType::Review);
card.lapses = 7;
card.due = 0;
col.storage.update_card(&card)?;
// fail it, which should cause it to be marked as a leech
col.clear_study_queues();
col.answer_again();
let assert_post_review_state = |col: &mut Collection| -> Result<()> {
let card = col.storage.get_card(cid)?.unwrap();
assert_eq!(card.interval, 1);
assert_eq!(card.lapses, 8);
assert_eq!(
col.storage.get_all_revlog_entries(TimestampSecs(0))?.len(),
2
);
let note = col.storage.get_note(nid)?.unwrap();
assert_eq!(note.tags, vec!["leech".to_string()]);
assert_eq!(col.storage.all_tags()?.is_empty(), false);
let deck = col.get_deck(DeckId(1))?.unwrap();
assert_eq!(deck.common.review_studied, 1);
assert_eq!(col.next_card()?.is_some(), false);
Ok(())
};
let assert_pre_review_state = |col: &mut Collection| -> Result<()> {
// the card should have its old state, but a new mtime (which we can't
// easily test without waiting)
let card = col.storage.get_card(cid)?.unwrap();
assert_eq!(card.interval, 4);
assert_eq!(card.lapses, 7);
// the revlog entry should have been removed
assert_eq!(
col.storage.get_all_revlog_entries(TimestampSecs(0))?.len(),
1
);
// the note should no longer be tagged as a leech
let note = col.storage.get_note(nid)?.unwrap();
assert_eq!(note.tags.is_empty(), true);
assert_eq!(col.storage.all_tags()?.is_empty(), true);
let deck = col.get_deck(DeckId(1))?.unwrap();
assert_eq!(deck.common.review_studied, 0);
assert_eq!(col.next_card()?.is_some(), true);
let q = col.get_queue_single()?;
assert_eq!(&[q.new_count, q.learning_count, q.review_count], &[0, 0, 1]);
Ok(())
};
// ensure everything is restored on undo/redo
assert_post_review_state(&mut col)?;
col.undo()?;
assert_pre_review_state(&mut col)?;
col.redo()?;
assert_post_review_state(&mut col)?;
col.undo()?;
assert_pre_review_state(&mut col)?;
col.undo()?;
assert_initial_state(&mut col)?;
Ok(())
}
#[test]
#[ignore = "undo code is currently broken"]
fn undo_counts1() -> Result<()> {
let mut col = open_test_collection();
assert_eq!(col.counts(), [0, 0, 0]);
add_note(&mut col, true)?;
assert_eq!(col.counts(), [2, 0, 0]);
col.answer_good();
assert_eq!(col.counts(), [1, 1, 0]);
col.answer_good();
assert_eq!(col.counts(), [0, 2, 0]);
col.answer_good();
assert_eq!(col.counts(), [0, 1, 0]);
col.answer_good();
assert_eq!(col.counts(), [0, 0, 0]);
// now work backwards
col.undo()?;
assert_eq!(col.counts(), [0, 1, 0]);
col.undo()?;
assert_eq!(col.counts(), [0, 2, 0]);
col.undo()?;
assert_eq!(col.counts(), [1, 1, 0]);
col.undo()?;
assert_eq!(col.counts(), [2, 0, 0]);
col.undo()?;
assert_eq!(col.counts(), [0, 0, 0]);
Ok(())
}
#[test]
fn undo_counts2() -> Result<()> {
let mut col = open_test_collection();
assert_eq!(col.counts(), [0, 0, 0]);
add_note(&mut col, true)?;
assert_eq!(col.counts(), [2, 0, 0]);
col.answer_again();
assert_eq!(col.counts(), [1, 1, 0]);
col.answer_again();
assert_eq!(col.counts(), [0, 2, 0]);
col.answer_again();
assert_eq!(col.counts(), [0, 2, 0]);
col.answer_again();
assert_eq!(col.counts(), [0, 2, 0]);
col.answer_good();
assert_eq!(col.counts(), [0, 2, 0]);
col.answer_easy();
assert_eq!(col.counts(), [0, 1, 0]);
col.answer_good();
// last card, due in a minute
assert_eq!(col.counts(), [0, 0, 0]);
Ok(())
}
#[test]
fn undo_counts_relearn() -> Result<()> {
let mut col = open_test_collection();
add_note(&mut col, true)?;
col.storage
.db
.execute_batch("update cards set due=0,queue=2,type=2")?;
assert_eq!(col.counts(), [0, 0, 2]);
col.answer_again();
assert_eq!(col.counts(), [0, 1, 1]);
col.answer_again();
assert_eq!(col.counts(), [0, 2, 0]);
col.answer_easy();
assert_eq!(col.counts(), [0, 1, 0]);
col.answer_easy();
assert_eq!(col.counts(), [0, 0, 0]);
Ok(())
}
}