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.

Sign up for GitHub

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

Jump to bottom

Feat/simulator with existing cards and limit #141

Merged
merged 2 commits into from
Dec 21, 2023
Merged

Feat/simulator with existing cards and limit #141

merged 2 commits into from
Dec 21, 2023

Conversation

L-M-Sherlock
Copy link
Member

Due to the change of scheduler, the old simulator add-on becomes inaccurate. I want to provide a native simulator for FSRS.

In this PR, I add two features:

  • Accept existing cards from Anki
  • Set New cards/day and Maximum reviews/day like Anki

@dae, I know you are busy recently. Do you have a plan to support the native simulator in 2024? I hope my code could be helpful.

@L-M-Sherlock L-M-Sherlock added the enhancement New feature or request label Dec 20, 2023
dae
dae reviewed Dec 21, 2023
View reviewed changes
src/optimal_retention.rs Outdated Show resolved Hide resolved
@dae
Copy link
Collaborator

dae commented Dec 21, 2023

LGTM.

Do you have a plan to support the native simulator in 2024?

Do you mean including the changes from this PR into Anki, or do you mean something else?

@L-M-Sherlock
Copy link
Member Author

Do you mean including the changes from this PR into Anki, or do you mean something else?

Ideally, the feature could be supported by Anki natively. It includes a GUI to configure the simulation and display the result. But it would requires a lot of development. Alternatively, you can just export a API to python.

@L-M-Sherlock
Copy link
Member Author

L-M-Sherlock commented Dec 21, 2023 •
edited
Loading

And this PR changes the struct SimulatorConfig. You need to modify the related code in Anki.

            learn_limit: usize::MAX,
            review_limit: usize::MAX,

@L-M-Sherlock L-M-Sherlock merged commit 8ff6ccf into main Dec 21, 2023
3 checks passed
@L-M-Sherlock L-M-Sherlock deleted the Feat/simulator-with-existing-cards&learn&review-limit branch December 21, 2023 06:56
@dae
Copy link
Collaborator

dae commented Dec 22, 2023

I won't have time to include this in 23.12, but will include it in the release after that. Could you please clarify what learn_limit and review_limit represent? Are they intended to be the daily review and new card limits the user has set, or something else?

It includes a GUI to configure the simulation and display the result.

If fsrs-rs adds an API that outputs data suitable for rendering into a graph, I can help with integrating the input/graph part, but it will be a while until I have free time to do so.

dae added a commit to ankitects/anki that referenced this pull request Dec 22, 2023
@dae
Update FSRS
4908be6 
We're on the commit prior to
open-spaced-repetition/fsrs-rs#141, as that is
going to require work that will need to wait until after 23.12
@L-M-Sherlock
Copy link
Member Author

Could you please clarify what learn_limit and review_limit represent?

learn_limit is New cards/day and review_limit is Maximum reviews/day.

@L-M-Sherlock
Copy link
Member Author

L-M-Sherlock commented Dec 22, 2023 •
edited
Loading

Note: When we set max_cost_perday, the learn_limit and review_limit should be assigned to usize::MAX. When we set learn_limit and review_limit, the max_cost_perday should be assigned to usize::MAX.

@salmanuc salmanuc mentioned this pull request Feb 18, 2024
Closed
@salmanuc
Copy link

salmanuc commented Mar 2, 2024

Will this feature make to the 24.04 release? @dae

@dae dae mentioned this pull request Mar 3, 2024
Closed
@dae
Copy link
Collaborator

dae commented Mar 3, 2024

I've logged it on ankitects/anki#3046

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dae dae dae left review comments

@asukaminato0721 asukaminato0721 asukaminato0721 approved these changes

Assignees
No one assigned
Labels
enhancement New feature or request
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@L-M-Sherlock @dae @salmanuc @asukaminato0721