GNOME Bugzilla – Bug 483393
Reports: Allow mixed denominators in numeric collectors
Last modified: 2018-06-29 21:51:13 UTC
collecting commidities with different denom's causes a failure. Fix: ;; Same as above but with gnc:numeric (define (gnc:make-numeric-collector) (let ;;; values ((value (gnc:numeric-zero))) (lambda (action amount) ;;; Dispatch function (case action ((add) (if (gnc:gnc-numeric? amount) (set! value (gnc:numeric-add-fixed amount value)) (gnc:warn "gnc:numeric-collector called with wrong argument: " amount))) ((total) value) (else (gnc:warn "bad gnc:numeric-collector action: " action)))))) should be: ;; Same as above but with gnc:numeric (define (gnc:make-numeric-collector) (let ;;; values ((value (gnc:numeric-zero))) (lambda (action amount) ;;; Dispatch function (case action ((add) (if (gnc:gnc-numeric? amount) (set! value (gnc:numeric-add amount value GNC-DENOM-AUTO GNC-DENOM-LCD)) (gnc:warn "gnc:numeric-collector called with wrong argument: " amount))) ((total) value) (else (gnc:warn "bad gnc:numeric-collector action: " action))))))
You say (gnc:numeric-add amount value GNC-DENOM-AUTO GNC-DENOM-LCD)) instead of (gnc:numeric-add-fixed amount value)) ? Are you sure this doesn't fix any of the current use cases of this?
I am not sure of what you are asking.
For what it's worth, this bit me today. The asset pie chart report was completely dropping some accounts because they had subaccounts denominated in the same commodity but with different smallest fractions. As a result, the commodity collector ended up with a gnc_numeric_error(GNC_ERROR_DENOM_DIFF) as the numeric for that commodity, and summing over the collector gave an error return. The change proposed in comment 0 makes a lot of sense to me based on my (admittedly limited) understanding of gnc-numeric.
Created attachment 114286 [details] [review] create patch from proposed solution (In reply to comment #0) > Fix: I made this into a patch so it turns up on Bugzilla's open patch search at http://bugzilla.gnome.org/reports/patch-report.cgi?product=GnuCash
Nothing wrong with the math, but I wondered about the performance. I thought that using LCD on each add might be horribly expensive, but because of the gnc_numeric library is written, there's really no significant cost when the denominators are already equal. And when they are different, the performance hit is minimized by some code that identifies the special case where one denominator is a multiple of the other. So this looks OK to me. But I'll go ahead and run some simple performance tests. There are a couple of other places where the same modifications might be made.
Fix committed as r17433. Requesting backport for 2.2. I did some cursory performance tests with 15 years of data. The patched version was no slower. In fact, I was surprised to find that it ran marginally faster.
Applied to branches/2.2 as r17514 for inclusion in GnuCash 2.2.7. Thanks.
GnuCash bug tracking has moved to a new Bugzilla host. This bug has been copied to https://bugs.gnucash.org/show_bug.cgi?id=483393. Please update any external references or bookmarks.