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/optimize card priority calculation and fix learn span behavior #263

Conversation

L-M-Sherlock
Copy link
Member

This PR includes several improvements to the FSRS simulator:

  1. Enhanced card priority calculation:
    • Modified priority tuple to use i32 instead of usize
    • Adjusted difficulty scaling for accuracy in priority calculation
    • Added filtering for cards beyond learn span
  2. Added new test case:
    • Validates that changing learn span doesn't affect review counts per day in the same date

Copy link
Member

@Luc-Mcgrady Luc-Mcgrady left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The learn_span still effects the earlier cards in my deck.
The test fails if you up the numbers high enough

image

I'll dm you a deck if the test isn't enough.

src/optimal_retention.rs Outdated Show resolved Hide resolved
L-M-Sherlock and others added 2 commits December 23, 2024 23:12
@L-M-Sherlock
Copy link
Member Author

The inconsistency starts from here:

image

@L-M-Sherlock
Copy link
Member Author

@Luc-Mcgrady I hope the latest commit solve your problem.

@Luc-Mcgrady
Copy link
Member

image

It's still weird in Anki but I can't get the test to fail. I'm guessing this might be a problem with anki's code?

@L-M-Sherlock
Copy link
Member Author

It's still weird in Anki but I can't get the test to fail. I'm guessing this might be a problem with anki's code?

Could you use dbg to print the input args from your Anki?

@Luc-Mcgrady
Copy link
Member

Luc-Mcgrady commented Dec 23, 2024

[rslib/src/scheduler/fsrs/simulator.rs:32:9] &req = SimulateFsrsReviewRequest {
    params: [
        0.08572003,
        0.40419477,
        4.1386,
        19.5147,
        5.3538256,
        1.4269736,
        0.9058566,
        8.294402e-5,
        1.3015078,
        0.016733568,
        0.6890899,
        2.0352721,
        0.15819947,
        0.2383912,
        1.4175994,
        0.027590245,
        2.8237,
    ],
    desired_retention: 0.8,
    deck_size: 0,
    days_to_simulate: 10,
    new_limit: 0,
    review_limit: 200,
    max_interval: 36500,
    search: "preset:\"Geography\" -is:suspended",
}
[rslib/src/scheduler/fsrs/simulator.rs:32:9] &req = SimulateFsrsReviewRequest {
    params: [
        0.08572003,
        0.40419477,
        4.1386,
        19.5147,
        5.3538256,
        1.4269736,
        0.9058566,
        8.294402e-5,
        1.3015078,
        0.016733568,
        0.6890899,
        2.0352721,
        0.15819947,
        0.2383912,
        1.4175994,
        0.027590245,
        2.8237,
    ],
    desired_retention: 0.8,
    deck_size: 0,
    days_to_simulate: 20,
    new_limit: 0,
    review_limit: 200,
    max_interval: 36500,
    search: "preset:\"Geography\" -is:suspended",
}

image

It does set the due date of certain cards to the limit but I cant see how that would effect anything

@L-M-Sherlock
Copy link
Member Author

The due affects the priority. Maybe it causes the inconsistency.

@L-M-Sherlock
Copy link
Member Author

Weird. These cards are new, so they are filtered in the simulator:

if let Some(existing_cards) = existing_cards {
cards.extend(
existing_cards
.into_iter()
.filter(|card| card.stability > 1e-9),
);

@Luc-Mcgrady
Copy link
Member

image

Well I just copied the entire output of my deck into the rs file, and besides making my IDE cry I think it proves its not Anki's problem

src/optimal_retention.rs Outdated Show resolved Hide resolved
@L-M-Sherlock
Copy link
Member Author

Nice! Thanks for your assistance in fixing the bug~

@L-M-Sherlock L-M-Sherlock enabled auto-merge (squash) December 24, 2024 02:14
@L-M-Sherlock L-M-Sherlock merged commit 7da138a into main Dec 24, 2024
3 checks passed
@L-M-Sherlock L-M-Sherlock deleted the Fix/optimize-card-priority-calculation-and-fix-learn-span-behavior branch December 24, 2024 02:14
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.

2 participants