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.

Sign up for GitHub

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

Jump to bottom

Stats.sum doesn't handle NaNs, infinities and overflow correctly #11059

Closed
g3xzh opened this issue Dec 18, 2013 · 12 comments
Closed

Stats.sum doesn't handle NaNs, infinities and overflow correctly #11059

g3xzh opened this issue Dec 18, 2013 · 12 comments
Labels
T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.

Comments

@g3xzh
Copy link
Contributor

g3xzh commented Dec 18, 2013

Like @huonw has put it here: Stats.sum doesn't handle "extreme" values nicely.
@g3xzh
Copy link
Contributor Author

g3xzh commented Dec 18, 2013

I think I will take this issue.
@huonw, it will be more than a pleasure, if you mentor me through this too.

g3xzh added a commit to g3xzh/rust that referenced this issue Dec 19, 2013
@g3xzh
Fix sum() accuracy
05395cb 
`[1e20, 1.0, -1e20].sum()` returns `0.0`. This happens because during
the summation, `1.0` is too small relative to `1e20`, making it
negligible.

I have tried Kahan summation but it hasn't fixed the problem.
Therefore, I've used Python's `fsum()` implementation with some
help from Jason Fager and Huon Wilson.
For more details, read:
www.cs.cmu.edu/~quake-papers/robust-arithmetic.ps

Moreover, benchmark and unit tests were added.

Note: `Status.sum` is still not fully fixed. It doesn't handle
NaNs, infinities and overflow correctly. See issue 11059:
rust-lang#11059
@g3xzh
Copy link
Contributor Author

g3xzh commented Dec 24, 2013

@huonw ,
How about this for handling summation overflow?
http://stackoverflow.com/a/199455 (the addition_is_safe function).

@huonw
Copy link
Member

huonw commented Dec 26, 2013

@g3xzh I don't see any reason to depart from the algorithm that Python is using: http://bugs.python.org/file10357/msum4.py

@g3xzh
Copy link
Contributor Author

g3xzh commented Dec 29, 2013

@huonw - 10x. I hope I will finish working on it this week.

@huonw
Copy link
Member

huonw commented Feb 5, 2014

@g3xzh any progress? :)

@g3xzh
Copy link
Contributor Author

g3xzh commented Feb 5, 2014

@huonw there was some. However, I cannot compile it (rustc is broken on my machine) and because of the exam season I could not continue working on it. :-\

@huonw huonw added the A-libs label Feb 6, 2014
@huonw
Copy link
Member

huonw commented Feb 6, 2014

If someone else is interested, is it ok if they work on it?

@g3xzh
Copy link
Contributor Author

g3xzh commented Feb 7, 2014

I don't mind. My last exam, this semester, is in the 13rd, though. :)

@g3xzh g3xzh mentioned this issue Feb 17, 2014
Closed
@steveklabnik
Copy link
Member

This is now in tests, not in extra.

@steveklabnik
Copy link
Member

Triage: as the test crate is not public anymore, this doesn't appear in user-facing code anymore. http://manishearth.github.io/rust-internals-docs/test/stats/trait.Stats.html

#29553 is related, in the sense that it might make this crate stable, or at least parts of it.

@tbu-
Copy link
Contributor

tbu- commented Mar 21, 2017

This seems to be "fixed" because it's no longer available to code that might feed NaNs, infinity or overflowing values to it.

@steveklabnik steveklabnik added T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. and removed A-libs labels Mar 24, 2017
@aturon
Copy link
Member

aturon commented Apr 15, 2017

Closing, due to the last two comments.

@aturon aturon closed this as completed Apr 15, 2017
@oli-obk oli-obk mentioned this issue Sep 6, 2017
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Add NaNs, infinities and overflows support to sum g3xzh/rust
5 participants
@steveklabnik @aturon @huonw @g3xzh @tbu-