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

Sort by R after postponing #407

Merged
merged 1 commit into from
May 10, 2024

Conversation

user1823
Copy link
Contributor

Fixes #406

@user1823
Copy link
Contributor Author

The postpone function uses fuzz. So, I have taken the average when estimating the retention to prevent excessive calculations.

@user1823
Copy link
Contributor Author

In addition, I have replaced interval with stability in the second sort condition because stability is what actually decides how fast R falls.

@@ -38,8 +38,8 @@ def postpone(did):
ivl,
json_extract(data, '$.s'),
CASE WHEN odid==0
THEN {mw.col.sched.today} - (due - ivl)
ELSE {mw.col.sched.today} - (odue - ivl)
THEN {mw.col.sched.today} - (due - ivl) + ivl * 0.075
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's the purpose of plus ivl * 0.075?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have the same question as well.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is used to get an estimate of the elapsed days at the new due date.

The code for the new interval is this:

        delay = elapsed_days - ivl
        new_ivl = min(
            max(1, math.ceil(ivl * (1.05 + 0.05 * random.random())) + delay), max_ivl
        )

For the reason explained in #407 (comment), I assume this to be:

        delay = elapsed_days - ivl
        new_ivl = min(
            max(1, math.ceil(ivl * (1.05 + 0.025)) + delay), max_ivl
        )

If you put the value of delay, you get

        new_ivl = min(
            max(1, math.ceil(ivl * 0.075) + elapsed_days), max_ivl
        )

For the purpose of sorting, we can skip math.ceil and assume that the value is between 1 and max_ivl. So, we get new_ivl = ivl * 0.075 + elapsed_days

At due date, R = power_forgetting_curve(new_ivl, stability)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, interesting. That's a clever way to take fuzz into account.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

assume that the value is between 1 and max_ivl

If you don't want to make this assumption, we can change it to something like

THEN min({mw.col.sched.today} - (due - ivl) + ivl * 0.075, max_ivl)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok

Copy link
Contributor Author

@user1823 user1823 May 10, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The value not being > 1 is not actually a problem because it is elapsed_days + ivl * 0.075 and elapsed_days would always be at least equal to 1 (you can't postpone a learning card).

The main concern is that this value can sometimes become greater than the max_ivl but the add-on would not assign that value of interval. So, the retention estimate would become inaccurate.

@L-M-Sherlock L-M-Sherlock requested a review from Expertium May 10, 2024 13:15
Copy link
Member

@L-M-Sherlock L-M-Sherlock left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@L-M-Sherlock L-M-Sherlock merged commit 5b1ad66 into open-spaced-repetition:main May 10, 2024
1 check passed
@user1823 user1823 deleted the patch-1 branch May 10, 2024 17:57
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.

[Feature Request] Rework Advance and Postpone
3 participants