GnuCash
Contact   Instructions
Bug 794755 - Commodity Register displays fractional prices
Summary: Commodity Register displays fractional prices
Status: RESOLVED FIXED
Alias: None
Product: GnuCash
Classification: Unclassified
Component: Register (show other bugs)
Version: 3.0
Hardware: Other Windows
: Normal normal
Target Milestone: ---
Assignee: ui
QA Contact: ui
URL:
Whiteboard:
Keywords:
: 796904 (view as bug list)
Depends on:
Blocks:
 
Reported: 2018-03-28 04:06 EDT by Alen Siljak
Modified: 2018-10-10 09:29 EDT (History)
20 users (show)

See Also:


Attachments
Test file showing fraction display in share prices (1.81 KB, application/x-gnucash)
2018-04-27 20:44 EDT, lj618
no flags Details

Description Alen Siljak 2018-03-28 04:06:28 EDT
The register in 2.7.8 displays fractional price for commodity accounts. 
This is a bit confusing to look at as my brain is too slow process the division of i.e. 1250 with 2449.

See https://imgur.com/SA3Kkbh
Comment 1 rollenwiese 2018-04-03 15:25:36 EDT
I have the same exact issue after upgrading to 3.0 on

Arch Linux
- XML or Postgres backends
Comment 2 Alen Siljak 2018-04-04 08:55:22 EDT
Hmm, this is not always the case. I have some records that are displayed as expected while others are displayed in fractions.
Any hints as to where I should look? Where is the register price displayed from? Is it calculated or read from the price table, recalculated, etc.?
Comment 3 John Ralls 2018-04-04 09:43:15 EDT
I think the logic there displays a decimal if it can make one without rounding and a fraction otherwise. I'd start with gnucash/gnome/dialog-price-edit.c and then libgnucash/engine/gnc-pricedb.c
Comment 4 Alen Siljak 2018-04-04 11:05:26 EDT
Interesting. The price in the register row is displayed as 3791/36209.

Once the row has a focus, set by clicking on it (whether it is the transaction row or a split row), the price displayed in this row displays changes to decimal format - i.e. 1.1047.
Comment 5 Chris Good 2018-04-04 19:04:11 EDT
But goes back to fraction when another split is clicked.
Comment 6 lj618 2018-04-07 17:31:38 EDT
In case it might help developers, when the fractions are displayed in the register, a number of these messages are logged:

  WARN <qof> [gnc_numeric_to_decimal()] Rounding required when 'never round' specified.

I also notice that in some cases when the transaction is selected and the price is displayed as a decimal, the decimal value differs from that shown by Gnucash-2,6,x. For example: Gnucash-2.6.x shows the correct share price of 14.42; Gnucash-3.0 shows 14 + 39902 / 95007 when the row is not selected, and when the row is selected it shows 14.41999 which is incorrect.
Comment 7 David Carlson 2018-04-08 00:30:43 EDT
Actually, 39902/95007 is .41999.  If that value came from a purchase or sale transaction it is probably the value required to make the total dollar amount and the number of shares both correct.  That is quite often the case on brokerage statements, where they always round prices to two decimal places.  Recent Releases before 3.0 also round decimal prices to two decimal places unless the user has changed the default, but earlier releases showed the fractional numbers.
Comment 8 Chris Good 2018-04-21 21:40:25 EDT
Is this the same problem that now causes stocks for which a buy has been entered in GnuCash 3.0, to show a Price (Unit) with 6 decimals in the Advanced Portfolio Report, while all my other stocks show only 2 decimals?
Comment 9 Chris Good 2018-04-22 01:56:19 EDT
I see from testing in 2.6.19 (Linux) that the (unit) price previously showed between 2 and 6 decimals if it came from a transaction, rather than the price database, so my previous comment is NOT a change in behaviour for 3.0.
However, the Advanced Portfolio Report now shows (unit) prices like $29 + 58581/84250 when the price comes from a transaction but presumably this will be fixed when this bug is fixed.
Comment 10 Geert Janssens 2018-04-27 08:15:27 EDT
I can't reproduce this with gnucash built from the current maint branch (3.0-105-gd0fca7794).

I have tried creating commodity related transactions or transactions involving multiple currencies. In all cases the amounts show normally. Perhaps I'm doing it wrong though as I'm not using commodities myself.

