On further experimentation, it is a problem specific to the computeInterestIncrement formula (probably divide-by-zero).  I will work on a patch.

Marking "NEEDINFO" as a flag that we're waiting for your patch.

Created attachment 371564 [details]
Proposed new fin.scm

This is not a patch file, because I don't have a development tool set up.  It is just a revision of fin.scm to address the bug (and add some additional functionality).

I've investigated and "fixed" the bug, but I'm not sure of the best way to proceed.  Firstly, I don't have a development environment set up to easily generate diffs.  Also, I have some questions about how my proposed changes should actually be incorporated:

1. The actual cause of the parse failure seems to be that for some reason the scheduled transaction editor calls the scheme method with an unexpected value for i.  I haven't looked into why/how (I assume it is in the C code).  It doesn't actually do so when _creating_ the scheduled transactions - that part works fine.  Adding a check to the scheme method stops the problem:
  (if (> n 1000000) 0
  ...
  )
(where n is the argument that i is being passed to)

2. The gnc:computeInterestIncrement function does not actually work as described in  https://wiki.gnucash.org/wiki/Scheduled_Transactions#Bank_Account_Interest.  This is because the gnc:futureValue function treats the time argument, t, as a time in _years_, but gnc:computeInterestIncrement calls gnc:futureValue as though t is the time in compounding periods.  (So it works as expected if and only if the compounding is annual.)  I fixed this by changing gnc:futureValue function to treat t as a number of compounding periods.  But I don't know what the usual approach is on this project when changing the behaviour of existing functions that people might be using as they are.  Should the existing gnc:futureValue functionality be left as is, and gnc:computeInterestIncrement be rewritten to use a different function?

3. For my purposes, there are two gaps in the functionality provided by fin.scm:
It is not possible to specify a payment amount - the functions all calculate the "appropriate" payment using the annuity equation, but many lenders round the payment up to the next dollar.  Also, the use of the annuity equation to directly calculate the balance after i periods assumes that no rounding has happened at previous periods; it is fairly common (I think) for the balance to be rounded to the nearest cent after each payment.  So I wrote some new (recursive) functions to calculate the balances, interest amounts, and principal amounts for the general case.  None of this functionality is needed to actually address this bug though, so maybe it should be submitted as a separate feature request.

Please advise.

The change you made to (futureValue) is wrong:
-    ;; t: time
+    ;; t: time in periods
     ;;
     ;; formula from http://www.riskglossary.com/articles/compounding.htm
-  (* a (expt (+ 1 (/ r n)) (* n t))))
+    ;; For some reason, we need to check whether t is ludicrously large, or
+    ;; the function can't be used by the scheduled transaction editor.
+  (if (> t 1000000) -1 (* a (expt (+ 1 (/ r n)) t))))

In normal math, the existing formula is 
   Amount * (1 + rate/number)^years*number 
where "number" is the number of periods per year (e.g. 12 for a note that compounds monthly).

Your changed function returns -1 if the term is more than 1 million years (why bother? and in any case you should return #f to indicate an error) and otherwise removes the multiplication of the number of years by the periods/per year, i.e.
  Amount * (1 + rate/number)^years
That will produce the wrong result unless n = 1, meaning that the note compounds annually.

As for clamping the interest rate at 1 million in gnc:computeInterestIncrement, that's silly too: Even the most egregious loan shark doesn't try to get a 100 million % APR. In any case it's treating the symptom, not the cause.

As for the other stuff, please submit either a separate enhancement request for each or (better) a Github Pull Request. See https://wiki.gnucash.org/wiki/index.php?title=Development&redirect=no#Submitting_Patches.
That said, we really want to get away from bouncing back-and-forth between Scheme and C/C++, so we'd rather have fin.scm turned into fin.cpp.

(In reply to John Ralls from comment #5)
> The change you made to (futureValue) is wrong:
> -    ;; t: time
> +    ;; t: time in periods
>      ;;
>      ;; formula from http://www.riskglossary.com/articles/compounding.htm
> -  (* a (expt (+ 1 (/ r n)) (* n t))))
> +    ;; For some reason, we need to check whether t is ludicrously large, or
> +    ;; the function can't be used by the scheduled transaction editor.
> +  (if (> t 1000000) -1 (* a (expt (+ 1 (/ r n)) t))))
> 
> In normal math, the existing formula is 
>    Amount * (1 + rate/number)^years*number 
> where "number" is the number of periods per year (e.g. 12 for a note that
> compounds monthly).

