Created attachment 373175 [details]
Test file demonstrating problem

There seem to be at least two problems here.  First, the price for AMP in the price DB is not really what we want.  This is presumably because of 62a4e73f which changed price-quotes.scm to not use GNC-DENOM-SIGFIGS when converting prices.  This results in a price of 8515625000000001/3906250000000000 which is correct, but a bit silly.  It may not be worth doing anything about this.

The other problem is that the advanced portfolio report shows this price without any rounding.  This could be fixed.  I'd rather not add yet another option to do this.  Perhaps it could use the commodity's fraction traded valu to figure out how much precision it needs for the price.

Heh. Bet the real problem is converting the price to a double. Instead convert it to a scheme exact rational so you get 218/100 and convert *that* to a GncNumeric.

There are cases where actual trade prices from transactions will result in numbers that need to be rounded, particularly when the user is lazy like me and rolls all of the fees and commissions into the basis. I'm in favor of rounding to 1/100 of the currency SCU, but we had someone from I think Slovenia pop up a few years ago saying that mutual funds there quote prices in millionths (6 places) so I suppose some users will want more.

I'm not sure I understand.  Are you talking about price-quotes.scm or the advanced portfolio report?  Neither converts anything to a double except to format it for output so far as I can tell.  price-quotes.scm gets a string from the Perl code and converts it to a rational.  The report calls gnc_pricedb_lookup_latest_any_currency to get that rational from the price DB.  It gets 8515625000000001/3906250000000000 which it prints in the debug output as "Starting account AMP, initial price: $2.180000000000000256".

I shouldn't have sent that comment as I was heading off to dinner without thinking about it more.  I realized right away that you were talking about the fact that price-quotes.scm converts the string to a double and then converts that to a GNCNumeric. Fixing this to skip the double and go directly to GNCNumeric would be better, but I'm not sure how to do that.

(gnc-scm-to-numeric price)

You mean add this call around line 400 in price-quotes.scm?  I don't see how that helps.  As far as I can tell, gnc-scm-to-numeric simply converts the Scheme representation of a number to a gnc_numeric.  In price-quotes.scm "price" is converted to a floating point value by the call to (read) that processes the output of the Perl script.  If you call gnc-scm-to-numeric on it you get 0/1 since it is not a valid argument.  I put a breakpoint there to be sure that is what is happening.

What am I messing?

I think there are two issues.

1. price-quotes.scm receives unreasonably-detailed-exact-fraction (u.d.e.f.) for saving into pricedb instead of decimal, and should probably be fixed. in the book above, the u.d.e.f. has already polluted the book's price-data. I can try dig in.

2. advanced-portfolio.scm needs to handle u.d.e.f. and trim it down... perhaps we can use strategy that price must be shown with commodity-scu + 2 additional digits?

oops mta said so in #1 already