Can someone post a sample file to illustrate this ? It can be a file with only one transaction showing the isse.
Comment 11 lj618 2018-04-27 20:42:17 EDT
To reproduce, you need to (1) use a mutual fund that allows fractions of shares, not a stock that is traded in whole shares only, and (2) enter the total BUY amount and the number of shares, and let GnuCash calculate the share price.

While (2) might seem odd - don't we know the share price? - I find that I have to do it this way to get my records to match the brokerage. The numbers that must match are the number of shares I own, and the total price I paid.

I will attach a file fractiontest.gnucash which is a simple test file with 3 transactions, 2 of which have the fraction display issue with GnuCash-3.0.
Comment 12 lj618 2018-04-27 20:44:30 EDT
Created attachment 371479 [details]
Test file showing fraction display in share prices
Comment 13 Frank H. Ellenberger 2018-05-01 15:42:00 EDT
Because I prefer the precise rational price, we should probably have an additional column rounded decimal price? Then the user couldactivate the preferrred column.
Comment 14 Alen Siljak 2018-05-09 03:32:05 EDT
The instruction in comment 11 (https://bugzilla.gnome.org/show_bug.cgi?id=794755#c11) is not entirely correct. One does not need to enter the total amount. I never do that, for example. However, I do not (yet) know how exactly to reproduce as it is not bothering me too much.
I normally enter just the number of shares and the price, as reported by the brokerage service.
Comment 15 chris 2018-06-25 13:21:55 EDT
Just curious if this is a recognized bug and there are plans on fixing.  This new behavior is very annoying.  Thanks!!!
Comment 16 Geert Janssens 2018-06-25 13:40:09 EDT
Yes, it's recognized as a bug and will be fixed eventually. There are still more urgent issues to solve which is why it didn't happen yet.

Patches/PR's are welcome of course...
Comment 17 chris 2018-07-10 13:02:35 EDT
I was able to resolve this issue as follows: 
in split-register-model.c/gnc_split_register_get_price_entry()
add the following line:

    price = gnc_numeric_convert (price, gnc_commodity_get_fraction (gnc_default_currency ()), GNC_HOW_RND_ROUND_HALF_UP);

just before this line:
    return xaccPrintAmount (price, gnc_default_price_print_info ());
Comment 18 Christopher Lam 2018-07-13 00:43:51 EDT
chris: nearly there, thank you for pointer. we shouldn't use gnc_default_currency which is set once per book, we should use the split->transaction->currency.

I think you're right that GNC_HOW_RND_ROUND_HALF_UP (i.e. 1.1->1.0 and 1.5->2 and -1.5->-2.0 ) is appropriate- anyone please double-check?

Meanwhile I've slipstreamed this fix onto https://github.com/Gnucash/gnucash/pull/380

a better fix is:

modified   gnucash/register/ledger-core/split-register-model.c
@@ -1362,6 +1362,7 @@ gnc_split_register_get_price_entry (VirtualLocation virt_loc,
     SplitRegister *reg = user_data;
     gnc_numeric price;
     Split *split;
+    gnc_commodity *currency;
 
     if (!gnc_split_register_use_security_cells (reg, virt_loc))
         return NULL;
@@ -1372,6 +1373,10 @@ gnc_split_register_get_price_entry (VirtualLocation virt_loc,
     if (gnc_numeric_zero_p (price))
         return NULL;
 
+    currency = xaccTransGetCurrency (xaccSplitGetParent (split));
+    price = gnc_numeric_convert (price,
+                                 gnc_commodity_get_fraction(currency),
+                                 GNC_HOW_RND_ROUND_HALF_UP);
     return xaccPrintAmount (price, gnc_default_price_print_info ());
 }
Comment 19 Christopher Lam 2018-07-13 00:54:33 EDT
@Frank - if a more precise price is required I guess we can increase the fraction by 100?

So I guess it would be

+    currency = xaccTransGetCurrency (xaccSplitGetParent (split));
+    price = gnc_numeric_convert (price,
+                                 100 * gnc_commodity_get_fraction(currency),
+                                 GNC_HOW_RND_ROUND_HALF_UP);

This looks a better approach; because the price does not display trailing zeros, so an exact price of $20 will show 20, a price of $20+1/3 display as 20.3333, and a price of $20+2/3 will show 20.6667

I'll amend PR accordingly
Comment 20 John Ralls 2018-07-13 10:09:27 EDT
I think most users would expect a decimal representation of price to always show the same number of digits to the right of the decimal; from your example then 20.0000.

Aside from that, it shouldn't be necessary to do any manipulation of the number in split-register.c. Simply setting force-fit on the print info should do the job. I don't see any reason not to do that in gnc_default_price_print_info(). BTW, that sets the number of decimal places to 6, but I think using the number of places for currency fraction * 100 is better.
Comment 21 Christopher Lam 2018-07-14 02:40:02 EDT
I do not know how to amend gnc_default_price_print_info() -- this function doesn't have direct access to commodity -- and must defer to someone better in C to fix.
Comment 22 John Ralls 2018-07-15 17:24:19 EDT
(In reply to Chris from comment #21)
> I do not know how to amend gnc_default_price_print_info() -- this function
> doesn't have direct access to commodity -- and must defer to someone better
> in C to fix.

It's not that hard, https://github.com/Gnucash/gnucash/commit/1fffbaf856921906e195803726375765a2eacaa0.

Anticipating that some users might like the exact price display I also made  Preferences>General>Numbers: Force decimal prices, defaulting to exact fractions, so users who want decimal prices need to set.
Comment 23 Geert Janssens 2018-07-31 11:44:40 EDT
(In reply to John Ralls from comment #22)
> Anticipating that some users might like the exact price display I also made 
> Preferences>General>Numbers: Force decimal prices, defaulting to exact
> fractions, so users who want decimal prices need to set.

I have evaluated this new option on the same criteria as I used for Bob's new options. I believe this is a good one to add as it increases gnucash' flexibility in the accounting area.

I would not have chosen fractions as the default though. They are harder to read (next to impossible in some cases) and only a few users would really want them. I understand decimal representation would cause rounding on the other hand.

Would you accept a change of default ?
Comment 24 John Ralls 2018-07-31 12:11:23 EDT
As a default (mine, not this particular option's) I think a new option's default should be to preserve the current behavior following the "least surprise" principle. Cases like this where we're changing behavior following user complaints merit considering the reverse with a comment in the release notes: While the new behavior would be a surprise, for most users it would likely be a pleasant one.
Comment 25 Carl Reinke 2018-07-31 23:00:54 EDT
Wasn't the default (and only option, as far as I know) in GnuCash 2.6 decimal prices?  I don't recall having set a setting to have decimal prices in 2.6.

It was an unpleasant surprise when I upgraded and suddenly "10.88" turned into "10 + 209800/238409".  No financial institution that I interact with would show me a fraction like that.
Comment 26 David Carlson 2018-07-31 23:27:04 EDT
I am curious about where a decimal price representation would actually cause a user to make an error.   

If it does, so would using the price listed in my brokers' monthly statement. 

Users who might actually care already know that all handheld calculators make rounding errors and adjust their behavior accordingly. 

I think that the default should follow the most common usage.
Comment 27 John Ralls 2018-07-31 23:55:34 EDT
No, the fractional display thing has been around (and people have been complaining about it) forever, it's just more common with 3.2 because 2.6 and earlier had a lot of gratuitous rounding to 6 decimal digits and 3.x doesn't. That rounding breaks things like Bitcoin and certain eastern-European mutual funds that quote to more digits and causes trouble for currency exchanges where one currency unit is a small fraction of the other, so part of the point of reworking the numeric system  for 3.0 was to remove the 6-digit rounding.

Back-calculating a purchase of 2098 somethings (shares of something?) at 10 209800/238409 doesn't produces a total price of 22826.24070400026844624154288 and that doesn't seem reasonable. Maybe there's some other rounding going on? What was the actual amount and value that produced that price?
Comment 28 Carl Reinke 2018-08-01 00:02:08 EDT
Shares: (953.636)
Sell:  10,375.56

https://i.imgur.com/IIHDvhW.png
Comment 29 John Ralls 2018-08-01 00:06:17 EDT
(In reply to David Carlson from comment #26)
> I am curious about where a decimal price representation would actually cause
> a user to make an error.   
> 
> If it does, so would using the price listed in my brokers' monthly
> statement. 
> 
> Users who might actually care already know that all handheld calculators
> make rounding errors and adjust their behavior accordingly. 
> 
> I think that the default should follow the most common usage.

Users who are numerate and understand how computers work understand that. Normal people assume that whatever a calculator or a brokerage statement tells them must be right.

A user who nets the commissions and fees in a stock transaction instead of recording them in separate splits will often get a price that can't be represented as a decimal with GnuCash's precision like the one that Carl presented. In the example Carl provided the rounded off value is small enough that it's unlikely to cause a problem unless the number of shares is very large, but I frequently encounter cases where the rounding error causes a 1-penny imbalance. (I have to report a trust's accounts to a court in Virginia. I export GnuCash transaction reports to a spreadsheet to format the reports and usually have to make a couple of adjustments to get it to balance.)
Comment 30 John Ralls 2018-08-01 00:35:12 EDT
(In reply to Carl Reinke from comment #28)
> Shares: (953.636)
> Sell:  10,375.56
> 
> https://i.imgur.com/IIHDvhW.png

Ah, good:
>>> from fractions import Fraction
>>> r=Fraction(10375560,953636)
>>> r.numerator
2593890
>>> r.denominator
238409
>>> 2593890 - 2384090
209800

So GnuCash's price agrees with Python, and 
>>> from decimal import *
>>> Decimal(10375560)/Decimal(953636)
Decimal('10.88000033555780192861846658')
>>> Decimal(953.636) * Decimal(10.88)
Decimal('10375.55968000000038912816080')
>>> Decimal(10375.56)/Decimal(10.88)
Decimal('953.6360294117645905628583110')

Showing the rounding errors from using 10.88 instead of the correct number. It doesn't make a difference rounding to USD or Euro cents but it would screw up the balance in Bitcoin, which uses 10^-9 for its smallest currency unit.

It's a good check of the internals but somewhat skew to the topic at hand, which is whether more users would prefer the *display* of a rounded decimal price or an exact one in the price editor and the price column in the register. I'd expect anyone watching this bug to bin in the "rounded decimal" camp. Those preferring the exact representation are happy with the current display and have no motivation to be here.
Comment 31 Geert Janssens 2018-08-01 06:50:47 EDT
Frank preferred fractions, and perhaps you do as well (I can't infer clearly from your comments). I always assumed our intention was to store fractions and display decimal, but I understand there are some practical issues we need to solve.

In the past several 'bugs' with respect to displaying fractions were 'fixed'. I understand we had rounding issues in previous versions which we just papered over and we're trying very hard to do better now. Your work on the numerics internals are a giant leap in that direction.

On the other hand I think the user experience suffers from suddenly displaying fractions. It's inconsistent as most values are displayed as decimals.

Here's another consideration: if we only can have automatic rounding while displaying (if reduction without rounding fails), how big is the risk that this rounding will creep up in subsequent calculations ? Or put differently shouldn't these calculations that work on display values rather than internal values be considered buggy ? Shouldn't they be using the internal representation rather than the display representation ?

If we choose to display fractions instead of decimals we are in fact working round these bugs, no ?

I have design ideas to potentially handle this more elegantly in the future (again ;) ), but I won't add them here on the bug report. It would just get lost and is only for 4.x anyway. I plan to gather all such ideas and put them somewhere for further discussion.
Comment 32 John Ralls 2018-08-01 10:33:47 EDT
I think that the only calculation risk from displaying a rounded decimal is if the user copies the displayed value to another field, though I suppose it's possible that on saving a dialog the rounded value could overwrite the "real" one. I hadn't thought of that and it needs to be investigated.

Otherwise, let's not lose sight of the fact that the display of rounded decimals is now (or at least will be on release of 3.3) an option and that we're debating whether the default should be to display the precise number or the rounded decimal one.
Comment 33 Carl Reinke 2018-08-01 11:22:14 EDT
I would suggest that the default should cater to the significantly more common usage and that USD and Euro are significantly more common than any currency that would need fractional display.  Even Bitcoin is normally displayed in decimal -- it just potentially needs 8 digits after the decimal point.
Comment 34 Ron Yorston 2018-10-06 04:42:39 EDT
Thanks for fixing this.

But why is the default to display fractional prices?
Comment 35 John Ralls 2018-10-10 09:29:42 EDT
*** Bug 796904 has been marked as a duplicate of this bug. ***

Note You need to log in before you can comment on or make changes to this bug.