Yes. As I noted (bullet 2. in my Comment4), gnc:computeInterestIncrement was calling gnc:futureValue as though the t argument used periods - since it's being used in the scheduled transaction editor, each created transaction uses "i" as the number of _periods_, not the number of years.  To fix computeInterestIncrement, I modified the interface of futureValue to accept periods (noting that I'm not sure what the preferred approach is when modifying existing behaviour).

> 
> Your changed function returns -1 if the term is more than 1 million years
> (why bother? and in any case you should return #f to indicate an error) and

As noted in bullet 1. of Comment4, this was the check that was necessary to prevent the "Couldn't parse sx-debit-formula" error in the scheduled transaction editor.  I agree that this fix is treating the symptom and not the cause (which is presumably somewhere in the C code).  The suggestion to use #f instead of -1 is well taken.

> As for clamping the interest rate at 1 million in
> gnc:computeInterestIncrement, that's silly too: Even the most egregious loan
> shark doesn't try to get a 100 million % APR. In any case it's treating the
> symptom, not the cause.

The rate isn't being clamped in gnc:computeInterestIncrement - the i argument is the period number, not the interest rate.  It's what is causing the "Couldn't parse sx-debit-formula" error when it is not checked against an arbitrarily large value.

> 
> As for the other stuff, please submit either a separate enhancement request
> for each or (better) a Github Pull Request. See
> https://wiki.gnucash.org/wiki/index.
> php?title=Development&redirect=no#Submitting_Patches.
> That said, we really want to get away from bouncing back-and-forth between
> Scheme and C/C++, so we'd rather have fin.scm turned into fin.cpp.

OK.

(In reply to Tim from comment #6)
 
> Yes. As I noted (bullet 2. in my Comment4), gnc:computeInterestIncrement was
> calling gnc:futureValue as though the t argument used periods - since it's
> being used in the scheduled transaction editor, each created transaction
> uses "i" as the number of _periods_, not the number of years.  To fix
> computeInterestIncrement, I modified the interface of futureValue to accept
> periods (noting that I'm not sure what the preferred approach is when
> modifying existing behaviour).

Nope. The arguments to futureValue will be a = 50000.0, r = 0.03125, n = 12, and i will be whatever you tell it when you invoke the SX... or the random value that the SX dialog inserts to make sure that the transaction balances, see below. 

Now it's true that when you use "i" in an SX it's special *in the SX* in that the SX will insert its invocation number into that "i". But once it gets to gnc:computeInterestIncrement it's just whatever number the SX sent. You need to compensate for the SX misusing the function by passing 1 for period, e.g.
  gnc:computeInterestIncrement(5000.00, 0.03125, 1, i)

> > As for clamping the interest rate at 1 million in
> > gnc:computeInterestIncrement, that's silly too: Even the most egregious loan
> > shark doesn't try to get a 100 million % APR. In any case it's treating the
> > symptom, not the cause.
> 
> The rate isn't being clamped in gnc:computeInterestIncrement - the i
> argument is the period number, not the interest rate.  It's what is causing
> the "Couldn't parse sx-debit-formula" error when it is not checked against
> an arbitrarily large value.

Ah, right, sorry.

So the fundamental problem would appear to be that for some reason the SX parser is passing a value for the variable. A very little investigation shows that the reason is that the SX dialog stuffs a random value into each variable and uses the result to check that the transaction balances. The random number range was 0..2^32, which is pretty absurd. I've changed the function to use a range from 1..1000. It will be in tomorrow's nightly build at https://code.gnucash.org/builds/win32/maint. Please test it.

GnuCash bug tracking has moved to a new Bugzilla host. The new URL for this bug is https://bugs.gnucash.org/show_bug.cgi?id=795362. Please continue processing the bug there and please update any external references or bookmarks.