Closed kcroker closed 8 years ago
@timrae Personally, I don't mind if the changes go straight into Stats.java. I believe it already deviates a lot from the original python (I've barely looked at it), and it's a fairly standalone module that doesn't impact the rest of libanki.
The stats were basically a code dump and nobody has bothered to work on it since then. I have no plans or desire to maintain it to the same standards as the rest of libanki, so I don't mind letting whoever wants to work on it do whatever they want with it.
@jvanprehn
Thanks, I think this is looking good! Looking at the branch diff, isn't the method private ArrayList<int[]> calculateDues(int type)
unused?
Also, you can remove the // JPR was here
type comments in the final pull request, as comments should only be used to describe the functionality :) Please add the standard GPL spiel that's at the top of all the other AnkiDroid classes together with your name to the new hook file and metaclass too, along with a bit of javadoc describing what the main new classes do.
@hssm
Yeah I know the stats module may never be maintained to the same standard as the rest of libanki, but I personally feel that's all the more reason to not add any more bells and whistles to it. The more features (especially non-standard ones) that are added, the less likely it becomes for one of us to be willing to fix a bug.
I think we will eventually have a plugins framework one day, and this is quite clearly plugin material, so I like the way @jvanprehn has implemented it in his latest commit, as it should let us move it around or pull it out into a plugin without even having to really think about it.
@timrae You're right, those are good points.
Okay; I'll just leave it as a hook then which communicates with stats.java using the metaclass. I gave it a little bit more thought. Maybe I can remove some variables from the metaclass and determine them in stats.java to make the interface as small as possible so less likely that something changes which is in the interface between stats.java and the hook.
I forgot to remove private ArrayList<int[]> calculateDues(int type). Thanks for pointing that out. // JPR will be removed just before I do the pull request. Before that I will still need to do a couple of things like the GPL spiel, javadoc, removing hard-coded variables etc. (see previous posts). I'll also do an Analyze to see if I forgot to remove more stuff which isn't used. I find the JPR thing useful while coding since it allows me to jump between changes more quickly than doing diffs.
For now I did a couple of things:
Screenshots:
Looking good, thanks for your hard work on this, I'm looking forward to reviewing the PR. On 3/01/2016 5:08 am, "jvanprehn" notifications@github.com wrote:
Okay; I'll just leave it as a hook then which communicates with stats.java using the metaclass. I gave it a little bit more thought. Maybe I can remove some variables from the metaclass and determine them in stats.java to make the interface as small as possible so less likely that something changes which is in the interface between stats.java and the hook.
I forgot to remove private ArrayList calculateDues(int type). Thanks for pointing that out. // JPR will be removed just before I do the pull request. Before that I will still need to do a couple of things like the GPL spiel, javadoc, removing hard-coded variables etc. (see previous posts). I'll also do an Analyze to see if I forgot to remove more stuff which isn't used. I find the JPR thing useful while coding since it allows me to jump between changes more quickly than doing diffs.
For now I did a couple of things:
- Support year view and life view
- Display the number of cards in each state with lines in the same forecast graph
Screenshots: [image: anki_forecast_cumulative_days] https://cloud.githubusercontent.com/assets/16454627/12075941/877966ca-b194-11e5-91a0-787b09ec8bf6.png [image: anki_forecast_cumulative_weeks] https://cloud.githubusercontent.com/assets/16454627/12075943/8f50198e-b194-11e5-8565-8b254dc9543b.png [image: anki_forecast_cumulative_months] https://cloud.githubusercontent.com/assets/16454627/12075944/929be636-b194-11e5-93e9-2a172dd04923.png
— Reply to this email directly or view it on GitHub https://github.com/ankidroid/Anki-Android/issues/3619#issuecomment-168425242 .
Is there an existing bug which changes the deck option 'maximum reviews/day' (and maybe others) for all decks when it is changed for one deck?
I saw this when testing the statistics. I'm now reading the deck options so that I can take the max. number of new cards and the max. number of reviews per day for doing the simuations.
Then I saw that changing the option in one deck also changed the statistics of the other deck.
I first thought I was doing something wrong in the statistics module. But then I saw that it also happens in the deck picker.
The number of cards to review today changed to 40 for both decks, but I only changed it for one of the decks.
Great! It works: Max # cards to review = 45 for deck 'Inburgering':
Max # cards to review = 30 for deck 'Bahasa Sunda Dian':
All decks shows the sum of the two:
Implemented traversal of the future-review-tree until the probability of reaching a particular node drops below 1 minus a pre-configurable threshold or until it reaches a configured # of days in the future, whichever comes first.
While implementing this, I noticed that taking the max # reviews per day limit into account (which I implemented earlier) violates one of the assumpions of gregreen's algorithm: cards should be independent.
Doing it correctly would mean simulating possible combinations of reviews over cards (as far as I thought about it). And that is expensive! So I won't do it unless somebody comes up with something smart which makes it fast.
However, I think I'm going to stop adding features for now and work towards a pull-request next weekend. Otherwise it will never be finished :).
Pull request: https://github.com/ankidroid/Anki-Android/pull/4065
@jvanprehn You're a goddamn beast! Well done!
On Sun, Jan 24, 2016 at 6:07 PM, jvanprehn notifications@github.com wrote:
Pull request: #4065 https://github.com/ankidroid/Anki-Android/pull/4065
— Reply to this email directly or view it on GitHub https://github.com/ankidroid/Anki-Android/issues/3619#issuecomment-174312121 .
I found some time to start working on the Anki Desktop plugin.
It's based on the code by gregreen.
The result so far is here, but it's still a work in progress: https://github.com/jvanprehn/AnkiStatistics/blob/anki-desktop-statistics/src/Forecast_Simulate.py
Todo's are:
I'm posting this here because I wrote in a previous post that I would do the desktop plugin.
Cool, nice work!
Thanks for the great work!
One thing I've noticed in beta testing of the statistics feature (on Android) is that it doesn't take the new cards already studied today into account when determining how many new cards should be added today. That is, if you've already studied 7 new cards today, and your limit is 10 per day, the simulator still assumes 10 additional new cards will be studied today (instead of 3, in this example). Other than that, the feature works great though!
To add to that (android), if a parent deck has new cards to review set to 10, and a child decks new cards to review is 0, even if there are no new cards in that child deck, the advanced statistics will show you have 10 new reviews to do that day.
As it currently stands, the Forecast statistic does not propogate previous performance trends when determining hypothetical future card loads. E.g. if a user on average, and with small deviation, pushes 60% of the daily load at Hard, 20% at Good, and 10% Easy, and 10% Again; then forecast can easily intelligently anticipate card loads. At present, it does not seem to do this at all, nor does it display the anticipated new daily alottment, and so Forecast is not useful at all for determining effective review-only periods over which one can reduce the daily card load.