[Bug] Card first studied today is due today as review card after undo rescheduling

[user1823]

After I rescheduled my cards using the helper (after updating the parameters), 11 of the 50 new cards that I studied today and passed through the learning steps were due today as review cards.

I think that it has something to do with undo rescheduling because I clicked undo rescheduling before rescheduling my cards. Also, if I do undo rescheduling again, 36 of the 50 new cards become due today.

Card info of one card after rescheduling:

Card info of the same card after undo rescheduling:

[user1823]

The reason I did undo rescheduling was that there were some cards that had got a very absurd due date (screenshot below).

[L-M-Sherlock]

The reason I did undo rescheduling was that there were some cards that had got a very absurd due date (screenshot below).

Could you share the card info before rescheduling?

[user1823]

The reason I did undo rescheduling was that there were some cards that had got a very absurd due date (screenshot below).

Could you share the card info before rescheduling?

Sorry, but I don't have the previous card info of the cards with these absurd due dates. The reason is that they were last rescheduled several days ago but I don't know exactly when.

[L-M-Sherlock]

Are those cards in filtered decks?

[user1823]

Are those cards in filtered decks?

No

[L-M-Sherlock]

After I rescheduled my cards using the helper (after updating the parameters), 11 of the 50 new cards that I studied today and passed through the learning steps were due today as review cards.

What's your expected behavior?

[user1823]

After I rescheduled my cards using the helper (after updating the parameters), 11 of the 50 new cards that I studied today and passed through the learning steps were due today as review cards.

What's your expected behavior?

After passing the learning steps, the interval should be atleast 1 day. In fact, the card info shows that the interval is 1 day, but the due date is 28th May (i.e. today).

[L-M-Sherlock]

The code of resetting is here:

fsrs4anki-helper/utils.py

Lines 187 to 195 in b9657e5

	def reset_ivl_and_due(cid: int, revlogs: List[RevlogEntry]):
	card = mw.col.get_card(cid)
	card.ivl = int(revlogs[0].interval / 86400)
	due = int(round((revlogs[0].time + revlogs[0].interval - mw.col.sched.day_cutoff) / 86400) + mw.col.sched.today)
	if card.odid:
	card.odue = due
	else:
	card.due = due
	card.flush()

Maybe I need to replace round with ceil?

[L-M-Sherlock]

Could you share these cards with me? I need to do some tests.

[L-M-Sherlock]

Patch: fsrs4anki-helper.zip
Commit: 131f39c

[L-M-Sherlock]

My fault. The mw.col.sched.day_cutoff is the date of the next day. I plus 1 day for the resetting cards. Now it works normally.

[user1823]

It doesn't seem like the problem has been solved. After undo rescheduling with this version, the situation is the same as that with the earlier version.

I think that you made a mistake. I think that

fsrs4anki-helper/utils.py

Line 190 in 131f39c

due = int(round((revlogs[0].time + revlogs[0].interval - mw.col.sched.day_cutoff + 1) / 86400) + mw.col.sched.today)

should be replaced by

due = int(round((revlogs[0].time + revlogs[0].interval - mw.col.sched.day_cutoff) / 86400) + 1 + mw.col.sched.today)

[user1823]

By the way, if you still need my collection for testing, here it is: Default.apkg.zip

[user1823]

due = int(round((revlogs[0].time + revlogs[0].interval - mw.col.sched.day_cutoff) / 86400) + 1 + mw.col.sched.today)

After making this change, the problem seems to have been solved.

But, one question still remains: Even if the undo rescheduling function made an error, why did the rescheduling function not correct that?

[user1823]

But, one question still remains: Even if the undo rescheduling function made an error, why did the rescheduling function not correct that?

I got it. It is because of the following code:

fsrs4anki-helper/reschedule.py

Lines 248 to 256 in b9657e5

	new_ivl = [again_ivl, hard_ivl, good_ivl, easy_ivl][last_rating - 1]
	offset = new_ivl - card.ivl
	card.ivl = new_ivl
	due_before = card.odue if card.odid else card.due
	if card.odid: # Also update cards in filtered decks
	card.odue += offset
	else:
	card.due += offset
	due_after = card.odue if card.odid else card.due

What about using the following? Is there any problem with using this?

card.due = last_date + new_ivl

[L-M-Sherlock]

I got it. It is because of the following code:

Is this code wrong? I copy them from another add-on.

[user1823]

I got it. It is because of the following code:

Is this code wrong? I copy them from another add-on.

The code is not wrong. But it can lead to problems when the original card due date, card interval and last review date are not matching.

Logically, before the add-on makes any modification, the following equation should be true:

card.due = last_date + card.ivl

But, in certain cases, such as the present case where the helper assigned incorrect due date on undo rescheduling, the above equation is not true and this causes problems.

In my opinion, the code that I suggested would not cause any problem.

[L-M-Sherlock]

OK, I will implement it.

[L-M-Sherlock]

Patch: fsrs4anki-helper.zip
Commit: 48bae04

[user1823]

Patch: fsrs4anki-helper.zip
Commit: 48bae04

It looks and works fine.

What is the purpose of the following two lines of code? I think that they are unused and can be removed.

fsrs4anki-helper/reschedule.py

Line 251 in 48bae04

due_before = card.odue if card.odid else card.due

fsrs4anki-helper/reschedule.py

