Fix/set pretrain_only by fsrs_items

[L-M-Sherlock]

It fixed https://forums.ankiweb.net/t/anki-24-04-beta/41792/18.

[dae]

I'm concerned this change is undoing work done in #3027. We want to show 'reviews' rather than 'items' to the user, as FSRS items are not an intuitive thing for users.

Also, could I trouble you to explain why this change works? The goal of the pretrain_only argument was so that the Anki crate could have control over the threshold. Why does the code work when using FSRS items and not when using revlogs?

[L-M-Sherlock]

Also, could I trouble you to explain why this change works? The goal of the pretrain_only argument was so that the Anki crate could have control over the threshold. Why does the code work when using FSRS items and not when using revlogs?

Because revlogs.len() is greater than review_count. When review_count is 900 and revlogs.len() is 1001, the pretrain_only will be False. It's unintended.

[L-M-Sherlock]

And review_count is fsrs_items plus the number of cards. If the user has 400 new cards and learn them in a day, the review_count is 400, but the fsrs_items is 0. In this case, FSRS will raise NotEnoughData error.

[abdnh]

If the user has 400 new cards and learn them in a day, the review_count is 400, but the fsrs_items is 0.

review_count will be 0 when fsrs_items is 0 according to my understanding of fsrs_items_for_training().

[L-M-Sherlock]

review_count will be 0 when fsrs_items is 0 according to my understanding of fsrs_items_for_training().

They are different. fsrs_items is the number of sample for training. It only contains revlogs with length >= 2, because the reviews.history shouldn't be empty.

#[derive(Debug, Clone, Deserialize, Serialize, PartialEq, Eq)]
pub struct FSRSItem {
    pub reviews: Vec<FSRSReview>,
}

#[derive(Debug, Clone, Deserialize, Serialize, PartialEq, Eq)]
pub struct FSRSReview {
    /// 1-4
    pub rating: u32,
    /// The number of days that passed
    pub delta_t: u32,
}

impl FSRSItem {
    // The previous reviews done before the current one.
    pub(crate) fn history(&self) -> impl Iterator<Item = &FSRSReview> {
        self.reviews.iter().take(self.reviews.len() - 1)
    }

    pub(crate) fn current(&self) -> &FSRSReview {
        self.reviews.last().unwrap()
    }
}

[abdnh]

I'm not arguing they are equivalent, just that fsrs_items_for_training() returns review_count=0 when fsrs_items=0. That's because it's incrementing review_count inside .flat_map() after .filter_map(), and single_card_revlog_to_items() returns None when there are no items.

[L-M-Sherlock]

OK. But it's possible that the review_cout is 400 and the fsrs_items is 200, right?

[L-M-Sherlock]

I update the code. Now fsrs_items is only used for pretrain_only and progress.

[abdnh]

Should we use it for progress though?

[L-M-Sherlock]

Should we use it for progress though?

If we use review count for progress, it will never reach 100%.

[dae]

Only current and total are used to calculate the % in renderWeightProgress - the reviews count is purely shown for the user. So I think there's no need to change s.reviews?

[dae]

Thanks Jarrett!

[Expertium]

Sorry for barging in, but I suggest adding the review count to the "Evaluate" window.