pass sort options into test scheduler

- split new card fetch order and subsequent sort order; use latter
when building queues
- default to spacing siblings when burying is off, with options to
show each sibling in turn, and shuffle the fetched cards
This commit is contained in:
Damien Elmes 2021-05-12 16:33:18 +10:00
parent b64f7a9456
commit a1bd6b481d
12 changed files with 176 additions and 95 deletions

View file

@ -309,15 +309,21 @@ service CardsService {
message DeckConfig {
message Config {
enum NewCardOrder {
NEW_CARD_ORDER_DUE = 0;
NEW_CARD_ORDER_RANDOM = 1;
enum NewCardFetchOrder {
NEW_CARD_FETCH_ORDER_DUE = 0;
NEW_CARD_FETCH_ORDER_RANDOM = 1;
}
enum NewCardSortOrder {
NEW_CARD_SORT_ORDER_TEMPLATE_THEN_DUE = 0;
NEW_CARD_SORT_ORDER_TEMPLATE_THEN_RANDOM = 1;
NEW_CARD_SORT_ORDER_DUE = 2;
NEW_CARD_SORT_ORDER_RANDOM = 3;
}
enum ReviewCardOrder {
REVIEW_CARD_ORDER_SHUFFLED_BY_DAY = 0;
REVIEW_CARD_ORDER_SHUFFLED = 1;
REVIEW_CARD_ORDER_INTERVALS_ASCENDING = 2;
REVIEW_CARD_ORDER_INTERVALS_DESCENDING = 3;
REVIEW_CARD_ORDER_DAY_THEN_RANDOM = 0;
REVIEW_CARD_ORDER_INTERVALS_ASCENDING = 1;
REVIEW_CARD_ORDER_INTERVALS_DESCENDING = 2;
// REVIEW_CARD_ORDER_RELATIVE_OVERDUE = 3;
}
enum ReviewMix {
REVIEW_MIX_MIX_WITH_REVIEWS = 0;
@ -350,8 +356,9 @@ message DeckConfig {
uint32 graduating_interval_good = 18;
uint32 graduating_interval_easy = 19;
NewCardOrder new_card_order = 20;
ReviewCardOrder review_order = 32;
NewCardFetchOrder new_card_fetch_order = 20;
NewCardSortOrder new_card_sort_order = 32;
ReviewCardOrder review_order = 33;
ReviewMix new_mix = 30;
ReviewMix interday_learning_mix = 31;

View file

@ -10,7 +10,7 @@ use crate::{
backend_proto::{self as pb},
prelude::*,
scheduler::{
new::NewCardSortOrder,
new::NewCardDueOrder,
states::{CardState, NextCardStates},
},
stats::studied_today,
@ -132,9 +132,9 @@ impl SchedulingService for Backend {
input.shift_existing,
);
let order = if random {
NewCardSortOrder::Random
NewCardDueOrder::Random
} else {
NewCardSortOrder::Preserve
NewCardDueOrder::Preserve
};
self.with_col(|col| {
col.sort_cards(&cids, start, step, order, shift)

View file

@ -9,7 +9,7 @@ pub use schema11::{DeckConfSchema11, NewCardOrderSchema11};
pub use update::UpdateDeckConfigsIn;
pub use crate::backend_proto::deck_config::{
config::{LeechAction, NewCardOrder, ReviewCardOrder, ReviewMix},
config::{LeechAction, NewCardFetchOrder, NewCardSortOrder, ReviewCardOrder, ReviewMix},
Config as DeckConfigInner,
};
@ -58,8 +58,9 @@ impl Default for DeckConfig {
minimum_lapse_interval: 1,
graduating_interval_good: 1,
graduating_interval_easy: 4,
new_card_order: NewCardOrder::Due as i32,
review_order: ReviewCardOrder::ShuffledByDay as i32,
new_card_fetch_order: NewCardFetchOrder::Due as i32,
new_card_sort_order: NewCardSortOrder::TemplateThenDue as i32,
review_order: ReviewCardOrder::DayThenRandom as i32,
new_mix: ReviewMix::MixWithReviews as i32,
interday_learning_mix: ReviewMix::MixWithReviews as i32,
leech_action: LeechAction::TagOnly as i32,

View file

@ -10,7 +10,7 @@ use serde_repr::{Deserialize_repr, Serialize_repr};
use serde_tuple::Serialize_tuple;
use super::{
DeckConfig, DeckConfigId, DeckConfigInner, NewCardOrder, INITIAL_EASE_FACTOR_THOUSANDS,
DeckConfig, DeckConfigId, DeckConfigInner, NewCardFetchOrder, INITIAL_EASE_FACTOR_THOUSANDS,
};
use crate::{serde::default_on_invalid, timestamp::TimestampSecs, types::Usn};
@ -46,6 +46,8 @@ pub struct DeckConfSchema11 {
interday_learning_mix: i32,
#[serde(default)]
review_order: i32,
#[serde(default)]
new_sort_order: i32,
#[serde(flatten)]
other: HashMap<String, Value>,
@ -210,6 +212,7 @@ impl Default for DeckConfSchema11 {
new_per_day_minimum: 0,
interday_learning_mix: 0,
review_order: 0,
new_sort_order: 0,
}
}
}
@ -260,10 +263,11 @@ impl From<DeckConfSchema11> for DeckConfig {
minimum_lapse_interval: c.lapse.min_int,
graduating_interval_good: c.new.ints.good as u32,
graduating_interval_easy: c.new.ints.easy as u32,
new_card_order: match c.new.order {
NewCardOrderSchema11::Random => NewCardOrder::Random,
NewCardOrderSchema11::Due => NewCardOrder::Due,
new_card_fetch_order: match c.new.order {
NewCardOrderSchema11::Random => NewCardFetchOrder::Random,
NewCardOrderSchema11::Due => NewCardFetchOrder::Due,
} as i32,
new_card_sort_order: c.new_sort_order,
review_order: c.review_order,
new_mix: c.new_mix,
interday_learning_mix: c.interday_learning_mix,
@ -308,7 +312,7 @@ impl From<DeckConfig> for DeckConfSchema11 {
}
}
let i = c.inner;
let new_order = i.new_card_order();
let new_order = i.new_card_fetch_order();
DeckConfSchema11 {
id: c.id,
mtime: c.mtime_secs,
@ -329,8 +333,8 @@ impl From<DeckConfig> for DeckConfSchema11 {
_unused: 0,
},
order: match new_order {
NewCardOrder::Random => NewCardOrderSchema11::Random,
NewCardOrder::Due => NewCardOrderSchema11::Due,
NewCardFetchOrder::Random => NewCardOrderSchema11::Random,
NewCardFetchOrder::Due => NewCardOrderSchema11::Due,
},
per_day: i.new_per_day,
other: new_other,
@ -360,6 +364,7 @@ impl From<DeckConfig> for DeckConfSchema11 {
new_per_day_minimum: i.new_per_day_minimum,
interday_learning_mix: i.interday_learning_mix,
review_order: i.review_order,
new_sort_order: i.new_card_sort_order,
}
}
}
@ -377,6 +382,7 @@ fn clear_other_duplicates(top_other: &mut HashMap<String, Value>) {
"newPerDayMinimum",
"interdayLearningMix",
"reviewOrder",
"newSortOrder",
] {
top_other.remove(*key);
}

View file

@ -150,7 +150,7 @@ impl Collection {
let previous_config_id = DeckConfigId(normal.config_id);
let previous_order = configs_before_update
.get(&previous_config_id)
.map(|c| c.inner.new_card_order())
.map(|c| c.inner.new_card_fetch_order())
.unwrap_or_default();
// if a selected (sub)deck, or its old config was removed, update deck to point to new config
@ -168,7 +168,7 @@ impl Collection {
// if new order differs, deck needs re-sorting
let current_order = configs_after_update
.get(&current_config_id)
.map(|c| c.inner.new_card_order())
.map(|c| c.inner.new_card_fetch_order())
.unwrap_or_default();
if previous_order != current_order {
self.sort_deck(deck_id, current_order, usn)?;
@ -183,7 +183,7 @@ impl Collection {
#[cfg(test)]
mod test {
use super::*;
use crate::{collection::open_test_collection, deckconfig::NewCardOrder};
use crate::{collection::open_test_collection, deckconfig::NewCardFetchOrder};
#[test]
fn updating() -> Result<()> {
@ -257,7 +257,7 @@ mod test {
assert_eq!(card1_pos(&mut col), 1);
reset_card1_pos(&mut col);
assert_eq!(card1_pos(&mut col), 0);
input.configs[1].inner.new_card_order = NewCardOrder::Random as i32;
input.configs[1].inner.new_card_fetch_order = NewCardFetchOrder::Random as i32;
assert_eq!(
col.update_deck_configs(input.clone())?.changes.changes.card,
true

View file

@ -306,9 +306,15 @@ impl Collection {
}
let next_pos = cache.next_position.unwrap();
match cache.deck_configs.get(&did).unwrap().inner.new_card_order() {
crate::deckconfig::NewCardOrder::Random => Ok(random_position(next_pos)),
crate::deckconfig::NewCardOrder::Due => Ok(next_pos),
match cache
.deck_configs
.get(&did)
.unwrap()
.inner
.new_card_fetch_order()
{
crate::deckconfig::NewCardFetchOrder::Random => Ok(random_position(next_pos)),
crate::deckconfig::NewCardFetchOrder::Due => Ok(next_pos),
}
}

View file

@ -7,7 +7,7 @@ use rand::seq::SliceRandom;
use crate::{
card::{CardQueue, CardType},
deckconfig::NewCardOrder,
deckconfig::NewCardFetchOrder,
prelude::*,
search::{SortMode, StateKind},
};
@ -37,7 +37,7 @@ pub(crate) struct NewCardSorter {
}
#[derive(PartialEq)]
pub enum NewCardSortOrder {
pub enum NewCardDueOrder {
NoteId,
Random,
Preserve,
@ -48,7 +48,7 @@ impl NewCardSorter {
cards: &[Card],
starting_from: u32,
step: u32,
order: NewCardSortOrder,
order: NewCardDueOrder,
) -> Self {
let nids = nids_in_desired_order(cards, order);
@ -69,20 +69,20 @@ impl NewCardSorter {
}
}
fn nids_in_desired_order(cards: &[Card], order: NewCardSortOrder) -> Vec<NoteId> {
if order == NewCardSortOrder::Preserve {
fn nids_in_desired_order(cards: &[Card], order: NewCardDueOrder) -> Vec<NoteId> {
if order == NewCardDueOrder::Preserve {
nids_in_preserved_order(cards)
} else {
let nids: HashSet<_> = cards.iter().map(|c| c.note_id).collect();
let mut nids: Vec<_> = nids.into_iter().collect();
match order {
NewCardSortOrder::NoteId => {
NewCardDueOrder::NoteId => {
nids.sort_unstable();
}
NewCardSortOrder::Random => {
NewCardDueOrder::Random => {
nids.shuffle(&mut rand::thread_rng());
}
NewCardSortOrder::Preserve => unreachable!(),
NewCardDueOrder::Preserve => unreachable!(),
}
nids
}
@ -128,7 +128,7 @@ impl Collection {
cids: &[CardId],
starting_from: u32,
step: u32,
order: NewCardSortOrder,
order: NewCardDueOrder,
shift: bool,
) -> Result<OpOutput<usize>> {
let usn = self.usn()?;
@ -142,7 +142,7 @@ impl Collection {
cids: &[CardId],
starting_from: u32,
step: u32,
order: NewCardSortOrder,
order: NewCardDueOrder,
shift: bool,
usn: Usn,
) -> Result<usize> {
@ -171,9 +171,9 @@ impl Collection {
col.sort_deck(
deck,
if random {
NewCardOrder::Random
NewCardFetchOrder::Random
} else {
NewCardOrder::Due
NewCardFetchOrder::Due
},
col.usn()?,
)
@ -183,7 +183,7 @@ impl Collection {
pub(crate) fn sort_deck(
&mut self,
deck: DeckId,
order: NewCardOrder,
order: NewCardFetchOrder,
usn: Usn,
) -> Result<usize> {
let cids = self.search_cards(match_all![deck, StateKind::New], SortMode::NoOrder)?;
@ -217,13 +217,13 @@ mod test {
let cards = vec![c1.clone(), c2.clone(), c3.clone()];
// Preserve
let sorter = NewCardSorter::new(&cards, 0, 1, NewCardSortOrder::Preserve);
let sorter = NewCardSorter::new(&cards, 0, 1, NewCardDueOrder::Preserve);
assert_eq!(sorter.position(&c1), 0);
assert_eq!(sorter.position(&c2), 1);
assert_eq!(sorter.position(&c3), 2);
// NoteId/step/starting
let sorter = NewCardSorter::new(&cards, 3, 2, NewCardSortOrder::NoteId);
let sorter = NewCardSorter::new(&cards, 3, 2, NewCardDueOrder::NoteId);
assert_eq!(sorter.position(&c3), 3);
assert_eq!(sorter.position(&c2), 5);
assert_eq!(sorter.position(&c1), 7);
@ -231,7 +231,7 @@ mod test {
// Random
let mut c1_positions = HashSet::new();
for _ in 1..100 {
let sorter = NewCardSorter::new(&cards, 0, 1, NewCardSortOrder::Random);
let sorter = NewCardSorter::new(&cards, 0, 1, NewCardDueOrder::Random);
c1_positions.insert(sorter.position(&c1));
if c1_positions.len() == cards.len() {
return;
@ -241,11 +241,11 @@ mod test {
}
}
impl From<NewCardOrder> for NewCardSortOrder {
fn from(o: NewCardOrder) -> Self {
impl From<NewCardFetchOrder> for NewCardDueOrder {
fn from(o: NewCardFetchOrder) -> Self {
match o {
NewCardOrder::Due => NewCardSortOrder::NoteId,
NewCardOrder::Random => NewCardSortOrder::Random,
NewCardFetchOrder::Due => NewCardDueOrder::NoteId,
NewCardFetchOrder::Random => NewCardDueOrder::Random,
}
}
}

View file

@ -70,7 +70,9 @@ impl QueueBuilder {
let previous_card_was_sibling_with_higher_ordinal = self
.new
.last()
.map(|previous| previous.note_id == card.note_id && previous.extra > card.extra)
.map(|previous| {
previous.note_id == card.note_id && previous.template_index > card.template_index
})
.unwrap_or(false);
if previous_card_was_sibling_with_higher_ordinal {
@ -87,7 +89,9 @@ impl QueueBuilder {
.enumerate()
.rev()
.filter_map(|(idx, queued_card)| {
if queued_card.note_id != card.note_id || queued_card.extra < card.extra {
if queued_card.note_id != card.note_id
|| queued_card.template_index < card.template_index
{
Some(idx + 1)
} else {
None
@ -147,26 +151,26 @@ mod test {
NewCard {
id: CardId(1),
note_id: NoteId(1),
extra: 0,
template_index: 0,
..Default::default()
},
NewCard {
id: CardId(2),
note_id: NoteId(2),
extra: 1,
template_index: 1,
..Default::default()
},
NewCard {
id: CardId(3),
note_id: NoteId(2),
extra: 2,
template_index: 2,
..Default::default()
},
NewCard {
id: CardId(4),
note_id: NoteId(2),
// lowest ordinal, should be used instead of card 2/3
extra: 0,
template_index: 0,
..Default::default()
},
];

View file

@ -19,7 +19,7 @@ use super::{
CardQueues, Counts, LearningQueueEntry, MainQueueEntry, MainQueueEntryKind,
};
use crate::{
deckconfig::{NewCardOrder, ReviewCardOrder, ReviewMix},
deckconfig::{NewCardSortOrder, ReviewCardOrder, ReviewMix},
prelude::*,
};
@ -31,8 +31,8 @@ pub(crate) struct DueCard {
pub mtime: TimestampSecs,
pub due: i32,
pub interval: u32,
pub hash: u64,
pub original_deck_id: DeckId,
pub hash: u64,
}
/// Temporary holder for new cards that will be built into a queue.
@ -43,8 +43,8 @@ pub(crate) struct NewCard {
pub mtime: TimestampSecs,
pub due: i32,
pub original_deck_id: DeckId,
/// Used to store template_idx, and for shuffling
pub extra: u64,
pub template_index: u32,
pub hash: u64,
}
impl From<DueCard> for MainQueueEntry {
@ -85,6 +85,14 @@ pub(super) struct BuryMode {
bury_reviews: bool,
}
#[derive(Default, Clone, Debug)]
pub(super) struct QueueSortOptions {
pub(super) new_order: NewCardSortOrder,
pub(super) review_order: ReviewCardOrder,
pub(super) day_learn_mix: ReviewMix,
pub(super) new_review_mix: ReviewMix,
}
#[derive(Default)]
pub(super) struct QueueBuilder {
pub(super) new: Vec<NewCard>,
@ -92,13 +100,17 @@ pub(super) struct QueueBuilder {
pub(super) learning: Vec<DueCard>,
pub(super) day_learning: Vec<DueCard>,
pub(super) seen_note_ids: HashMap<NoteId, BuryMode>,
pub(super) new_order: NewCardOrder,
pub(super) review_order: ReviewCardOrder,
pub(super) day_learn_mix: ReviewMix,
pub(super) new_review_mix: ReviewMix,
pub(super) sort_options: QueueSortOptions,
}
impl QueueBuilder {
pub(super) fn new(sort_options: QueueSortOptions) -> Self {
QueueBuilder {
sort_options,
..Default::default()
}
}
pub(super) fn build(
mut self,
top_deck_limits: RemainingLimits,
@ -107,7 +119,7 @@ impl QueueBuilder {
current_day: u32,
) -> CardQueues {
self.sort_new();
self.sort_reviews();
self.sort_reviews(current_day);
// split and sort learning
let learn_ahead_secs = learn_ahead_secs as i64;
@ -115,14 +127,18 @@ impl QueueBuilder {
let learn_count = due_learning.len();
// merge day learning in, and cap to parent review count
let main_iter = merge_day_learning(self.review, self.day_learning, self.day_learn_mix);
let main_iter = merge_day_learning(
self.review,
self.day_learning,
self.sort_options.day_learn_mix,
);
let main_iter = main_iter.take(top_deck_limits.review as usize);
let review_count = main_iter.len();
// cap to parent new count, note down the new count, then merge new in
self.new.truncate(top_deck_limits.new as usize);
let new_count = self.new.len();
let main_iter = merge_new(main_iter, self.new, self.new_review_mix);
let main_iter = merge_new(main_iter, self.new, self.sort_options.new_review_mix);
CardQueues {
counts: Counts {
@ -207,9 +223,27 @@ impl Collection {
let (decks, parent_count) = self.storage.deck_with_parents_and_children(deck_id)?;
let deck_map = self.storage.get_decks_map()?;
let config = self.storage.get_deck_config_map()?;
let sort_options = decks
.get(parent_count)
.unwrap()
.config_id()
.and_then(|config_id| config.get(&config_id))
.map(|config| QueueSortOptions {
new_order: config.inner.new_card_sort_order(),
review_order: config.inner.review_order(),
day_learn_mix: config.inner.interday_learning_mix(),
new_review_mix: config.inner.new_mix(),
})
.unwrap_or_else(|| {
// filtered decks do not space siblings
QueueSortOptions {
new_order: NewCardSortOrder::Due,
..Default::default()
}
});
let limits = remaining_limits_capped_to_parents(&decks, &config, timing.days_elapsed);
let selected_deck_limits = limits[parent_count];
let mut queues = QueueBuilder::default();
let mut queues = QueueBuilder::new(sort_options);
for (deck, mut limit) in decks.iter().zip(limits).skip(parent_count) {
if limit.review > 0 {

View file

@ -5,58 +5,75 @@ use std::{cmp::Ordering, hash::Hasher};
use fnv::FnvHasher;
use super::{DueCard, NewCard, NewCardOrder, QueueBuilder, ReviewCardOrder};
use super::{DueCard, NewCard, NewCardSortOrder, QueueBuilder, ReviewCardOrder};
impl QueueBuilder {
pub(super) fn sort_new(&mut self) {
match self.new_order {
NewCardOrder::Random => {
self.new.iter_mut().for_each(NewCard::hash_id_and_mtime);
self.new.sort_unstable_by(shuffle_new_card);
match self.sort_options.new_order {
NewCardSortOrder::TemplateThenDue => {
self.new.sort_unstable_by(template_then_due);
}
NewCardOrder::Due => {
self.new.sort_unstable_by(|a, b| a.due.cmp(&b.due));
NewCardSortOrder::TemplateThenRandom => {
self.new.iter_mut().for_each(NewCard::hash_id_and_mtime);
self.new.sort_unstable_by(template_then_random);
}
NewCardSortOrder::Due => self.new.sort_unstable_by(new_position),
NewCardSortOrder::Random => {
self.new.iter_mut().for_each(NewCard::hash_id_and_mtime);
self.new.sort_unstable_by(new_hash)
}
}
}
pub(super) fn sort_reviews(&mut self) {
self.review.iter_mut().for_each(DueCard::hash_id_and_mtime);
pub(super) fn sort_reviews(&mut self, _current_day: u32) {
self.day_learning
.iter_mut()
.for_each(DueCard::hash_id_and_mtime);
self.day_learning.sort_unstable_by(day_then_hash);
match self.review_order {
ReviewCardOrder::ShuffledByDay => {
self.review.sort_unstable_by(shuffle_by_day);
self.day_learning.sort_unstable_by(shuffle_by_day);
}
ReviewCardOrder::Shuffled => {
self.review.sort_unstable_by(shuffle_due_card);
self.day_learning.sort_unstable_by(shuffle_due_card);
match self.sort_options.review_order {
ReviewCardOrder::DayThenRandom => {
self.review.iter_mut().for_each(DueCard::hash_id_and_mtime);
self.review.sort_unstable_by(day_then_hash);
}
ReviewCardOrder::IntervalsAscending => {
self.review.sort_unstable_by(intervals_ascending);
self.day_learning.sort_unstable_by(shuffle_due_card);
}
ReviewCardOrder::IntervalsDescending => {
self.review
.sort_unstable_by(|a, b| intervals_ascending(b, a));
self.day_learning.sort_unstable_by(shuffle_due_card);
}
} // ReviewCardOrder::RelativeOverdue => {
// self.review
// .iter_mut()
// .for_each(|card| card.set_hash_to_relative_overdue(current_day));
// self.review.sort_unstable_by(due_card_hash)
// }
}
}
}
fn shuffle_new_card(a: &NewCard, b: &NewCard) -> Ordering {
a.extra.cmp(&b.extra)
fn template_then_due(a: &NewCard, b: &NewCard) -> Ordering {
(a.template_index, a.due).cmp(&(b.template_index, b.due))
}
fn shuffle_by_day(a: &DueCard, b: &DueCard) -> Ordering {
fn template_then_random(a: &NewCard, b: &NewCard) -> Ordering {
(a.template_index, a.hash).cmp(&(b.template_index, b.hash))
}
fn new_position(a: &NewCard, b: &NewCard) -> Ordering {
a.due.cmp(&b.due)
}
fn new_hash(a: &NewCard, b: &NewCard) -> Ordering {
a.hash.cmp(&b.hash)
}
fn day_then_hash(a: &DueCard, b: &DueCard) -> Ordering {
(a.due, a.hash).cmp(&(b.due, b.hash))
}
fn shuffle_due_card(a: &DueCard, b: &DueCard) -> Ordering {
#[allow(dead_code)]
fn due_card_hash(a: &DueCard, b: &DueCard) -> Ordering {
a.hash.cmp(&b.hash)
}
@ -75,6 +92,11 @@ impl DueCard {
hasher.write_i64(self.mtime.0);
self.hash = hasher.finish();
}
#[allow(dead_code)]
fn set_hash_to_relative_overdue(&mut self, _current_day: u32) {
todo!()
}
}
impl NewCard {
@ -82,6 +104,6 @@ impl NewCard {
let mut hasher = FnvHasher::default();
hasher.write_i64(self.id.0);
hasher.write_i64(self.mtime.0);
self.extra = hasher.finish();
self.hash = hasher.finish();
}
}

View file

@ -217,9 +217,10 @@ impl super::SqliteStorage {
id: row.get(0)?,
note_id: row.get(1)?,
due: row.get(2)?,
extra: row.get::<_, u32>(3)? as u64,
template_index: row.get(3)?,
mtime: row.get(4)?,
original_deck_id: row.get(5)?,
hash: 0,
}) {
break;
}

View file

@ -61,5 +61,5 @@ License: GNU AGPL, version 3 or later; http://www.gnu.org/licenses/agpl.html
<EnumSelector
label={tr.schedulingOrder()}
choices={newOrderChoices}
defaultValue={defaults.newCardOrder}
bind:value={$config.newCardOrder} />
defaultValue={defaults.newCardFetchOrder}
bind:value={$config.newCardFetchOrder} />