Line 256 in 48bae04

due_after = card.odue if card.odid else card.due

Another thing:
It seems that disperse_siblings.py, postpone.py and advance.py have the same problem.

fsrs4anki-helper/disperse_siblings.py

Lines 143 to 149 in 48bae04

	if card.odid:
	offset = card.odue - due
	card.odue = due
	else:
	offset = card.due - due
	card.due = due
	card.ivl = card.ivl - offset

fsrs4anki-helper/postpone.py

Lines 84 to 89 in 48bae04

	offset = new_ivl - card.ivl
	card.ivl = new_ivl
	if card.odid: # Also update cards in filtered decks
	card.odue += offset
	else:
	card.due += offset

fsrs4anki-helper/advance.py

Lines 84 to 89 in 48bae04

	offset = new_ivl - card.ivl
	card.ivl = new_ivl
	if card.odid: # Also update cards in filtered decks
	card.odue += offset
	else:
	card.due += offset

I think that you should define a common function for setting the due date. But, note that the disperse_siblings.py would require a separate function because it sets the interval by using the due date (instead of the vice-versa).

[L-M-Sherlock]

What is the purpose of the following two lines of code? I think that they are unused and can be removed.

It is used for load balance. Load Balance should track the review count on each due date.

[L-M-Sherlock]

It seems that disperse_siblings.py, postpone.py and advance.py have the same problem.

disperse_siblings is special here. I will implement it for advance and postpone first.

[L-M-Sherlock]

Patch: fsrs4anki-helper.zip
Commit: 9a81771

The disperse_siblings would not be modified because we need to compare the original due of siblings.

[user1823]

Patch: fsrs4anki-helper.zip
Commit: 9a81771

The code looks good. I will test it practically tomorrow.

The disperse_siblings would not be modified because we need to compare the original due of siblings.

I think that it can be modified to the following:

 if card.odid:
     card.odue = due 
 else: 
     card.due = due 
 card.ivl = due - last_review_date

[L-M-Sherlock]

I think that it can be modified to the following:

 if card.odid:
     card.odue = due 
 else: 
     card.due = due 
 card.ivl = due - last_review_date

It will not change anything. If the original due is wrong, the result of desperse_siblings will be wrong, too. Because desperse_siblings doesn't calculate the new due date for siblings. It combines the possible due date based on the cards' stability and fuzz range and finds the most sparse combination.

[user1823]

It will not change anything. If the original due is wrong, the result of desperse_siblings will be wrong, too.

That's not true.

In the following code, desperse_siblings calculates the due ranges from the stability, requestRetention and fuzz. This doesn't depend upon the original due.

fsrs4anki-helper/disperse_siblings.py

Lines 78 to 87 in 48bae04

	new_ivl = int(round(stability * math.log(parameters['r']) * easy_bonus / math.log(0.9)))
	if new_ivl <= 2.5:
	return range(due, due + 1), last_due
	elapsed_days = int((revlogs[0].time - revlogs[1].time) / 86400) if len(revlogs) >= 2 else 0
	min_ivl, max_ivl = get_fuzz_range(new_ivl, elapsed_days)
	step = max(1, math.floor((max_ivl - min_ivl) / (4 if siblings_cnt <= 4 else 2)))
	due_range = range(last_due + min_ivl, last_due + max_ivl + 1, step)
	if due_range.stop < mw.col.sched.today:
	due_range = range(due, due + 1)
	return due_range, last_due

Then, in the following code, it sets the due and ivl of the card to the best value calculated. So, if we improve this code, this part would also become independent of the original due.

fsrs4anki-helper/disperse_siblings.py

Lines 141 to 149 in 48bae04

	for cid, due in best_due_dates.items():
	card = mw.col.get_card(cid)
	if card.odid:
	offset = card.odue - due
	card.odue = due
	else:
	offset = card.due - due
	card.due = due
	card.ivl = card.ivl - offset

By the way, I also found another problem. In the following code, the last review date is calculated using the due and ivl.

fsrs4anki-helper/disperse_siblings.py

Lines 66 to 71 in 48bae04

	ivl = card.ivl
	if card.odid:
	due = card.odue
	else:
	due = card.due
	last_due = due - ivl

This can be updated to something like the following:

last_due = get_last_review_date()

Also, I think that last_due should be renamed to last_review_date.

[user1823]

You might be wondering what is the purpose of all these changes. So, let me explain.

The due and ivl are mutable properties. So, they can be easily changed to something absurd by other add-ons or even the FSRS Helper add-on (as in the current issue).

On the other hand, the information in the revlogs is immutable, which is set by Anki during review. So, we should use this information because it is more reliable.

[L-M-Sherlock]

Patch: fsrs4anki-helper.zip
Commit: fa5a4bd

[user1823]

It works fine. Thanks!

[L-M-Sherlock]

That's OK. I will merge this branch tomorrow. If you happen to encounter any bugs, please report them here. I have yet to test it a lot.

[user1823]

The only thing I noticed is some sluggishness. But, I am not sure if it is caused by these changes (it may be because my computer is running slow due to some other reason).

[L-M-Sherlock]

The only thing I noticed is some sluggishness. But, I am not sure if it is caused by these changes (it may be because my computer is running slow due to some other reason).

Your feeling is true. Because the new implementation requires extra access to the database for query of revlogs.