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

Small tau values cause failed to converge errors #20

Closed
LazerJesus opened this issue Sep 23, 2023 · 5 comments
Closed

Small tau values cause failed to converge errors #20

LazerJesus opened this issue Sep 23, 2023 · 5 comments

Comments

@LazerJesus
Copy link

hey,
i am working on my implementation and stumbled on a minor issue.
Any tau value smaller than 0.0608 + that time cant be computed.

try {
    const tau = 0.0607;
    const time = 3.56;
    ebisu.updateModel([4, 4, tau], 1, 1, time);
} catch (e) {
    console.log(e);
}

results in error:

394 |   let et;
395 |   if (rebalance) {
396 |     const status = {};
397 |     const sol = fmin((et2) => Math.abs(moment(1, et2) - 0.5), { lowerBound: 0 }, status);
398 |     if (!("converged" in status) || !status.converged) {
399 |       throw new Error("failed to converge");
                ^
error: failed to converge
      at _updateRecallSingle (/Users/finn/valence/code/spanish2/backend/node_modules/ebisu-js/dist/ebisu.cjs:399:12)
      at updateRecall (/Users/finn/valence/code/spanish2/backend/node_modules/ebisu-js/dist/ebisu.cjs:315:11)
      at /Users/finn/valence/code/spanish2/backend/src/games/spacedrepetition/updateNode.js:76:4
      at processTicksAndRejections (:55:76)

its an issue in an upstream library minimize-golden-section-1d I THINK (dont quote me on that).
anyways, no biggie for me just sending signal.

@LazerJesus
Copy link
Author

LazerJesus commented Oct 1, 2023

happended again. tau of .16 and .26 work. .24 not.
this time its actually a problem.

    const tau = 0.24;
    const time = 14.39;
    const model = [4, 4, tau];
    const update = ebisu.updateModel(model, 1, 1, time);
    
394 |   let et;
395 |   if (rebalance) {
396 |     const status = {};
397 |     const sol = fmin((et2) => Math.abs(moment(1, et2) - 0.5), { lowerBound: 0 }, status);
398 |     if (!("converged" in status) || !status.converged) {
399 |       throw new Error("failed to converge");
                ^
error: failed to converge
      at _updateRecallSingle (/Users/finn/valence/code/spanish2/backend/node_modules/ebisu-js/dist/ebisu.cjs:399:12)
      at updateRecall (/Users/finn/valence/code/spanish2/backend/node_modules/ebisu-js/dist/ebisu.cjs:315:11)
      at /Users/finn/valence/code/spanish2/backend/src/games/spacedrepetition/updateNode.js:102:19
      at processTicksAndRejections (:55:76)

@fasiha
Copy link
Owner

fasiha commented Oct 2, 2023

Thanks soooo much for reporting this and so sorry for the delay! Fixed in version 2.1.1—please try it out to confirm it works 😁 per the new unit tests, this shouldn't happen for much smaller tau/t ratios (the new unit test asserts that a quiz that's 100'000x the halflife will continue to work 💪)

@LazerJesus
Copy link
Author

LazerJesus commented Mar 11, 2024

$ ebisu.updateRecall([ 3.9951083696258847, 3.9951083696258847, 238402764.59541383 ], 1, 1, 1.1461897222222222);

> {
  iterations: 100,
  argmin: 221614253.88549274,
  minimum: 1.2212453270876722e-15,
  converged: false
} {
  prior: [ 3.9951083696258847, 3.9951083696258847, 238402764.59541383 ],
  result: 1,
  tnow: 1.0757555555555558,
  q0: 0,
  rebalance: true,
  tback: undefined
}
Error: failed to converge
    at _updateRecallSingle (/Users/finn/vivalence/code/spanish/app/frontend/node_modules/ebisu-js/dist/ebisu.cjs:420:13)
    at _updateRecallSingle (/Users/finn/vivalence/code/spanish/app/frontend/node_modules/ebisu-js/dist/ebisu.cjs:417:16)
    at Proxy.updateRecall (/Users/finn/vivalence/code/spanish/app/frontend/node_modules/ebisu-js/dist/ebisu.cjs:318:12)

getting the same error again. this time with larger tau values.

on 2.1.2

this isnt blocking me at all, just an fyi.

@fasiha
Copy link
Owner

fasiha commented Mar 12, 2024

@LazerJesus thanks for this even more crazy example 😂 I released v2.1.3 that gives you a backdoor to fix this: more details in #25 but

ebisu.updateRecall([4, 4, 1e9], 1, 1, 1.5) // 💣❌

will throw but

ebisu.updateRecall([4, 4, 1e9], 1, 1, 1.5, undefined, undefined, undefined, {tolerance: 1e-6}) // ✅✅

will now work. The default tolerance is 1e-8 and by relaxing it a bit, this extreme case converges. I decided to not change the default tolerance because I think this edge case is pretty rare but the backdoor is there if you ever need this.

One billion hours/days halflife 🤪

@LazerJesus
Copy link
Author

what can i say, i know this fact reeaaallllyyyy well 😆

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

2 participants