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

New Report: Balance Forecast #467

Closed
wants to merge 9 commits into from
Closed

New Report: Balance Forecast #467

wants to merge 9 commits into from

Conversation

ZDBioHazard
Copy link
Contributor

This report forecasts the combined balances of the selected accounts based on the scheduled transactions and plots them on a line graph.

You can set a "reserve" amount, which will draw a red line on the graph, so you can easily see if your forecast dips below a given value.

There is also a "future minimum" line which shows what the lowest future balance will be at a given point. I find this useful in conjunction with the "target" line for planning.


Let me know what you all think. Before this report, the only way I could find to predict future balances on a graph was to create scheduled transactions far in advance and use the "Average Balance" report.

I wanted to find a way to use each interval's minimum balance as a worst-case scenario for paycheck-to-paycheck budgeting, but that looked like it was going to take a lot of split processing, and I was having a hard time figuring out how to retrieve and manipulate data from accounts.

I was also thinking about adding an optional table that lists all the times the balance goes below the reserve threshold.

This is my first experiment with Scheme, so let me know if there are any issues.

@christopherlam
Copy link
Contributor

christopherlam commented Feb 28, 2019

Hi @ZDBioHazard

First, welcome and thank you for your contribution.

Second, if this is your first foray into scheme then the code quality is very clean, and uses the API very well!

