Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Memory states from SM-2 for incomplete review history #248

Conversation

user1823
Copy link
Contributor

@user1823 user1823 commented Sep 26, 2023

Context: open-spaced-repetition/fsrs-rs#63 (comment)

I think that I have added most of the code required to make it work. You will just need to do some final touches so that it can become functional. Specifically, you will need to edit line 391 and maybe minor changes to the other parts.

I hope that editing this PR requires less effort than writing all the code yourself.

user1823 and others added 3 commits September 26, 2023 20:18
Context: open-spaced-repetition/fsrs-rs#63 (comment)

I think that I have added most of the code required to make it work. You will just need to do some final touches so that it can become functional.

I hope that editing this PR requires less effort than writing all the code yourself.
@L-M-Sherlock
Copy link
Member

Patch: fsrs4anki-helper.zip

Could you help me test it?

@user1823
Copy link
Contributor Author

user1823 commented Sep 26, 2023

It didn't work, i.e., the card didn't get custom data.

For example, this card:
image

@L-M-Sherlock
Copy link
Member

Patch: fsrs4anki-helper.zip

I hope it solve the problem.

@user1823
Copy link
Contributor Author

It seems to work well, though I will test it more.

However, I am slightly curious as to why the interval decreased from 1.04 years to 1.01 years. However, this is not a problem.

image

@L-M-Sherlock
Copy link
Member

However, I am slightly curious as to why the interval decreased from 1.04 years to 1.01 years. However, this is not a problem.

It would be due to fuzz.

@user1823
Copy link
Contributor Author

I have tested this and it seems to be working fine. Thanks!

I am attaching some screenshots for future reference (or if you are interested in seeing them).

image image image image

@user1823
Copy link
Contributor Author

Before (left) After (right)
imageimage

Total Count increased by 87 cards. 🚀🚀🚀

@user1823
Copy link
Contributor Author

Before merging this, I advise you to check how this affects cards with the first rating as "Manual". Examples:


Source of images: https://forums.ankiweb.net/t/anki-23-10-beta/34912/18

I can't test this case because I don't have such cards.

@user1823
Copy link
Contributor Author

user1823 commented Sep 26, 2023

Also, we should test for the case where first review kind is FILTERED and "reschedule cards based on my answers in this deck" is enabled & disabled (both cases separately).

@L-M-Sherlock
Copy link
Member

Also, we should test for the case where first review kind is FILTERED and "reschedule cards based on my answers in this deck" is enabled & disabled (both cases separately).

If you enable "reschedule cards based on my answers in this deck", the review kind is REVIEW instead of FILTERED.

Before merging this, I advise you to check how this affects cards with the first rating as "Manual". Examples:

The second review is treated as the first learning.

@user1823
Copy link
Contributor Author

Also, we should test for the case where first review kind is FILTERED and "reschedule cards based on my answers in this deck" is enabled & disabled (both cases separately).

If you enable "reschedule cards based on my answers in this deck", the review kind is REVIEW instead of FILTERED.

It doesn't seem to be the case when the card is reviewed before it is due.

image

Before merging this, I advise you to check how this affects cards with the first rating as "Manual". Examples:

The second review is treated as the first learning.

Assuming that this is actually what happens, it looks good to me.

@L-M-Sherlock
Copy link
Member

It doesn't seem to be the case when the card is reviewed before it is due.

Weird. I read the doc of Anki, it said that:

/// Old Anki versions called this "Cram" or "Early", and assigned it when
/// reviewing cards ahead. It is now only used for filtered decks with
/// rescheduling disabled.

Source: https://github.com/ankitects/anki/blob/05499297e065b7536e701e105eb613a761b7c135/rslib/src/revlog/mod.rs#L65-L75

I need to ask @dae.

@dae
Copy link

dae commented Sep 28, 2023

That comment is wrong - it's also applied when a review card is answered early (eg not using is:due to create the filtered deck)

@L-M-Sherlock
Copy link
Member

Fine. Could I distinguish these two types of review via ease != 0?

@L-M-Sherlock L-M-Sherlock merged commit c17aae0 into open-spaced-repetition:main Sep 28, 2023
@user1823 user1823 deleted the Memory-states-from-SM-2-for-incomplete-review-history branch September 28, 2023 04:12
@dae
Copy link

dae commented Sep 28, 2023

Yes, I think so - the v2 scheduler didn't write revlog entries when not rescheduling, and v3 uses ease 0 when not rescheduling.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants