fsprojects / ExcelFinancialFunctions

.NET Standard library providing the full set of financial functions from Excel.
https://fsprojects.github.io/ExcelFinancialFunctions
Other
195 stars 66 forks source link

Behavior of AccrInt does not match Excel when firstInterest <= settlement #22

Closed zahnz closed 2 years ago

zahnz commented 5 years ago

Description

Calling the AccrInt(issue, firstInterest, settlement, rate, par, frequency, basis, calcMethod = AccrIntCalcMethod.FromIssueToSettlement) function with firstInterest <= settlement results in an exception. This does not occur in Excel (tested 2007 and 2013).

Repro steps

  1. Add DLL to a C# project

  2. Execute following code:

    using Excel.FinancialFunctions;
    
            DateTime issue = DateTime.Parse("08/15/18");
            DateTime firstInterest = DateTime.Parse("02/15/19");
            DateTime settlement = DateTime.Parse("02/15/19");
            double rate = 0.02125;
            double par = 100;
            Frequency freq = Frequency.SemiAnnual;
            DayCountBasis basis = DayCountBasis.ActualActual;
            var ret = Financial.AccrInt(issue, firstInterest, settlement, rate, par, freq, basis).ToString();
            Console.WriteLine("Result of test is: " + ret.ToString());

    This results in an exception.

Expected behavior

We expect this to match the output of the Excel function call =ACCRINT(08/15/18, 02/15/19, 02/15/19, 0.02125, 100, 2, 1), which evaluates to 1.0625.

Actual behavior

We get the following exception:

System.Exception: firstInterest must be after settlement at Excel.FinancialFunctions.Bonds.calcAccrInt(DateTime issue, DateTime firstInterest, DateTime settlement, Double rate, Double par, Frequency frequency, DayCountBasis basis, AccrIntCalcMethod calcMethod)

Known workarounds

Comment out the line in bonds.fs under let calcAccrInt issue firstInterest settlement rate par (frequency:Frequency) basis (calcMethod:AccrIntCalcMethod) = that looks like this:

        (firstInterest > settlement)    |> elseThrow "firstInterest must be after settlement"

For example, it's currently (as of writing) at this line in the code. I'm not sure how this might affect other functionality or expectations. Compiling and testing this change doesn't seem to break any of the ~200000 test cases on my machine though.

Related information

Thanks for authoring this fantastic library!

luajalla commented 5 years ago

Hi @zanoo

Thank you for reporting this. The official documentation does not mention any check for this value, and with other tests working I'd say it's safe to delete it.

I've added the fix and test here, however the solution format and build tools also require upgrade which is only partially complete atm and I might have not enough time to get back to it very soon... Could you please checkout the branch and see if it works for you (FSharp.Core version etc)? Any help with finalizing the upgrade is also appreciated :)

Kind regard, Natallie

zahnz commented 5 years ago

Hey @luajalla, apologies for getting back to you so late. Thanks for adding the fix and test. I was able to check out the branch and build it using the build.sh script. All tests passed successfully. My FSharp.Core.dll version is 3.259.4.0. I'm using Mono v4.6.1.5 and the F# compiler for F# v4.0. I'm not quite sure how to help finalize the upgrade, as I have minimal Mono and Paket experience. If you have any suggestions on how to help, let me know.

jcoliz commented 2 years ago

Hi, I am cleaning up old issues. This issue refers to commit 736ed9b140e5d28875137c5e39443188c8ce6a9f. It has not been merged into the primary branch. Leaving the issue open to track.

As a side note, it's worth checking whether or not to remove the whole check, or just change ">" to ">=". Will need to examine Excel behavior in case of "<".

jcoliz commented 2 years ago

I brought Natallie's change over to a feature branch in this repo. It passed all the tests including the "console" interop tests. It's resting in the feature/issue-22 branch. I want to release the netcore replatforming out first without code changes. So I'll target this for a following bugfix release.

Note that I fixed it more narrowly than she did. I kept the firstinterest check in, just allowed it to be equal to settlement date.