Update FSRS; fix handling of invalid revlogs

State is now inferred from SM-2 data when the revlog is not suitable

Cargo.lock

@@ -1532,7 +1532,7 @@ dependencies = [
 [[package]]
 name = "fsrs"
 version = "0.1.0"
-source = "git+https://github.com/open-spaced-repetition/fsrs-rs.git?rev=fefe01fca179f930fc2b837db85fe63cf0cb75f3#fefe01fca179f930fc2b837db85fe63cf0cb75f3"
+source = "git+https://github.com/open-spaced-repetition/fsrs-rs.git?rev=9e20a8b32ebae50230cc39e2331e9593660d56ed#9e20a8b32ebae50230cc39e2331e9593660d56ed"
 dependencies = [
  "burn",
  "itertools 0.11.0",

Cargo.toml

@@ -40,7 +40,7 @@ rev = "184b2ca50ed39ca43da13f0b830a463861adb9ca"
 
 [workspace.dependencies.fsrs]
 git = "https://github.com/open-spaced-repetition/fsrs-rs.git"
-rev = "fefe01fca179f930fc2b837db85fe63cf0cb75f3"
+rev = "9e20a8b32ebae50230cc39e2331e9593660d56ed"
 # path = "../../../fsrs-rs"
 
 [workspace.dependencies]

rslib/src/scheduler/answering/mod.rs

@@ -8,7 +8,6 @@ mod relearning;
 mod review;
 mod revlog;
 
-use fsrs::MemoryState;
 use fsrs::NextStates;
 use fsrs::FSRS;
 use rand::prelude::*;
@@ -30,7 +29,7 @@ use crate::deckconfig::DeckConfig;
 use crate::deckconfig::LeechAction;
 use crate::decks::Deck;
 use crate::prelude::*;
-use crate::scheduler::fsrs::weights::fsrs_items_for_memory_state;
+use crate::scheduler::fsrs::memory_state::single_card_revlog_to_item;
 use crate::search::SearchNode;
 
 #[derive(Copy, Clone)]
@@ -347,7 +346,7 @@ impl Collection {
         )
     }
 
-    fn card_state_updater(&mut self, card: Card) -> Result<CardStateUpdater> {
+    fn card_state_updater(&mut self, mut card: Card) -> Result<CardStateUpdater> {
         let timing = self.timing_today()?;
         let deck = self
             .storage
@@ -357,20 +356,16 @@ impl Collection {
         let fsrs_enabled = self.get_config_bool(BoolKey::Fsrs);
         let fsrs_next_states = if fsrs_enabled {
             let fsrs = FSRS::new(Some(&config.inner.fsrs_weights))?;
-            let memory_state = if let Some(state) = card.memory_state {
+            if card.memory_state.is_none() && card.ctype != CardType::New {
-                Some(MemoryState::from(state))
-            } else if card.ctype == CardType::New {
-                None
-            } else {
                 // Card has been moved or imported into an FSRS deck after weights were set,
                 // and will need its initial memory state to be calculated based on review
                 // history.
                 let revlog = self.revlog_for_srs(SearchNode::CardIds(card.id.to_string()))?;
-                let mut fsrs_items = fsrs_items_for_memory_state(revlog, timing.next_day_at);
+                let item = single_card_revlog_to_item(revlog, timing.next_day_at);
-                fsrs_items.pop().map(|(_cid, item)| fsrs.memory_state(item))
+                card.set_memory_state(&fsrs, item);
-            };
+            }
             Some(fsrs.next_states(
-                memory_state,
+                card.memory_state.map(Into::into),
                 config.inner.desired_retention,
                 card.days_since_last_review(&timing).unwrap_or_default(),
             ))

rslib/src/scheduler/fsrs/memory_state.rs

@@ -2,11 +2,13 @@
 // License: GNU AGPL, version 3 or later; http://www.gnu.org/licenses/agpl.html
 
 use anki_proto::scheduler::ComputeMemoryStateResponse;
+use fsrs::FSRSItem;
 use fsrs::FSRS;
+use itertools::Itertools;
 
-use crate::card::FsrsMemoryState;
+use crate::card::CardType;
 use crate::prelude::*;
-use crate::scheduler::fsrs::weights::fsrs_items_for_memory_state;
+use crate::revlog::RevlogEntry;
 use crate::scheduler::fsrs::weights::single_card_revlog_to_items;
 use crate::scheduler::fsrs::weights::Weights;
 use crate::search::JoinSearches;
@@ -48,8 +50,7 @@ impl Collection {
                 let mut card = self.storage.get_card(card_id)?.or_not_found(card_id)?;
                 let original = card.clone();
                 if weights_and_desired_retention.is_some() {
-                    let state = fsrs.memory_state(item);
+                    card.set_memory_state(&fsrs, item);
-                    card.memory_state = Some(state.into());
                     card.desired_retention = desired_retention;
                 } else {
                     card.memory_state = None;
@@ -62,7 +63,7 @@ impl Collection {
     }
 
     pub fn compute_memory_state(&mut self, card_id: CardId) -> Result<ComputeMemoryStateResponse> {
-        let card = self.storage.get_card(card_id)?.or_not_found(card_id)?;
+        let mut card = self.storage.get_card(card_id)?.or_not_found(card_id)?;
         let deck_id = card.original_deck_id.or(card.deck_id);
         let deck = self.get_deck(deck_id)?.or_not_found(card.deck_id)?;
         let conf_id = DeckConfigId(deck.normal()?.config_id);
@@ -73,20 +74,54 @@ impl Collection {
         let desired_retention = config.inner.desired_retention;
         let fsrs = FSRS::new(Some(&config.inner.fsrs_weights))?;
         let revlog = self.revlog_for_srs(SearchNode::CardIds(card.id.to_string()))?;
-        let items = single_card_revlog_to_items(revlog, self.timing_today()?.next_day_at, false);
+        let item = single_card_revlog_to_item(revlog, self.timing_today()?.next_day_at);
-        if let Some(mut items) = items {
+        card.set_memory_state(&fsrs, item);
-            if let Some(last) = items.pop() {
-                let state = fsrs.memory_state(last);
-                let state = FsrsMemoryState::from(state);
-                return Ok(ComputeMemoryStateResponse {
-                    state: Some(state.into()),
-                    desired_retention,
-                });
-            }
-        }
         Ok(ComputeMemoryStateResponse {
-            state: None,
+            state: card.memory_state.map(Into::into),
             desired_retention,
         })
     }
 }
+
+impl Card {
+    pub(crate) fn set_memory_state(&mut self, fsrs: &FSRS, item: Option<FSRSItem>) {
+        self.memory_state = item
+            .map(|i| fsrs.memory_state(i))
+            .or_else(|| {
+                if self.ctype == CardType::New {
+                    None
+                } else {
+                    Some(fsrs.memory_state_from_sm2(self.ease_factor(), self.interval as f32))
+                }
+            })
+            .map(Into::into);
+    }
+}
+
+/// When updating memory state, FSRS only requires the last FSRSItem that
+/// contains the full history.
+pub(crate) fn fsrs_items_for_memory_state(
+    revlogs: Vec<RevlogEntry>,
+    next_day_at: TimestampSecs,
+) -> Vec<(CardId, Option<FSRSItem>)> {
+    revlogs
+        .into_iter()
+        .group_by(|r| r.cid)
+        .into_iter()
+        .map(|(card_id, group)| {
+            (
+                card_id,
+                single_card_revlog_to_item(group.collect(), next_day_at),
+            )
+        })
+        .collect()
+}
+
+/// When calculating memory state, only the last FSRSItem is required.
+pub(crate) fn single_card_revlog_to_item(
+    entries: Vec<RevlogEntry>,
+    next_day_at: TimestampSecs,
+) -> Option<FSRSItem> {
+    let items = single_card_revlog_to_items(entries, next_day_at, false);
+    items.and_then(|mut i| i.pop())
+}

rslib/src/scheduler/fsrs/weights.rs

@@ -107,24 +107,6 @@ fn fsrs_items_for_training(revlogs: Vec<RevlogEntry>, next_day_at: TimestampSecs
     revlogs
 }
 
-/// When updating memory state, FSRS only requires the last FSRSItem that
-/// contains the full history.
-pub(crate) fn fsrs_items_for_memory_state(
-    revlogs: Vec<RevlogEntry>,
-    next_day_at: TimestampSecs,
-) -> Vec<(CardId, FSRSItem)> {
-    let mut out = vec![];
-    for (card_id, group) in revlogs.into_iter().group_by(|r| r.cid).into_iter() {
-        let entries = group.into_iter().collect_vec();
-        if let Some(mut items) = single_card_revlog_to_items(entries, next_day_at, false) {
-            if let Some(item) = items.pop() {
-                out.push((card_id, item));
-            }
-        }
-    }
-    out
-}
-
 /// Transform the revlog history for a card into a list of FSRSItems. FSRS
 /// expects multiple items for a given card when training - for revlog
 /// `[1,2,3]`, we create FSRSItems corresponding to `[1,2]` and `[1,2,3]`
@@ -153,7 +135,7 @@ pub(crate) fn single_card_revlog_to_items(
         if idx > 0 {
             entries.drain(..idx);
         }
-    } else if training {
+    } else {
         // we ignore cards that don't have any learning steps
         return None;
     }
@@ -207,8 +189,8 @@ pub(crate) fn single_card_revlog_to_items(
                 .take(outer_idx + 1)
                 .enumerate()
                 .map(|(inner_idx, r)| FSRSReview {
-                    rating: r.button_chosen as i32,
+                    rating: r.button_chosen as u32,
-                    delta_t: delta_ts[inner_idx] as i32,
+                    delta_t: delta_ts[inner_idx],
                 })
                 .collect();
             FSRSItem { reviews }
@@ -242,7 +224,7 @@ mod tests {
         }
     }
 
-    fn review(delta_t: i32) -> FSRSReview {
+    fn review(delta_t: u32) -> FSRSReview {
         FSRSReview { rating: 3, delta_t }
     }
 
@@ -358,23 +340,6 @@ mod tests {
         );
     }
 
-    #[test]
-    fn bypassed_learning_is_handled() {
-        assert_eq!(
-            convert(
-                &[
-                    RevlogEntry {
-                        ease_factor: 2500,
-                        ..revlog(RevlogReviewKind::Manual, 7)
-                    },
-                    revlog(RevlogReviewKind::Review, 6),
-                ],
-                false,
-            ),
-            fsrs_items!([review(0)])
-        );
-    }
-
     #[test]
     fn single_learning_step_skipped_when_training() {
         assert_eq!(