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

Use Kahan summation for fsum and fsumf #742

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

skeeto
Copy link

@skeeto skeeto commented Feb 14, 2023

This compensated summation gives a more precise result than the original algorithm, as demonstrated through extra precision in the tests. It also drops recursion and the extra branching.

I used double precision internally for fsumf since I assume its purpose is to interface with single precision, not necessary constrain itself to only using single precision. If needed, it can be trivially changed to use single precision internally while still passing the tests.

In fsumf, naive summation using double precision is likely sufficient, and more precise than single precision Kaham summation. In other words, Kahan summation with double precision in fsumf may be overkill.

In the fsumf test, neither 10000.1 nor 1.1 can be represented exactly. The former is rounded down and the latter is rounded up (by less) to the nearest representable value. As a result, the most precise sum is just shy of the "obvious" result.


Changing the tests isn't strictly necessary. I wanted tests that fail under the old implementation but pass with the new.

This compensated summation gives a more precise result than the original
algorithm, as demonstrated through extra precision in the tests. It also
drops recursion and the extra branching.

I used double precision internally for fsumf since I assume its purpose
is to interface with single precision, not necessary constrain itself to
only using single precision. If needed, it can be trivially changed to
use single precision internally while still passing the tests.

In fsumf, naive summation using double precision is likely sufficient,
and more precise than single precision Kaham summation. In other words,
Kahan summation with double precision in fsumf may be overkill.

In the fsumf test, neither 10000.1 nor 1.1 can be represented exactly.
The former is rounded down and the latter is rounded up (by less) to the
nearest representable value. As a result, the most precise sum is just
shy of the "obvious" result.
Copy link
Owner

@jart jart left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you! It makes me so happy to see a change to Cosmo from one of my favorite bloggers!

Could you send an email to [email protected] from your [email protected] address saying "I intend to assign you the copyright to the changes I contribute to Cosmopolitan" please? See CONTRIBUTING.md. I ask it of everyone. It only has to happen once. It only applies to the changes you choose to contribute. We need it to protect the project. The way we're protecting it is by keeping a record that everyone's wishes are being respected. Please note that in order to assign copyright, it has to be yours to assign. For example, if you have a job where your employment contract states your employer owns everything you do, then you can't assign copyright on your own. In that case, what I recommend doing is writing an email to your manager telling them what you're doing, and making sure he/she feels comfortable with it. That way if an issue ever comes up, we'll have a record that you're OK with the ownership of Cosmopolitan, and that your employer is OK with it too.

@jart jart force-pushed the master branch 2 times, most recently from faa6285 to 2c4b887 Compare July 25, 2024 08:23
@jart jart force-pushed the master branch 2 times, most recently from 3048620 to 6245732 Compare December 22, 2024 06:13
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

Successfully merging this pull request may close these issues.

2 participants