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

Fix/buried new cards shouldn't be counted in daily load #3530

Conversation

L-M-Sherlock
Copy link
Contributor

@L-M-Sherlock L-M-Sherlock commented Oct 26, 2024

fix #3529, open-spaced-repetition/fsrs4anki-helper#480

New cards don't have memory state, so I exclude them in that way. And it's consistent with FSRS helper.

@user1823
Copy link
Contributor

What if you exclude cards having c.type = CARD_TYPE_NEW? This way, daily load will be useful for SM-2 users too.

@L-M-Sherlock
Copy link
Contributor Author

What if you exclude cards having c.type = CARD_TYPE_NEW? This way, daily load will be useful for SM-2 users too.

Because it will be inconsistent with the helper add-on. For details, please see:

@user1823
Copy link
Contributor

user1823 commented Oct 26, 2024

But, in that case (memory states lost due to changing deck), excluding these cards from the daily load makes the calculation inaccurate.

So, I would say that it is better to change the calculation in the helper add-on than making the calculation inaccurate in Anki too.

@L-M-Sherlock
Copy link
Contributor Author

Here is the related code:

https://github.com/open-spaced-repetition/fsrs4anki-helper/blob/4c7024612b1243c6089abbf44ff381c517b1ad24/stats.py#L25-L41

In the helper add-on, daily load and estimated total knowledge share the same query.

@user1823
Copy link
Contributor

If you think that changing the calculation in the add-on would be too difficult (because it is coupled with the other stats), just remove Daily Load from the add-on in 24.10+. Anki already has it and duplicating features is not necessary.

@brishtibheja
Copy link
Contributor

Is that necessary, a very tiny percentage of users use FSRS Helper who'll also notice discrepancies like this. The add-on feature is probably useful to those who wouldn't update to 24.10 version any time soon.

@L-M-Sherlock
Copy link
Contributor Author

The add-on feature is probably useful to those who wouldn't update to 24.10 version any time soon.

The add-on could know which version the user is running. If the version is greater or equal to 24.10, just hide daily load. If not, display it.

@user1823
Copy link
Contributor

Because the buried new cards shouldn't be included in any of the stats in this file, it might make sense to exclude those cards in the very beginning, i.e., just after for c in &self.cards {.

This won't affect the other stats. But, it might slightly improve the performance and prevent similar bugs in the future.

@dae
Copy link
Member

dae commented Oct 28, 2024

Further tweaks to this part of the code might be better done as part of #3379

@dae dae merged commit 0ce907f into ankitects:main Oct 28, 2024
1 check passed
@L-M-Sherlock L-M-Sherlock deleted the Fix/buried-new-cards-shouldn't-be-counted-in-daily-load branch October 28, 2024 04:26
L-M-Sherlock added a commit to open-spaced-repetition/fsrs4anki-helper that referenced this pull request Oct 29, 2024
user1823 added a commit to user1823/anki that referenced this pull request Nov 13, 2024
dae pushed a commit that referenced this pull request Nov 17, 2024
* Exclude new cards from Future Due stats

#3530 (comment)

Before 7ea573b, they were excluded anyway.

* Update future_due.rs

* Update comment

* Minor simplification
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.

Burying still affects daily load
4 participants