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

update optimal retention and parameters tooltip #3148

Merged

Conversation

L-M-Sherlock
Copy link
Contributor

I removed the words about the 1000 review limitation from the tooltip of FSRS parameters, and mentioned the 400 review limitation in the tooltip of optimal retention.

@dae
Copy link
Member

dae commented Apr 19, 2024

In this case, we'll probably want to 1) move the existing strings to the bottom of the file and 2) add new strings with an updated key (eg deck-config-weights-tooltip -> deck-config-weights-tooltip2). If the keys remain the same, translators won't be notified.

@L-M-Sherlock
Copy link
Contributor Author

OK. I will update the PR tonight.

ftl/core/deck-config.ftl Outdated Show resolved Hide resolved
ftl/core/deck-config.ftl Outdated Show resolved Hide resolved
ftl/core/deck-config.ftl Outdated Show resolved Hide resolved
L-M-Sherlock and others added 3 commits April 19, 2024 21:58
ftl/core/deck-config.ftl Outdated Show resolved Hide resolved
@user1823
Copy link
Contributor

user1823 commented Apr 22, 2024

I think that when the number of reviews is exactly 0 (which most likely implies an error in the search query), an error should be shown.

One example of a user entering an incorrect search query and not getting any feedback (if the same query was run in 24.04.2 beta): https://forums.ankiweb.net/t/why-cant-i-optmize-my-fsrs-parameters/43830/3

@L-M-Sherlock
Copy link
Contributor Author

One example of a user entering an incorrect search query and not getting any feedback

For this case, it's better to warn the user that the query doesn't match any reviews. But I don't know how to add a new warning popup in Anki. @dae, what do you think of? I hope you could help me.

@dae
Copy link
Member

dae commented Apr 22, 2024

Why don't we push that problem into a separate issue? We could take a similar approach to #3123 and display the item count somewhere, then users would know the search is wrong, without us having to make a separate error.

ftl/core/deck-config.ftl Outdated Show resolved Hide resolved
ftl/core/deck-config.ftl Outdated Show resolved Hide resolved
deck-config-compute-optimal-retention-tooltip3 =
This tool assumes that you’re starting with 0 learned cards, and will attempt to find the desired retention value
that will lead to the most material learnt, in the least amount of time. To accurately simulate your learning process,
this feature requires a minimum of 400+ reviews. The calculated number can serve as a reference when deciding what to
Copy link
Member

Choose a reason for hiding this comment

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

How likely is it that this number will change again? We should not be specific if there is a chance it will change again.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This number was not changed by me. Here is the discussion: #3019 (comment)

I don't have a plan to modify it in the future.

Copy link
Member

Choose a reason for hiding this comment

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

Are you happy with it at 400? Sorry, we should have checked with you at the time.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

400 is OK to me. And I plan to make the simulation less sensitive to outliers.

open-spaced-repetition/fsrs-optimizer#107

ftl/core/deck-config.ftl Outdated Show resolved Hide resolved
@user1823
Copy link
Contributor

Why don't we push that problem into a separate issue?

Yes, we can definitely move it to a new issue. I just thought that it would be easier to make all related changes at once.

We could take a similar approach to #3123 and display the item count somewhere, then users would know the search is wrong, without us having to make a separate error.

That would depend upon exactly how are you planning to display the count.

@dae
Copy link
Member

dae commented Apr 23, 2024

Looks good, pending that one question above.

@user1823

This comment was marked as resolved.

@L-M-Sherlock
Copy link
Contributor Author

L-M-Sherlock commented Apr 23, 2024

In a preset with 101 reviews, I can't evaluate in AnkiDroid (based on Anki 24.04) but I can evaluate in Anki 24.04.2 beta. Again, this PR prevents evaluation with less than 400 reviews. So, it seems that a lot of flip-flop is going on. It would be good if we make the final decision before altering the strings.

This PR doesn't prevent evaluation with less than 400 reviews.

Edit: I think that I misunderstood which feature this limit was introduced in this PR.

This PR doesn't introduce any limit.

@dae
Copy link
Member

dae commented Apr 24, 2024

Thanks all!

@dae dae merged commit 2c9accf into ankitects:main Apr 24, 2024
1 check passed
@L-M-Sherlock L-M-Sherlock deleted the update-optimal-retention-and-parameters-tooltip branch April 24, 2024 01:49
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.

3 participants