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

Limit by burden #1

Closed
aleksejrs opened this issue Feb 26, 2024 · 13 comments
Closed

Limit by burden #1

aleksejrs opened this issue Feb 26, 2024 · 13 comments

Comments

@aleksejrs
Copy link

A couple of day ago, I took the burden code from https://github.com/open-spaced-repetition/fsrs4anki-helper/blob/main/stats.py (MIT License) and added it to this add-on. It seemed to work.
The burden limit is used if present, else the young limit is used. It is slower, so use it only for some decks.
I don't understand SQL.

from aqt import mw, gui_hooks
from aqt.utils import qconnect
import aqt.qt as qt

import re
import threading
import time

def updateLimits(hookEnabledConfigKey=None, forceUpdate=False) -> None:
    addonConfig = mw.addonManager.getConfig(__name__)
    today = mw.col.sched.today

    if hookEnabledConfigKey and not addonConfig[hookEnabledConfigKey]:
        return

    for deckIndentifer in mw.col.decks.all_names_and_ids():
        deck = mw.col.decks.get(deckIndentifer.id)
        if deck['dyn'] == 1:
            continue # ignore 'Custom Study Session' style decks

        addonConfigLimits = None
        for limits in addonConfig["limits"]:
            if (isinstance(limits["deckNames"], str) and re.compile(limits["deckNames"]).match(deckIndentifer.name)) \
                or (isinstance(limits["deckNames"], list) and deckIndentifer.name in limits["deckNames"]):
                    addonConfigLimits = limits
                    break
        if not addonConfigLimits:
#            print("Limit New by Young: No user defined limits to apply.")
            continue # no user defined limits to apply

        limitAlreadySet = False if deck["newLimitToday"] is None else deck["newLimitToday"]["today"] == today

        if not forceUpdate and limitAlreadySet:
            continue

        deckConfig = mw.col.decks.config_dict_for_deck_id(deckIndentifer.id)

        lim = " AND did"

        try:
            burdenLimit = addonConfigLimits['burdenLimit']
            maxNewCardsPerDay = deckConfig['new']['perDay']
            burdenCount = burden("AND " + str(deckIndentifer.id))
            newLimit = max(0, min(maxNewCardsPerDay, burdenLimit - burdenCount))
        except KeyError:
            youngCardLimit = addonConfigLimits['youngCardLimit']
            maxNewCardsPerDay = deckConfig['new']['perDay']
            youngCount = len(list(mw.col.find_cards(f'deck:"{deckIndentifer.name}" prop:due<21 prop:ivl<21 -is:suspended')))
            newLimit = max(0, min(maxNewCardsPerDay, youngCardLimit - youngCount))

        print("BLimChange: {}: {}".format(deckIndentifer.name, newLimit))
        deck["newLimitToday"] = {"limit": newLimit, "today": mw.col.sched.today}
        mw.col.decks.save(deck)
        mw.reset()

def burden(lim) -> float:
    elapse_stability_ivl_list = mw.col.db.all(
        f"""
    SELECT 
        CASE WHEN odid==0
            THEN {mw.col.sched.today} - (due - ivl)
            ELSE {mw.col.sched.today} - (odue - ivl)
        END
        ,json_extract(data, '$.s')
        ,ivl 
        ,(SELECT COUNT(*) FROM cards c2 WHERE c1.nid = c2.nid)
        ,nid
    FROM cards c1
    WHERE queue >= 1 
    AND data != ''
    AND json_extract(data, '$.s') IS NOT NULL
    """
        + lim
    )
    # x[0]: elapsed days
    # x[1]: stability
    # x[2]: interval
    # x[3]: same nid count
    # x[4]: nid
    elapse_stability_ivl_list = filter(
        lambda x: x[1] is not None, elapse_stability_ivl_list
    )
    burden_list = list(
        map(
            lambda x: 1 / max(1, x[2]),
            elapse_stability_ivl_list,
        )
    )
    burden_sum = sum(burden_list)
    return burden_sum


def updateLimitsOnIntervalLoop():
    time.sleep(5 * 60) #HACK wait for anki to finish loading
    while True:
        addonConfig = mw.addonManager.getConfig(__name__)
        sleepInterval = max(60, addonConfig['updateLimitsIntervalTimeInMinutes'] * 60)
        time.sleep(sleepInterval)

        mw.taskman.run_on_main(lambda: updateLimits(hookEnabledConfigKey='updateLimitsOnInterval'))

updateLimitsOnIntervalThread = threading.Thread(target=updateLimitsOnIntervalLoop, daemon=True)
updateLimitsOnIntervalThread.start()

gui_hooks.main_window_did_init.append(lambda: updateLimits(hookEnabledConfigKey='updateLimitsOnApplicationStartup'))
gui_hooks.sync_did_finish.append(lambda: updateLimits(hookEnabledConfigKey='updateLimitsAfterSync'))

action = qt.QAction("Recalculate today's burden new card limit for all decks", mw)
#action = qt.QAction("Recalculate today's new card limit for all decks", mw)
qconnect(action.triggered, lambda: updateLimits(forceUpdate=True))
mw.form.menuTools.addAction(action)
@lune-stone
Copy link
Owner