If you don't mind I will try pick it apart some parts of the report, aiming for better fit into the current set of reports. I don't particularly use SX myself so I've created some to try understand the chart.

  1. The jqplot date handling is currently not very good, so, existing linecharts will send iso-dates (yyyy-mm-dd) instead for the x-axis dates. This means the chart is currently broken for my locale (uses dd-mm-yyyy and jqplot can't handle it very well) and needs to be fixed to run. Here's a fix for the gnc:html-linechart-set-row-labels! line:
    (let ((save-fmt (qof-date-format-get)))
      (qof-date-format-set QOF-DATE-FORMAT-ISO)
      (let ((dates (map qof-print-date (map car intervals))))
        (qof-date-format-set save-fmt)
        (gnc:html-linechart-set-row-labels! chart dates)))
  1. The (set! balances ... section is using the second map incorrectly; the second map should really be for-each because the second map output is discarded. I postulate that using these keywords correctly helps code readability.

  2. I note you're using (let* () ...) to run a sequence of instructions... (begin ...) is marginally neater. I personally prefer (when predicate? ...) myself.

  3. The minimum handling can be better rewritten as (make-list (length balances) (apply min balances)) ditto target/reserve

  4. There must be more documentation and helptext for the various options, and ideally a chapter in the Tutorial & Concepts guide.

  5. We prefer emacs-style indenting and commenting -- https://wiki.gnucash.org/wiki/CodingStandard#Guile_and_Scheme

I do not have an opinion whether this chart is useful for inclusion -- I'll leave this to the core devs. I attach a screenshot of a working chart. Early x/y points are regular balances, later ones indicate SX monthly outgoing payment.

image

@christopherlam
Copy link
Contributor

(As an alternative to adding a new report, the existing reports could be augmented to add future SX amounts in the future dates, and even possibly work on liability/income/expense accounts as well...)

@christopherlam
Copy link
Contributor

I have created a prototype branch https://github.com/christopherlam/gnucash/tree/maint-sx-adjustments to augment existing charts to take into account future SX transactions. There would be a new option to enable these adjustments. The advantages to augmenting reports are that there will be a lesser maintenance burden and fewer tests will need to be created. I would be very keen to assign authorship to @ZDBioHazard, mostly thanks to the very clean code presented here.

@ZDBioHazard
Copy link
Contributor Author

Here's an updated branch with some of the improvements you mentioned.

  1. Oh, I use the ISO format myself, so I never noticed. I'm surprised there weren't any more i18n issues - those always trip me up. Thanks for the fixed code.

  2. Good catch, that map was left over from when that code directly generated graph data.

  3. (let () ...) didn't seem like the right construct to me either. I forgot (when ...) existed. Thanks for the reminder.

  4. (make-list ...) works great for the "target" and "reserve" lines, Thanks for that. The "minimum" line works a bit different though. It is intended to show the minimum balance for only the future values of a given point. I posted a screenshot below that should illustrate it better. I was able to clean up the minimum calculation a bit though.

  5. Where should I look to include documentation? Is that in the gnucash-docs repository?

  6. I'll clean up the formatting after I have a chance to go through the style guides and get the logic cleaned up.


My income and expenses are very predictable, so I have tons of scheduled transactions which I use for forecasting like this. Here's an example of this report in action as I use it:

balance-forecast

The "reserve" is set to a balance I don't want to drop below. Let's say I'm trying to save $X to purchase something, and I want to know when I can expect to have that much. I set "target" to $X, and I can see on the graph that if all goes according to plan, I should be able to afford $X by mid-April, but it would be best to wait until July. The "minimum" line just helps visualize future dips so you don't accidentally miss a reserve-crossing if you're playing it close.


I'd love for the other reports to support scheduled transactions. (Especially the expense and cash flow reports, which I also use often.) However, the only SX-related function I could find (gnc-sx-all-instantiate-cashflow-all) only calculates total cash flow, and doesn't let you access individual SX transactions or splits. It seems to be purpose-built for the sx-summary.scm, which is the only report that uses it.

I would imagine the easiest way to add support would be to simply add an (optional) flag to all information retrieval functions to realize SX transactions before returning their data. Then only very minimal changes would have to be made to the reports. That seems like it would be quite a change however.

I don't know if I'm up to rewriting the other reports at the moment though. I had a heck of a time trying to figure out how the Gnucash API worked by looking at other reports, and the reports that do splits processing were very frustrating to try to glean insight from. Also, this report was written during breaks and lunches over 3 months, because that's all the free time I have lately. ;)

@christopherlam
Copy link
Contributor

4. I was able to clean up the minimum calculation a bit though.

Thank you - definitely better - I'd use srfi-1 as much as possible myself, and I usually try eradicate set! calls as far as possible... but this is being picky. Something like (map (lambda (i) (apply min (list-tail balances i))) (iota (length balances)))

I'd love for the other reports to support scheduled transactions

This may be a worthwhile idea; it will change the nature of reporting from 'booked amounts' to 'forecast amounts' though.

Documentation belongs to the gnucash-docs repository.

I can guarantee there will be requests for enhancements - support liability/expense/income accounts.

I'm still not 100% sure it's better to create a new set of forecast-type reports, or augment existing ones. The maintenance, localization and documentation burdens are a major issue for devs.

@ZDBioHazard
Copy link
Contributor Author

Something like (map (lambda (i) (apply min (list-tail balances i))) (iota (length balances)))

Wow, awesome. That's much better. I was trying to figure out how to slice a list.

I usually try eradicate set! calls as far as possible... but this is being picky.

I know the (set! series (append series (list (list ...)))) lines have been driving me crazy since the beginning.

This may be a worthwhile idea; it will change the nature of reporting from 'booked amounts' to 'forecast amounts' though.
...
I'm still not 100% sure it's better to create a new set of forecast-type reports, or augment existing ones. The maintenance, localization and documentation burdens are a major issue for devs.

I imagine that this would manifest as an "include scheduled transactions" checkbox on the report options page. Then all the (gnc:account-get-comm-balance-at-date ...)-type functions would be updated to include a boolean argument which makes them include scheduled transactions.

Updating all the reports to include and use this option sounds like a pain, but probably less of a pain than creating and maintaining a whole separate set of reports, or adding a second execution path to every API-related part of every existing report.

Maybe just add the option to the global preferences, update the functions internally, and skip changing any of the actual reports. As a bonus, everyone's private reports get forecast capability too. ;)

I can guarantee there will be requests for enhancements - support liability/expense/income accounts.

I'm not sure what you mean. This particular report supports liability/expense/income accounts.The "minimum" line doesn't make any sense outside of balance forecasting though.

@christopherlam
Copy link
Contributor

christopherlam commented Mar 1, 2019

Something like (map (lambda (i) (apply min (list-tail balances i))) (iota (length balances)))

Wow, awesome. That's much better. I was trying to figure out how to slice a list.

Ok prepare to be blown... the above still not optimized because it cycles through balances to count length, creates a numbered list of same length, then it keeps calling list-tail for every iteration. My favourite looping construct is a named let. The following starts with balances and an empty result, calls (apply min balances), conses to (initially empty) result, and loops again with the (cdr balances). When it runs out of balances, it means that results list is complete, and it outputs the reverse result.

(let loop ((result '())
           (balances balances))
  (if (null? balances)
      (reverse result)
      (loop (cons (apply min balances) result)
            (cdr balances))))

I know the (set! series (append series (list (list ...)))) lines have been driving me crazy since the beginning.

Agree but this is a minor issue and set! is not a major problem here. It would be more schemey to use (set! series (cons ... series)) to accumulate, and call (reverse series) when you need to use it. The Little Schemer will teach this.

(gnc:account-get-comm-balance-at-date ...)

The above function is IMHO not desirable to run repeatedly for each date point; it calls xaccAccountGetBalanceAsOfDate which will always cycle the account splitlist from the start for every date point and will lead to performance issues with a large account (e.g. upward of 10000 splits in 1 account). This is why I created gnc:account-get-balances-at-dates instead which will cycle through the splitlist once and accumulate balances at various date points. As a result net-charts.scm and category-barchart.scm are fast, and fairly easy to understand. It is in my projectlist to convert and deprecate gnc:account-get-comm-balance-at-date calls in charts to gnc:account-get-balances-at-dates.

With regards to updating existing reports, my vote would definitely be to augment the desired reports to instantiate sx transactions as part of the balance accumulator... the branch https://github.com/christopherlam/gnucash/tree/maint-sx-adjustments shows how it can be coded (and also uses the optimized balancelist function above). There would be new options in a new tab in the appriate reports.

[Scheduled Transactions]
[x] include future scheduled transactions in report

@christopherlam
Copy link
Contributor

christopherlam commented Mar 2, 2019

P.S. I think it is worth highlighting that the SX algorithms will adjust the date-specific balances by calling (gnc-sx-all-instantiate-cashflow-all start-date end-date) whereby start-date and end-date are dates derived from report-dates. This makes an assumption that there are no SX transactions due to take place prior to the report-start-date... e.g. you could choose report-start-date to be in the future. This would lead to an incorrect chart.

Alternatively you could select the sx-start-date to be (current-time) but I'm not sure it'll be valid either; I could have postponed multiple uninstantiated SX transactions.

More testing required, and digging into SX code. I think there should be a new function (gnc-sx-all-instantiate-cashflow-until end-date) instead myself.

@christopherlam
Copy link
Contributor

christopherlam commented Mar 5, 2019

Hi @ZDBioHazard I've experimented augmenting the existing charts, but find that the charts become too cluttered, and UI is quite difficult. So, I'd change my view and think that your report is uniquely useful. I'd add a couple of changes though:

  1. convert display checkboxes to complex to toggle amounts numberbox.
  2. convert as above

The issue still remains that the report-dates can be set to the future, which may miss some SX transactions. I'm not sure there is an easy algorithm to ensure all SX transactions are captured in the report. This limitation will need to be documented.

Moreover the "" empty strings in the options need to be filled with appropriate detailed documentation.

See the additional commit as follows.
https://github.com/christopherlam/gnucash/tree/ZDBioHazard-balance_forecast_report

@christopherlam
Copy link
Contributor

christopherlam commented Mar 5, 2019

Ok last commit added to adjust sx accumulator by sx instanstiations between 'earliest-split' and 'report start-date'. This means the report can choose any start/end dates and all sx transactions will now be captured, assuming that all sx transactions must take place after an account's earliest split!

Note the guile compose functions which creates a complex unary function, and avoids the following construct

(let* ((earliest-date
            (apply min
                   (map xaccTransGetDate
                        (map xaccSplitGetParent
                             (map car 
                                  (filter pair? (map xaccAccountGetSplitList accounts)))))))
  ...

is rewritten

  (let* ((earliest-date
            (apply min (map (compose xaccTransGetDate xaccSplitGetParent car)
                            (filter pair? (map xaccAccountGetSplitList accounts)))))
    ...

@christopherlam
Copy link
Contributor

christopherlam commented Mar 5, 2019

@ZDBioHazard I have now been able to remove call to gnc:account-get-comm-balance-at-date and replace with gnc:account-get-balances-at-dates to generate chart. This does make the chart faster, especially given that the default resolution is "daily". Previously for a multi-year chart, it would keep scanning the accounts from the starting split until date-point, and restart for the next date point, and so on; now it scans only once.

I have also enabled localisation for the series labels.

I hope you do not mind.

I am now happy that the chart is feature-complete, and can be merged in. I would ask that when it is merged, the chart should not be changed except for bugfixes and possibly future enhancements. I will aim to create implementation tests which will ensure the functionality is stabilised, and ask that the documentation is completed. Would you mind doing the needful?
https://github.com/christopherlam/gnucash/tree/ZDBioHazard-balance_forecast_report

@christopherlam
Copy link
Contributor

@ZDBioHazard I'd like to merge this chart into official release from 3.5 due in 3 weeks' time.

Apologies for taking over; would you be happy to write a paragraph for the documentation? Alternatively I can copy from the comments above?

@ZDBioHazard
Copy link
Contributor Author

Wow, perfect timing, I was just looking at this now.

Your edits look great. It looks like you fixed some of the things that were bothering me (like the boolean options), added in some of the things I forgot about (like no accounts and zero balances), and optimized the algorithms that had better solutions I just didn't know enough about Scheme to implement. I'm not 100% sure what's going on in this report code anymore, but I understand what the changes are for and why, so we're good.

I haven't had a chance to pull and test myself though. I'll try to do that tomorrow if I get a chance. I'm attending SCaLE 17x right now.

No worries on taking over this PR, that's what opensource and collaboration is all about. :)
Speaking of, I enabled maintainer updates on this PR, so feel free to push your edits.

I'll see about writing some documentation, but I have a busy couple weeks coming up, so if I don't make it, please feel free to chop up the above comments. Whatever I write will basically be a modified version of the above comments anyway.

Thanks for fixing up this report for me. It will be nice to have it mainlined. I know I've seen at least one mailing list post asking for something like this long ago.

@christopherlam
Copy link
Contributor

Cool I've managed to push to your repo. This is for any other core devs who wish to offer comment.

@christopherlam
Copy link
Contributor

@ZDBioHazard the main scheme magic is extensive use of srfi-1 loops. this takes some getting used to, but extremely expressive.

  1. create account-get-balances-at dates produce list of balances (list $30 $40 $50); using it via map creates list-of-accounts'-list-of-balances (list (list $30 $40 $50...) (list $200 $200 $230...) ...). But we need it transposed, easily done with (apply zip account-balances) -> (list (list $30 $200...) ($40 $200...) ($50 $230...)...)
  2. this transposed list has 1-to-1 mapping with intervals-list to produce date-specific list of accounts' balances, which is accumulated via monetary-collector (add $30, add $200 = total $230) to which the sx balance is added to produce date-specific sx-inclusive balances.

last commit has added some helptext and reuse existing localized strings to reduce translator burden.

to other devs - I don't think I can create suitable tests for this work because the sx creation routines are not very accessible to scheme.

ZDBioHazard and others added 9 commits March 11, 2019 21:48
This report forecasts the combined balances of the selected accounts
based on the scheduled transactions and plots them on a line graph.

You can set a "reserve" amount, which will draw a red line on the
graph, so you can easily see if your forecast dips below a given value.

There is also a "future minimum" line which shows what the lowest future
balance will be at a given point. I find this useful in conjunction with
the "target" line for planning.
* Fix dates display to ISO format
* Use make-list properly to create a list with identical elements
 * Draw the "balance" line over the "minimum" line.
 * X axis labels should be for the end of the period,
   as that's when all the balance samples are taken.
1. convert simple boolean to complex boolean to toggle amounts
2. convert list processing functions to scheme conventions
this commit will initialize the sx accumulator by adding all
instantiated sx amounts, from the earliest split posted-date among the
selected accounts, until the report start-date.
previous will call gnc:account-get-comm-balance-at-date which calls
xaccAccountGetBalanceAsOfDate for every account at every date
point. The xaccAccountGetBalanceAsOfDate is an expensive function
because it scans the account's whole splitlist from the start every
time. use gnc:account-get-balances-at-dates instead which scans an
account only once to generate a balancelist.

this should be a much faster chart.
this commit will modify to reuse some strings which are already
translated, and also add help strings for the various options.
The gnc:case-exchange-fn seems to be designed for single-date reports,
whereas gnc:case-exchange-time-fn for multi-date reports. It may be
faster to have a single exchange-fn definition.

The main reason for this change is to harmonize - all multidate charts
are using case-exchange-time-fn.
@christopherlam
Copy link
Contributor

merged, thanks. We'll update documentation in due course.

@christopherlam
Copy link
Contributor

Last word, I think this chart should be moved from top-level to Asset&Liabilities submenu?

@ZDBioHazard
Copy link
Contributor Author

A&L sounds good to me, I didn't consider menu placement during development.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants