open-spaced-repetition / ts-fsrs

ts-fsrs is a versatile package based on TypeScript that supports ES modules, CommonJS, and UMD.
https://open-spaced-repetition.github.io/ts-fsrs/
MIT License
213 stars 22 forks source link

[Question] why the seed includes review_time.getTime()? #131

Open L-M-Sherlock opened 1 month ago

L-M-Sherlock commented 1 month ago

https://github.com/open-spaced-repetition/ts-fsrs/blob/aaa154376876da9dc8176daddcd22335ba5318ec/src/fsrs/abstract_scheduler.ts#L91-L96

Does it mean I will get different interval if I review the same card in the same date?

ishiko732 commented 1 month ago

Does it mean I will get different interval if I review the same card in the same date?

Yes, when the interval > 2.5, it is possible to get different intervals on the same day due to variations in seconds. The initial idea was based on the conditions at that moment.

To maintain the same interval within the same day, you can use time % 86400000 (1 day).

L-M-Sherlock commented 1 month ago

The fuzz seed implemented in Anki is generated from card id and the number of reviews of the card. The seed will not change if the user doesn't review it. It's a good design because the fuzz factor will not change over time.

fn get_fuzz_seed_for_id_and_reps(card_id: CardId, card_reps: u32) -> Option<u64> {
    if *crate::PYTHON_UNIT_TESTS || cfg!(test) {
        None
    } else {
        Some((card_id.0 as u64).wrapping_add(card_reps as u64))
    }
}

source: https://github.com/ankitects/anki/blob/f00211df359f8214e9e97688776876a724f59266/rslib/src/scheduler/answering/mod.rs#L602-L609

The current implementation of fuzz seed in ts-fsrs can cause a weird case: the interval may decrease over time. For example, if the original interval is 100 days, and the fuzz factor is 1 in the first day, the fuzzed interval would be ~110 days. If I don't review it and delay the review to the next day, based DSR model, the original interval will increase to ~105 days. But the fuzz factor may decrease to 0, then the fuzzed interval would be ~95 days.

ishiko732 commented 1 month ago

Anki is generated from card id and the number of reviews of the card.

Since the card type in ts-fsrs does not have a card_id, making it incompatible with Anki's approach, should I remove the time variable?

L-M-Sherlock commented 1 month ago

Removing time will solve the aforementioned problem, but cause new problem. this.current.difficulty * this.current.stability is not random if the card's review logs are the same. I haven't figured out any good solution without introducing new property into Card object.

ishiko732 commented 1 month ago

I haven't figured out any good solution without introducing new property into Card object.

How about directly exposing init_seed, allowing developers to implement it themselves? If init_seed is not implemented, it defaults to the current situation.

jcyuan commented 2 weeks ago

personally i think a card has an id is reasonable. why this solution not work for ts-fsrs?

ishiko732 commented 2 weeks ago

personally i think a card has an id is reasonable. why this solution not work for ts-fsrs?

For related details, please see: https://github.com/open-spaced-repetition/go-fsrs/pull/3#issuecomment-1330685654

jcyuan commented 2 weeks ago

personally i think a card has an id is reasonable. why this solution not work for ts-fsrs?

For related details, please see: open-spaced-repetition/go-fsrs#3 (comment)

i got it, thank you. so maybe consider some soltuions like providing base virtual method to override by users.

ishiko732 commented 6 days ago

so maybe consider some soltuions like providing base virtual method to override by users.

Plan to use the proxy design pattern to implement this feature, which allows developers to use Reflect.get to retrieve the card.card_id passed into the repeat/next method and permits overriding the initSeed method.

jcyuan commented 5 days ago

so maybe consider some soltuions like providing base virtual method to override by users.

Plan to use the proxy design pattern to implement this feature, which allows developers to use Reflect.get to retrieve the card.card_id passed into the repeat/next method and permits overriding the initSeed method.

oh, i didn't realize there is Reflect.get method, actually i can hold a reference of the parameters passed to the methods, but with this the way is more elegant, yes. thanks Ishiko.