Fall back to SM2 relative order when memory state missing (#3771)

Closes #3770
This commit is contained in:
Damien Elmes 2025-01-26 17:26:46 +11:00 committed by GitHub
parent 71ae5a6b67
commit 5c84a0cb5e
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
4 changed files with 65 additions and 29 deletions

View file

@ -620,7 +620,7 @@ fn get_fuzz_factor(seed: Option<u64>) -> Option<f32> {
} }
#[cfg(test)] #[cfg(test)]
mod test { pub(crate) mod test {
use super::*; use super::*;
use crate::card::CardType; use crate::card::CardType;
use crate::deckconfig::ReviewMix; use crate::deckconfig::ReviewMix;
@ -741,7 +741,7 @@ mod test {
Ok(()) Ok(())
} }
fn v3_test_collection(cards: usize) -> Result<(Collection, Vec<CardId>)> { pub(crate) fn v3_test_collection(cards: usize) -> Result<(Collection, Vec<CardId>)> {
let mut col = Collection::new(); let mut col = Collection::new();
let nt = col.get_notetype_by_name("Basic")?.unwrap(); let nt = col.get_notetype_by_name("Basic")?.unwrap();
for _ in 0..cards { for _ in 0..cards {

View file

@ -1,5 +1,6 @@
// Copyright: Ankitects Pty Ltd and contributors // Copyright: Ankitects Pty Ltd and contributors
// License: GNU AGPL, version 3 or later; http://www.gnu.org/licenses/agpl.html // License: GNU AGPL, version 3 or later; http://www.gnu.org/licenses/agpl.html
mod error; mod error;
pub mod memory_state; pub mod memory_state;
pub mod params; pub mod params;

View file

@ -736,7 +736,7 @@ impl super::SqliteStorage {
} }
#[derive(Clone, Copy)] #[derive(Clone, Copy)]
enum ReviewOrderSubclause { pub(crate) enum ReviewOrderSubclause {
Day, Day,
Deck, Deck,
Random, Random,

View file

@ -346,51 +346,53 @@ fn add_extract_fsrs_relative_retrievability(db: &Connection) -> rusqlite::Result
move |ctx| { move |ctx| {
assert_eq!(ctx.len(), 5, "called with unexpected number of arguments"); assert_eq!(ctx.len(), 5, "called with unexpected number of arguments");
let Ok(card_data) = ctx.get_raw(0).as_str() else { let Ok(due) = ctx.get_raw(1).as_i64() else {
return Ok(None); return Ok(None);
}; };
if card_data.is_empty() { let Ok(interval) = ctx.get_raw(3).as_i64() else {
return Ok(None); return Ok(None);
} };
let card_data = &CardData::from_str(card_data); let Ok(next_day_at) = ctx.get_raw(4).as_i64() else {
let Ok(due) = ctx.get_raw(1).as_i64() else {
return Ok(None); return Ok(None);
}; };
let days_elapsed = if due > 365_000 { let days_elapsed = if due > 365_000 {
// (re)learning // (re)learning
let Ok(next_day_at) = ctx.get_raw(4).as_i64() else {
return Ok(None);
};
next_day_at.saturating_sub(due) as u32 / 86_400 next_day_at.saturating_sub(due) as u32 / 86_400
} else { } else {
let Ok(days_elapsed) = ctx.get_raw(2).as_i64() else { let Ok(days_elapsed) = ctx.get_raw(2).as_i64() else {
return Ok(None); return Ok(None);
}; };
let Ok(interval) = ctx.get_raw(3).as_i64() else {
return Ok(None);
};
let review_day = due.saturating_sub(interval); let review_day = due.saturating_sub(interval);
days_elapsed.saturating_sub(review_day) as u32 days_elapsed.saturating_sub(review_day) as u32
}; };
let Some(state) = card_data.memory_state() else { if let Ok(card_data) = ctx.get_raw(0).as_str() {
return Ok(None); if !card_data.is_empty() {
}; let card_data = &CardData::from_str(card_data);
let Some(mut desired_retrievability) = card_data.fsrs_desired_retention else { if let (Some(state), Some(mut desired_retrievability)) =
return Ok(None); (card_data.memory_state(), card_data.fsrs_desired_retention)
}; {
// avoid div by zero // avoid div by zero
desired_retrievability = desired_retrievability.max(0.0001); desired_retrievability = desired_retrievability.max(0.0001);
let current_retrievability = FSRS::new(None) let current_retrievability = FSRS::new(None)
.unwrap() .unwrap()
.current_retrievability(state.into(), days_elapsed) .current_retrievability(state.into(), days_elapsed)
.max(0.0001); .max(0.0001);
return Ok(Some(
// power should be the reciprocal of the value of DECAY in FSRS-rs,
// which is currently -0.5
-(current_retrievability.powi(-2) - 1.)
/ (desired_retrievability.powi(-2) - 1.),
));
}
}
}
// FSRS data missing; fall back to SM2 ordering
Ok(Some( Ok(Some(
// power should be the reciprocal of the value of DECAY in FSRS-rs, which is -((days_elapsed as f32) + 0.001) / (interval as f32).max(1.0),
// currently -0.5
-(current_retrievability.powi(-2) - 1.) / (desired_retrievability.powi(-2) - 1.),
)) ))
}, },
) )
@ -613,3 +615,36 @@ impl Display for SqlSortOrder {
) )
} }
} }
#[cfg(test)]
mod test {
use super::*;
use crate::scheduler::answering::test::v3_test_collection;
use crate::storage::card::ReviewOrderSubclause;
#[test]
fn missing_memory_state_falls_back_to_sm2() -> Result<()> {
let (mut col, _cids) = v3_test_collection(1)?;
col.set_config_bool(BoolKey::Fsrs, true, true)?;
col.answer_easy();
let timing = col.timing_today()?;
let order = SqlSortOrder::Ascending;
let sql_func = ReviewOrderSubclause::RetrievabilityFsrs { timing, order }
.to_string()
.replace(" asc", "");
let sql = format!("select {sql_func} from cards");
// value from fsrs
let mut pos: Option<f64>;
pos = col.storage.db_scalar(&sql).unwrap();
assert_eq!(pos, Some(0.0));
// erasing the memory state should not result in None output
col.storage.db.execute("update cards set data=''", [])?;
pos = col.storage.db_scalar(&sql).unwrap();
assert!(pos.is_some());
// but it won't match the fsrs value
assert!(pos.unwrap() < -0.0);
Ok(())
}
}