-
Notifications
You must be signed in to change notification settings - Fork 9
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
[wordvault] Use recall rate instead of lapses, refactor stats code structure #508
Conversation
@@ -0,0 +1,18 @@ | |||
export type ParsedFsrsCardStats = { |
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.
Not sure if we can get this type def from the back-end codegen, but based on how things were being implement right now I assume not
showTime = false, | ||
}: CardRecallStatsProps) { | ||
// The first time a card is incorrectly answered is not logged as a | ||
// lapse, so we exclude that from the calculation of recall rate. |
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.
This is not entirely true. If you answer it incorrectly after getting it right the very first time you see it, that counts as a lapse. If you answer it incorrectly the first time, and then answer it correctly the second time, you have no lapses. So subtracting 1 always is not quite 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.
Ahh I see, in that case I don't think we can get to recall % without a back-end change as well, right? probably not worth the effort then relative to other tasks I was gonna take a look at
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.
yeah, I don't think we can get recall % without keeping track of the actual number of times the question has been answered wrong. The best way I can think of getting this is from the log array that gets stored with each question, and just counting the incorrect entries. Or we can cache that number in that JSON blob too. But probably not worth it for right now.
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.
Looping back, how certain are you that this is correct?
If you answer it incorrectly the first time, and then answer it correctly the second time, you have no lapses
I'm pretty certain I have a non-zero number of cards I've never gotten right that show Times forgotten: {times seen - 1}
That comment might have been made before I figured out how it fully worked.
It should be changed.
To summarize, a card is considered not learned until you see it at least
once. So if you get it right the first time or you get it wrong, it doesn’t
start counting lapses until at least the second time.
…On Tue, Jan 14, 2025 at 8:40 AM Ben Muschol ***@***.***> wrote:
***@***.**** commented on this pull request.
------------------------------
In frontend/src/stats/card_recall_stats.tsx
<#508 (comment)>:
> + card: ParsedFsrsCardStats;
+ showTime?: boolean;
+ textProps?: Exclude<TextProps, "component">;
+ valueProps?: Exclude<TextProps, "component">;
+ excludeStats?: Set<CardRecallStat>;
+}
+
+export function CardRecallStats({
+ card,
+ textProps,
+ valueProps,
+ excludeStats = new Set(),
+ showTime = false,
+}: CardRecallStatsProps) {
+ // The first time a card is incorrectly answered is not logged as a
+ // lapse, so we exclude that from the calculation of recall rate.
Looping back, how certain are you that this is correct?
If you answer it incorrectly the first time, and then answer it correctly
the second time, you have no lapses
I'm pretty certain I have a non-zero number of cards I've never gotten
right that show Times forgotten: {times seen - 1}
—
Reply to this email directly, view it on GitHub
<#508 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAEO4ZTFJFMPA5U6XT26ZOL2KUHUPAVCNFSM6AAAAABVCWGV5CVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZDKNBZG4ZDQMJVGU>
.
You are receiving this because you commented.Message ID:
***@***.***>
|
Gotcha, I think the scheduler seems to update |
That code is actually very difficult for me to read and I don't know why they structured it that way. All I know is based on stepping through it with a debugger, and watching the behavior of it. Somehow it avoids incrementing the number of lapses the very first time that it gets asked and marked wrong. Probably the card is in a different "state" and some other branch executes. |
sg! I'll close this PR for now and switch my go-learning efforts to the short-term scheduler config |
Closes #504
src/frontend
directory a bit, so refactored to add astats
top-level dir:CardStats
component toStatisticsPage
CardRecallStats
component contains just the recall rate/scheduling stats./date_string
utility function to share between a couple componentsNo strong opinion on any of these refactors to feel free to request some (or all) of them get dropped from this PR