diff --git a/libgnucash/scm/price-quotes.scm b/libgnucash/scm/price-quotes.scm
index 732704f3d..133e622d4 100644
--- a/libgnucash/scm/price-quotes.scm
+++ b/libgnucash/scm/price-quotes.scm
@@ -404,11 +404,7 @@
               (else #f)))

       (if price
-          (set! price
-                (double-to-gnc-numeric price
-                                       GNC-DENOM-AUTO
-                                       (logior GNC-DENOM-REDUCE
-                                               GNC-RND-NEVER))))
+          (set! price (gnc-scm-to-numeric price)))

       (if gnc-time
           (set! gnc-time (timestr->time64 gnc-time time-zone))

Perl emits a string price that is imported into Scheme as an exact decimal, meaning multi-precision and BCD encoded. The swig wrapper for double-to-gnc-numeric converts that to a double, producing a goofy value because double, and double_to_gnc_numeric() preserves that goofy value as an absurd rational number that APR then has to make sense of.

Solution: Lose the double. Unless there's something wrong with the Scheme string conversion (and I haven't built and tested this to be sure that there isn't) 2.18 from the example above should turn into 109/50. This works because Scheme also has an exact rational class so gnc-scm-to-numeric does the decimal->rational conversion in Scheme and then constructs the GncNumeric from the Scheme rational's numerator and denominator.

That's what I already tried and it doesn't work.  I put these three lines just before the "if price" at line 406:

      (gnc:debug "price " price)
      (gnc:debug "price-type " price-type)
      (gnc:debug "(gnc-scm-to-numeric price)" (gnc-numeric-to-string (gnc-scm-to-numeric price)))


and got

* 17:18:59 DEBUG <gnc.scm> price 2.18
* 17:18:59 DEBUG <gnc.scm> price-type last
* 17:18:59 DEBUG <gnc.scm> (gnc-scm-to-numeric price)0/1

When I put a breakpoint at gnc_scm_to_numeric it took the error path since it couldn't get the numerator and denominator of the input value.  

I think the solution to this problem is the Scheme "rationalize" function.  It is called via (rationalize x y) and is defined to return the simplest rational number differing from x by no more than y.  Then the problem becomes picking a value for y.  Using y = 1/1000000000000000 seems to work.  Using this the call 

"(gnc-scm-to-numeric (rationalize (inexact->exact p) 1/1000000000000000))"

returns reasonable results for p between .12345e-9 and 12345678.87654.  This should cover the range we need for prices.

If this seems ok, I'll make the change and push it.

This still leaves the problem of fixing the Advanced Portfolio report to deal with ridiculous prices better, but that becomes less important if the quotes are more reasonable.

Ah, I thought that Scheme would do that on its own. Yes, that's a reasonable adjustment.

I pushed this change.  I didn't close the bug since the other part related to the advanced portfolio report hasn't been addressed.

Hi Mike,

I downloaded and tested Windows daily build gnucash-3.4-2019-02-21-git-3.4-130-gb334366f6+.setup.exe which should have your maint fix commit from 17 Feb 2019 but it still seems to be doing the same thing:

Rpt:
Account	  Symbol	Listing	Shares	Price
ANZ MG&CG ANZ.AX	ASX	8,568	$28 + 140392362583/4679745419433

Price Database shows ANZ price 28.030000
E:\Program Files (x86)\gnucash\bin>echo (yahoo_json "ANZ.AX") | perl gnc-fq-helper
(("ANZ.AX" (symbol . "ANZ.AX") (gnc:time-no-zone . "2019-03-01 12:00:00") (last . 28.03) (currency . "AUD")))

   <price:commodity>
      <cmdty:space>ASX</cmdty:space>
      <cmdty:id>ANZ.AX</cmdty:id>
    </price:commodity>
    <price:currency>
      <cmdty:space>CURRENCY</cmdty:space>
      <cmdty:id>AUD</cmdty:id>
    </price:currency>
    <price:time>
      <ts:date>2019-03-01 01:00:00</ts:date>
    </price:time>
    <price:source>Finance::Quote</price:source>
    <price:type>last</price:type>
    <price:value>131173264106707/4679745419433</price:value>
  </price>

Try patching advanced-portfolio.scm

modified   gnucash/report/standard-reports/advanced-portfolio.scm
@@ -933,9 +933,14 @@ by preventing negative stock balances.<br/>")
                      )
 	 	    (gnc:html-price-anchor
 	 	     price
-	 	     (gnc:make-gnc-monetary
-	  	     (gnc-price-get-currency price)
-		     (gnc-price-get-value price)))
+                     (let ((curr (gnc-price-get-currency price)))
+	 	       (gnc:make-gnc-monetary
+	  	        curr
+                        (gnc-numeric-convert
+                         (gnc-price-get-value price)
+                         (max 10000
+                              (* 100 (gnc-commodity-get-fraction curr)))
+                         GNC-HOW-RND-ROUND))))
 		    )))))
  	      (append! activecols (list (if use-txn (if pricing-txn "*" "**") " ")
 					(gnc:make-html-table-header-cell/markup

Hi Christopher Lam,
That works fine thanks. Now Price shows 2 decimals usually ($28.03 in my example above). In the case where the price database shows a price to 6 decimals, eg 26.942297, it shows $26.9423 which is also fine with me.

Note that price of 26.942297 came from an entered buy transaction, not imported from F::Q.

Right, I see that too. The Advanced Portfolio report still needs to be fixed.  Your change is a start toward that, but may not be general enough.  I'm not sure we need a new option.  It might be better to figure out a reasonable price format from the SCU of the commodity and the currency.  I.e., calculate the magic numbers 10000 and 100 in your code from these.

For what it's worth, the (non-advanced) Investment Portfolio report shows the same problem and also needs a fix.

Also the price DB window has a similar problem.  It uses 6 digits after the decimal point which isn't always enough.  For example, it shows the IDR->USD exchange rate as .000071 while the value stored is 1187/16748977 which is 0.00007087.  Since the SCU of both currencies is 100, it's not clear how to use them to calculate a reasonable price.  I"m just thinking out loud, but if you changed the SCU of IDR to 1 and then used the ratio of the SCUs plus 6 to get the number of digits after the decimal point it works better.

We already have an option, force-price-decimal, so reports should use it. We decided last summer that the denom should be 100 * SCU; the C implementation in gnc_price_print_info is 
        if (info->commodity)
            denom = gnc_commodity_get_fraction(info->commodity) * 100;
        else if (info->max_decimal_places &&
                 info->max_decimal_places <= maximum_decimals)
            denom = pow10[info->max_decimal_places];
        val = gnc_numeric_convert(val, denom, GNC_HOW_RND_ROUND_HALF_UP);
        value_is_decimal = gnc_numeric_to_decimal(&val, NULL);

I think the best approach would be to do something similar in report-system--there's a lot of overhead to gnc_price_print so you probably don't want to use it if you don't have to--and then call that everywhere there are prices to display in reports.

Chris Lam, have you done anything with my suggestion in comment 18 or anything else related to price display in reports?

There's 7557c5b5 which may have fixed it?

OK. (Oddly on GitHub that commit is wrapped up in its merge commit https://github.com/Gnucash/gnucash/commit/f8bad131a5376baa6cfb401b252245b1bc545901#diff-2c580ec8cc1dbd03f86961d6ffb4d0d3013992168f99d6cbe7a3019e4ed14fbd though it shows up correctly in my local repo).

Chris Good, can you test with 4.0 or later?

John Ralls, I've used 3.6, 3.8, 3.9, 3.10 and 4.0 without any problems.
Testing now in 4.0 shows:

1. Price db shows all prices to 6 decimals (all my stocks are in AUD - 2 decimals)

2. Adv Portfolio Rpt shows all unit prices (all imported) to 4 decimals
	  E.g Imported price from FQ:
	    <price:commodity>
	      <cmdty:space>ASX</cmdty:space>
	      <cmdty:id>AMP.AX</cmdty:id>
	    </price:commodity>
	    <price:currency>
	      <cmdty:space>CURRENCY</cmdty:space>
	      <cmdty:id>AUD</cmdty:id>
	    </price:currency>
	    <price:time>
	      <ts:date>2021-02-19 01:00:00 +0000</ts:date>
	    </price:time>
	    <price:source>Finance::Quote</price:source>
	    <price:type>last</price:type>
	    <price:value>27/20</price:value>
	  </price>

3. Price DB Entry from transaction entry:
	  <price>
	    <price:id type="guid">8e376fa400d54d22b0e7414edd455075</price:id>
	    <price:commodity>
	      <cmdty:space>ASX</cmdty:space>
	      <cmdty:id>BEN.AX</cmdty:id>
	    </price:commodity>
	    <price:currency>
	      <cmdty:space>CURRENCY</cmdty:space>
	      <cmdty:id>AUD</cmdty:id>
	    </price:currency>
	    <price:time>
	      <ts:date>2021-02-27 13:00:00 +0000</ts:date>
	    </price:time>
	    <price:source>user:split-register</price:source>
	    <price:type>transaction</price:type>
	    <price:value>11390000/1000000</price:value>
	  </price>

  Adv Portfolio Rpt still shows 4 decimals for the price from above transaction (11.3900)

I'm happy with it now,  thanks all.

Do try again with the preference "force prices as decimal" disabled...

Hi Christopher,
Sorry for the delay.
I upgraded to gnucash-4.4 (not 4.5 because of the outstanding PDF problem), disabled "Preferences -> Numbers, Date, Time" -> "Force Prices as decimal" and tested after importing prices.

1. price database shows prices with variable no of decimals eg
      AMP 15/04/2021	1.222
          19/02/2021	1.35
          15/02/2019	2.180000000000000256
      ANZ 15/04/2021	28.92
	  15/03/2019    26+ 7228162824427/20651893784077

In XML:
  AMP 2021-04-15 <price:value>611/500</price:value>
  AMP 2021-02-19 <price:value>27/20</price:value>
  AMP 2019-02-15 <price:value>8515625000000001/3906250000000000</price:value>
  ANZ 2019-03-15 <price:value>544177401210429/20651893784077</price:value>

2. Adv Portfolio Rpt shows all unit prices (all imported) to 4 decimals

3. Enable Preferences, Numbers, Date, Time, "Force Prices as decimal"

4. Price Database now shows all prices as decimal numbers (ie no fractions)
 but is hard to read because no of decimals varies between 1 and 3 and is right aligned.

I don't care about when "Force Prices as decimal" is disabled as I cannot see why you'd want that, but when it is enabled, I'd prefer to have the constant no of decimals I saw in 4.0. Either always 4 (preferably) or 6 decimals.