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

Balance calculation fails #5

Open
dvabuzyarov opened this issue Feb 23, 2020 · 12 comments
Open

Balance calculation fails #5

dvabuzyarov opened this issue Feb 23, 2020 · 12 comments

Comments

@dvabuzyarov
Copy link

I have a transaction that is fine in ledger:

2019/07/12 * box
55:assets:box -55 HUMMER @ 5372 EUR
55:expenses:delivery -1,700 EUR
55:expenses:delivery -13,818.44 EUR
55:assets:box 55 HUMMER @ 5,654.15345 EUR

but it fails with nledger
The root cause is that the balance of this transaction is visible as -0, and the IsZero property returns false.

        [TestMethod]
        public void Test()
        {
            var amount1 = new Amount(10.16);
            var amount2 = new Amount(-10.15345);
            Value balance = null;
            balance = Value.AddOrSetValue(balance, Value.Get(amount1));
            balance = Value.AddOrSetValue(balance, Value.Get(amount2));
            Assert.IsTrue(balance.IsZero);
        }

I would be happy to assist to sort it out.

@dmitry-merzlyakov
Copy link
Owner

Hi, thank you for your input, let me take a look, I will respond you in a few days. Basically, I confirm the issue with the test case; it is obviously about rounding. However, I cannot tell for sure which application manages this case less reasonable - I remember some examples when ledger rounding worked not so accurate as it was expected. Let me check, thank you!

@dvabuzyarov
Copy link
Author

I would propose to work on the issue in a pair session via Skype. I am going to use this lib in my personal project and expecting to find out some more issues, so it would be very valuable to get some insight directly from you.

@dmitry-merzlyakov
Copy link
Owner

Hi, it is likely you use another version of ledger that I used for porting: I see that your transaction fails for both nledger and ledger with exactly the same message: "Error: Only one posting with null amount allowed per transaction". My version of ledger version is 3.1.1; here is the output:

dmitry@DESKTOP-A712KAM:~$ ledger
Ledger 3.1.1-20160111, the command-line accounting tool

dmitry@DESKTOP-A712KAM:~$ ledger bal -f /mnt/c/dev/dmvs/nledger/source/nledger.cli/bin/debug/issues-5.test
While parsing file "/mnt/c/dev/dmvs/nledger/source/nledger.cli/bin/debug/issues-5.test", line 5:
Error: Only one posting with null amount allowed per transaction

NLedger produces the same:
C:\DEV\DMVS\NLedger\Source\NLedger.CLI\bin\Debug>NLedger-cli.exe bal -f issues-5.test While parsing file "C:\DEV\DMVS\NLedger\Source\NLedger.CLI\bin\Debug\issues-5.test", line 5: Error: Only one posting with null amount allowed per transaction

Currently, NLedger code is equal to ledger 3.1.1 branch NEXT commit fd486a59. Please, check ledger version on your side.

So, I do not consider this issue to be a defect - nledger behaves exactly as its origin. However, if you use a newer version of ledger, I think it is a good time to consider an upgrade for NLedger codebase. Per quick look through ledger commits, I would say that it will take several weeks. Let me know if you are interested in it, it is not a big deal for me.

In regards to the unit test example - I am not sure whether it is expected to work: your numbers are definitely differ, so that they never balance. I tried to imagine how to make it work (e.g. to play with commodities, precisions and flags) but did not fine any practical examples. If your code needs for something similar - I would recommend to create a ledger unit test with similar example - just to validate that ledger code can manage it.

And, of course, any time you want to discuss any aspects of ledger/nledger code - you are welcome; just ping me by skype dmitry.merzlyakov. Thank you for your interest to this stuff!

@dmitry-merzlyakov
Copy link
Owner

BTW, I created a test file for your case; you can find it in attach
issues-5.zip

@dvabuzyarov
Copy link
Author

Yes, that is true, I am using a newer version, mine version is Ledger 3.1.3-20190331.
The idea to update the codebase sounds good for me.

@dmitry-merzlyakov
Copy link
Owner

Hi, I have an update in regards to this ticket:

  1. I updated NLedger code base with the latest changes in ledger; now NLedger "next-dev" branch exactly matches ledger 3.1.3 (ledger master commit 7c45da22 - 3/19/2020 5:57:49 AM). You can check _CI.BuildLog.md to get the latest binaries;
  2. I created a ledger-style test for your transaction - \NLedger\Contrib\test\nledger\gh-issues-5.test

The point is that this test shows the same behavior for both NLedger and the latest ledger - an attempt to read your transaction leads to the error:
Error: Only one posting with null amount allowed per transaction

(The original ledger was built with the latest master sources on a fresh image of Ubuntu Server 19.10 following recommended steps).

So, I do not see any discrepancies between ledger and NLedger so far; your transaction causes an expected error.

If you do no think so, let's try the following steps:

  1. Please, make sure that you provided a complete transaction text (check gh-issues-5.test). If it is not complete - please, send me an updated test;
  2. If transaction text is OK - please, check that gh-issues-5.test does not cause an error for your ledger. In this case, we need to troubleshoot deeper why your ledger behaves differently than mine.

Thank you!

@dvabuzyarov
Copy link
Author

It is a really good work.
I will try it on the next weekend.

Thanks!

@alensiljak
Copy link

alensiljak commented Nov 21, 2023

It's been a few years. Is this issue still valid?

The error about empty values seems strange as all the postings have values.
However, the IsZero issue may be the same as the one just fixed in #29? Two birds, one stone story.

@dmitry-merzlyakov
Copy link
Owner

I think yes, this issue is still actual. There is a related test file - \Contrib\test\nledger\gh-issues-5.test; it reflects that the described transaction causes an error in NLedger:

2019/07/12 * box
     55:assets:box -55 HUMMER @ 5372 EUR
     55:expenses:delivery -1,700 EUR
     55:expenses:delivery -13,818.44 EUR
     55:assets:box 55 HUMMER @ 5,654.15345 EUR

test bal -> 1
__ERROR__
While parsing file "$FILE", line 5:
Error: Only one posting with null amount allowed per transaction
end test

I do not have the latest Ledger version handy, but I believe that 3.2.1-20200518 produced exactly the same error. It would be good to check what the latest version produces. If it is not an issue for it anymore - their fixing changes will be included into NLedger when I apply recent c++ code to NLedger.

Anyway, it is not related to 29 because the latter is a mistake in .Net code.

@alensiljak
Copy link

alensiljak commented Nov 22, 2023

Initially I thought the issue was due to wrong display on GitHub, using HTML, so I downloaded the test file.
One thing that seems immediately obvious is that all the postings have the amounts. And the amounts are separated with only one space character.
I don't know if this has something to do with me looking at it on the phone. But if it is really one space character, then the amounts are considered the part of the account name.

I've just added the second space to separate the amounts from the account name and the result is the following

~/.../shared/Documents $ ledger -f gh-issues-5.txt b
      -15,518.44 EUR  55:expenses:delivery

@alensiljak
Copy link

It looks like the syntax in that file is wrong. It is not a bug in Ledger or NLedger.

@alensiljak
Copy link

The correct syntax for the Xact would be:

2019/07/12 * box
     55:assets:box  -55 HUMMER @ 5372 EUR
     55:expenses:delivery  -1,700 EUR
     55:expenses:delivery  -13,818.44 EUR
     55:assets:box  55 HUMMER @ 5,654.15345 EUR

The test case gh-issues-5.test should be updated.

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

No branches or pull requests

3 participants