I've created a branch that adds support for this feature. I plan to test it out on my own over the next few days just to make sure it works before I merge & upload it to ankiweb. If you are willing and free I would appreciate it if you could double check that it also works and matches your expectations.

I'm curious though. Since burden is also measured in reviews/day and is pretty close to the actual reviews for a day, what would be the motivation to use burden over the built in Maximum reviews/day limit with New cards ignore review limit disabled?

@aleksejrs
Copy link
Author

aleksejrs commented Feb 27, 2024

I am using it, but I have lots of decks with a total burden of 333 reviews/day and a backlog (2733 due now, 4578 more in 30 days).

Burden is an estimate for the whole future as currently known based on what the Future Due graph shows and the intervals — not just one day (especially not just today) and not with an actual simulation of future reviews. Take a look at some Future Due graphs here: open-spaced-repetition/fsrs4anki-helper#372

@aleksejrs
Copy link
Author

It seems like your code calls the original function that calculates not just the burden, but also retention and stability, which might take extra time.

@lune-stone
Copy link
Owner

lune-stone commented Feb 27, 2024

... and a backlog.

I had not considered this use case. Having the limit based on burden would make it possible to work through a backlog while preserving a fairly steady rate of new cards.

Did you make it calculate the burden even if burden limit is not specified? That will be very slow.

I didn't notice the slowness on my machine, but the way it's coded it should skip the calculation if the limit is not defined (or defined as a large value).

These are the lines of interest in the commit

burdenLimit = addonConfigLimits.get('burdenLimit', 999999999)
...
burden = 0 if burdenLimit > deck_size else else ...

It seems like your code calls the original function that calculates not just the burden, but also retention and stability, which might take extra time.

My thinking was it would be nice to use the exact calculation that drives the stats page rather than a copy should fsrs4anki-helper update how they do their calculation, but there are some downsides to this approach. Being slightly less optimal is one of those like you pointed out.

@aleksejrs
Copy link
Author

aleksejrs commented Feb 27, 2024

I had not considered this use case. Having the limit based on burden would make it possible to work through a backlog while preserving a fairly steady rate of new cards.

I am not sure how good that is, but the backlog distracts me from testing it correctly (I didn't limit the one deck that follows Duolingo).

And it turns out that in my code,

  • burdenCount = burden("AND " + str(deckIndentifer.id)) gets the burden for the whole collection, not the specific deck. (you wrote it right)
  • it can't set a limit because the result is a floating-point number (you wrote it right)
  • it doesn't check the burden of subdecks.

Sorry I didn't test it well.

@aleksejrs
Copy link
Author

A huge slowdown happens if the deck list is displayed and "Enhance main window" is installed, so it redraws the table with every change.

@user1823
Copy link
Contributor

user1823 commented Feb 27, 2024

My thinking was it would be nice to use the exact calculation that drives the stats page rather than a copy should fsrs4anki-helper update how they do their calculation

I am one of the main contributors to FSRS and I can tell you that the burden calculation is very unlikely to be changed. So, it is better to copy the code (with attribution) and make necessary changes to optimize it for your use.

To calculate the burden, you just need to query the intervals using the SQL i.e., no need to query the due, stability, count, etc.

@lune-stone
Copy link
Owner

it doesn't check the burden of subdecks.

I'm not familiar with how Anki does subdecks so I think I'll separate this out into it's own issue and revisit after I get the rest of this change sorted out.

A huge slowdown happens if the deck list is displayed and "Enhance main window" is installed, so it redraws the table with every change.

I think a larger contributor to this slowdown is my use of mw.reset() inside the loop. I've updated the branch to only call the method once when needed.

...the burden calculation is very unlikely to be changed.

Good to know, thank you telling me. I've updated the branch to use a copy of the logic with what a comment that I think works for proper attribution, let me know if it needs to be changed.

@user1823
Copy link
Contributor

You can remove the following conditions from the SQL; they are used in FSRS helper to include only the cards that have FSRS memory states.

    AND data != ''
    AND json_extract(data, '$.s') IS NOT NULL

@user1823
Copy link
Contributor

I said that the code is unlikely to change but this thread directed my attention to that part of the code and I ended up proposing changes. 😅

I have proposed two changes in open-spaced-repetition/fsrs4anki-helper#373.

@lune-stone
Copy link
Owner

lune-stone commented Feb 28, 2024

I've merged the PR from @user1823 to fix the burden calculation for subdecks to the branch.

... I ended up proposing changes.

No worries. I'll keep an eye on the case and if it gets merged I'll update this branch to match wording/logic prior to merging.

@aleksejrs
Copy link
Author

aleksejrs commented Mar 1, 2024

Are cards in learning counted? A backlog prevents learning of new cards when "New/review order" is "Show after reviews", but if cards in learning are not counted as burden, recalculating will restore the limit.

@user1823
Copy link
Contributor

user1823 commented Mar 1, 2024

Are cards in learning counted?

Yes

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

No branches or pull requests

3 participants