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

[Feature Request] Keep delta_t as floats #437

Closed
Expertium opened this issue Aug 19, 2023 · 11 comments
Closed

[Feature Request] Keep delta_t as floats #437

Expertium opened this issue Aug 19, 2023 · 11 comments

Comments

@Expertium
Copy link
Collaborator

          Other day I was thinking that it might be worth to keep review intervals as floats, as opposed to calculating the day boundary and rounding them to days. Apart from helping those who travel a lot, it will remove necessity to enter timezone and day start, which is annoying an error prone, and also, it might improve accuracy for short interval reviews. Common sense suggests that if I learned a new card in evening and then reviewed it next day morning, I'll have better chance of remembering it if I was to learn it in morning and review it next day evening, however to the optimizer these two intervals are the same. I think this is also related to the spike on the stability to retention graph, which is at around 0.2 in my case. Despite this, I don't know if this change will bring any practical improvement.

image

Originally posted by @nb9618 in #429 (comment)

@Expertium
Copy link
Collaborator Author

          No, I mean `delta_t` and `t_history`. 

This obviously won't affect the scheduling as it is impossible to schedule a card at a specific time of a day, but it might improve accuracy of w and, if implemented in the add-on, accuracy of cards' memory states.

Originally posted by @nb9618 in #429 (comment)

@L-M-Sherlock
Copy link
Member

And the scheduler also only knows the elapsed days, which is an integer.

@L-M-Sherlock L-M-Sherlock changed the title [Feature Request] Kepp delta_t as floats [Feature Request] Keep delta_t as floats Aug 19, 2023
@user1823
Copy link
Collaborator

And the scheduler also only knows the elapsed days, which is an integer.

But things are changing. When the scheduler will get integrated into Anki's rust backend, several limitations would be removed. I realise that all this would not happen very soon. But, while discussing things like this one, we should think of the long term.

@dae
Copy link

dae commented Aug 27, 2023

I wonder whether it's really that useful, as once intervals grow above a few days, the fractional part is rather meaningless. For the training portion, revlog entries record days, so you'd need to infer the actual elapsed time based on the timestamps. For reviewing, the way the code is currently structured, only days are available to the scheduler.

@L-M-Sherlock
Copy link
Member

@ghost
Copy link

ghost commented Oct 9, 2023

I was thinking about how to differentiate between in-day and inter-day reviews when the information about day boundaries is not available. The easiest solution would be to pick a cut-off point, e.g. 12 hours, where any pair of reviews with the timestamp difference between them less than this point would be considered as having happened within the same day. There would be always a chance of misinterpreting corner cases, e.g. considering a review in the morning and then after 12 hours in the evening as having happened on different days, or a review late in the evening and then on the next day, in the early morning, as having happened on the same day. Depending on the chosen value of this point, the probability of these outcomes will change.

A more complicated approach would be to define something like an S-curve (note: here I mean the shape of the curve, and it has no relation to S as stability) with two end points, let's say 8 and 20 hours. This way, reviews with intervals of more than 20 hours, will be considered as inter-day, and with less than 8 hours as in-day, in which case S and D of the card after the later review will not be changed. For pairs of reviews within the range, the resulting S and D would be calculated as the intermediate values between the "previous" and the "updated as if it were an inter-day review" states, proportional to the value of the curve. If C = f(delta_t) is the value of the S-curve, and changes from 0 to 1 as delta_t increases, then stability after a "questionable" review will be S = C * S' + (1 - C) * S; and the same would stand for the difficulty. This way, there would be a smooth transition, as opposed to the simpler way of using a single cut-off point, which theoretically should alleviate the corner cases.

As a side note, I'm now convinced that with implementing float delta_t's there would probably be no significant improvement on the prediction accuracy for small stabilities, and taking into account the fact that the optimizer is now built into Anki, the only benefit of this change that I see is that it will improve calculation of memory states for people who change time zones.

@L-M-Sherlock
Copy link
Member

Algorithm Log Loss RMSE RMSE(bins)
FSRS v4 0.3820 0.3311 0.0547
FSRS v4 (float delta_t) 0.3870 0.3333 0.0557

The performance becomes worse.

@Expertium
Copy link
Collaborator Author

That's very strange. I wouldn't be very surprised if it was the same, but worse? That's unexpected.

@L-M-Sherlock
Copy link
Member

Sorry, I make a mistake in the pretrain stage. float delta_t is incompatible with the current pretrain implementation. So I round the delta_t in the pretrain stage. The final result is:

Algorithm Log Loss RMSE RMSE(bins)
FSRS v4 0.3820 0.3311 0.0547
FSRS v4 (float delta_t) 0.3840 0.3317 0.0542

Only RMSE(bins) is improved slightly.

@L-M-Sherlock
Copy link
Member

I think it is not very strange. Because the memory consolidation and forgetting mainly happen during sleep.

@L-M-Sherlock
Copy link
Member

Weighted by number of reviews

Algorithm Log Loss RMSE RMSE(bins)
LSTM float delta_t 0.4219 0.3431 0.0680
LSTM 0.4193 0.3424 0.0662

Weighted by ln(number of reviews)

Algorithm Log Loss RMSE RMSE(bins)
LSTM float delta_t 0.5886 0.3755 0.1387
LSTM 0.5934 0.3755 0.1382

It doesn't improve the performance to keep delta_t as floats. I think it's OK to close this issue.

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

No branches or pull requests

4 participants