Pairwise summation for dx & dy: minor improvement in computational accuracy

[halmaia]

The original
dx = ((c1 + c4 + c4 + c7) - (c3 + c6 + c6 + c9)) / H;
dy = ((c7 + c8 + c8 + c9) - (c1 + c2 + c2 + c3)) / V;
were replaced to
dx = ((c1 + c7) + (c4 + c4) - ((c3 + c9) + (c6 + c6))) / H;
dy = ((c7 + c9) + (c8 + c8) - ((c1 + c3) + (c2 + c2))) / V;
Pairwise summation reduces cancellation errors, so this form handles edge cases more reliably.
Minor cosmetics at variable "key".

[echoix]

I have no opinion on if it works as good or not, so I would approve if no one has opinions in a couple days.

However, how does this compare to Kahan sums, like was used somewhere else in this project last year?
https://en.wikipedia.org/wiki/Kahan_summation_algorithm

[halmaia]

The goal is the same, but Kahan is overkill for this simple problem. Edouard Choinière ***@***.***> ezt írta (időpont: 2026. máj. 2., Szo, 18:53):

…

*echoix* left a comment (OSGeo/grass#7364) <#7364 (comment)> I have no opinion on if it works as good or not, so I would approve if no one has opinions in a couple days. However, how does this compare to Kahan sums, like was used somewhere else in this project last year? https://en.wikipedia.org/wiki/Kahan_summation_algorithm — Reply to this email directly, view it on GitHub <#7364 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AEBCY4AX44KCKIO2QYNY5HT4YYRX7AVCNFSM6AAAAACYOLZUCCVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHM2DGNRUGI4DQOJRGQ> . Triage notifications on the go with GitHub Mobile for iOS <https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675> or Android <https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub>. You are receiving this because you authored the thread.Message ID: ***@***.***>

[github-actions]

[pre-commit] _{reported by reviewdog 🐶}

Suggested change

	/* Pairwise summation reduces cancellation errors, this form handles edge cases more reliably. */
	/* Pairwise summation reduces cancellation errors, this form
	* handles edge cases more reliably. */

[wenzeslaus]

I'm generally in favor, but we want to be careful not to change results needlessly. Can you please comment on how this changes result and in which cases? CI with tests is all over the place now, but from what you are saying many computations might be just the same, or not? We do have an errata category where we put things like change of results due to better formulas or fixes. Does this fit? Do you have a minimal reproducible example?

[echoix]

The ubuntugis ppa is down. Is it related to the Ubuntu DDoS two days ago?

[metzm]

I am all for computational accuracy and numerical stability. In this case, input is elevation and values in a 3x3 window are summed up. With elevation, we usually do not have extreme values that could cause problems with double precision floating point limits. Moreover, the values in a 3x3 window should be similar to each other, not causing any cancellation errors when summing them up. Cancellation errors typically occur when adding a (absolute) very small number to a (absolute) very large number. This is highly unlikely in this case. Moreover, this PR replaces the summations of four values with the summations of 2x2 values. Pairwise summation is apparently meant for a much larger number of values.

OTOH, this PR does not do any harm and could indeed improve computational accuracy for edge cases. An example would be appreciated.

There are other occurrences of of summations of four or more values in r.slope.aspect/main.c. Shouldn't these also be addressed with added parentheses to use pairwise summations?

[metzm]

This PR is not only about summation, but summation and subtraction.

dx = ((c1 + c4 + c4 + c7) - (c3 + c6 + c6 + c9)) / H;

is without inner parentheses

dx = (c1 + c4 + c4 + c7 - c3 - c6 - c6 - c9) / H;

and re-ordered to only sum up differences which should all be similarly small

 dx = ((c1 - c3) + 2 * (c4 - c6) + (c7 - c9)) / H;

Just an example for various options to improve computational accuracy.

[metzm]

Theory says that the sum of differences is safer than the difference of sums. Thus

dx = ((c1 - c3) + 2 * (c4 - c6) + (c7 - c9)) / H;

is the proper solution, similar for dy. Please adjust accordingly.

[metzm]

Replace the difference of sums with the sum of differences.