-
-
Notifications
You must be signed in to change notification settings - Fork 22
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
compute retrievability for all states #61
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
if now is None: | ||
now = datetime.now(timezone.utc) | ||
|
||
if self.state in (State.Learning, State.Review, State.Relearning): | ||
elapsed_days = max(0, (now - self.last_review).days) | ||
return (1 + FACTOR * elapsed_days / self.stability) ** DECAY |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Setting self.stability = 0
will trigger an error.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
self.stability = 0 only when the state is new, right?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good point. I haven't known that. @joshdavham, does it happen in py-fsrs?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@ishiko732 Yeah, explicitly setting a card's stability to 0 would cause an error if the card is not in the New state. This would probably have to be done by the user though, which I'm not sure they would do. Stability is supposed to be computed internally by the code, not set by user.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@L-M-Sherlock You're asking if stability asymptotically approaches 0 when a card's rated again a bunch of times? Yeah it does
I'm not sure if it would every actually reach 0 and cause an error however
Currently, you can only get a card’s retrievability when it’s in the Review state. I thought it would be good enhancement if you could also compute the retrievability for cards in the other states as well.
Changes:
get_retrievability
function.datetime.now(timezone.utc)
similar to the Card class's__init__
method or the FSRS class'sreview_card
method.test_retrievability
to demonstrate this change worksLet me know if this makes sense or if you have questions 👍