GitHub PullRequestReviewCommentEvent at 2014-02-23 06:00:43
Repository: rust
Actor: huonw
Payload: {"comment":{"url":"https://api.github.com/repos/mozilla/rust/pulls/comments/9974508","id":9974508,"diff_hunk":"@@ -166,38 +168,116 @@ impl Summary {\n }\n \n impl<'a> Stats for &'a [f64] {\n-\n-    // FIXME #11059 handle NaN, inf and overflow\n     fn sum(self) -> f64 {\n-        let mut partials : ~[f64] = ~[];\n \n+        fn samesign(x: f64, y: f64) -> bool {\n+            (x >= 0f64) == (y > 0f64)\n+        }\n+\n+        fn twosum(x: f64, y: f64) -> (f64, f64) {\n+            let hi = x + y;\n+            let lo = y - (hi - x);\n+            (hi, lo)\n+        }\n+\n+        let mut partials = Vec::new();\n+        partials.push(0.0);\n+        let twopow: f64 = f64::pow(2.0, (f64::MAX_EXP - 1) as f64);\n         for &mut x in self.iter() {\n-            let mut j = 0;\n-            // This inner loop applies `hi`/`lo` summation to each\n-            // partial so that the list of partial sums remains exact.\n-            for i in range(0, partials.len()) {\n-                let mut y = partials[i];\n-                if num::abs(x) < num::abs(y) {\n-                    mem::swap(&mut x, &mut y);\n+            if x.is_nan() {\n+                return x;\n+            } else if x.is_infinite() {\n+                partials.as_mut_slice()[0] += x;\n+            } else {\n+                let mut i = 1;\n+                for z in range(1, partials.len()) {\n+                    let mut y = partials.as_slice()[z];\n+                    if num::abs(x) < num::abs(y) {\n+                        mem::swap(&mut x, &mut y);\n+                    }\n+                    let (mut hi, mut lo) = twosum(x,y);\n+                    if hi.is_infinite() {\n+                        let sign = if hi > 0.0 { -1.0 } else { 1.0 };\n+                        x = x - twopow * sign - twopow * sign;\n+                        partials.as_mut_slice()[0] += sign;\n+                        if num::abs(x) < num::abs(y) {\n+                            mem::swap(&mut x, &mut y);\n+                        }\n+                        let (h, l) = twosum(x,y);\n+                        hi = h;\n+                        lo = l;\n+                    }\n+                    if lo != 0.0 {\n+                        partials.as_mut_slice()[i] = lo;\n+                        i += 1;\n+                    }\n+                    x = hi;\n                 }\n-                // Rounded `x+y` is stored in `hi` with round-off stored in\n-                // `lo`. Together `hi+lo` are exactly equal to `x+y`.\n-                let hi = x + y;\n-                let lo = y - (hi - x);\n-                if lo != 0f64 {\n-                    partials[j] = lo;\n-                    j += 1;\n+                if i >= partials.len() {\n+                    partials.push(x);\n+                } else {\n+                    partials.as_mut_slice()[i] = x;\n+                    partials.truncate(i + 1);\n                 }\n-                x = hi;\n             }\n-            if j >= partials.len() {\n-                partials.push(x);\n+        }\n+        // special cases arising from infinities\n+        if partials.as_slice()[0].is_infinite() {\n+            return partials.as_slice()[0];\n+        } else if partials.as_slice()[0].is_nan() {\n+            fail!(\"infinities of both signs in summands\");\n+        }\n+\n+        let plen = partials.len();\n+        if num::abs(partials.as_slice()[0]) == 1.0 && plen > 1 &&\n+                !samesign(partials.as_slice()[plen - 1], \n+                partials.as_slice()[0]) {\n+            let (hi, lo) = \n+                    twosum(partials.as_slice()[0] * twopow, \n+                    0.5 * partials.as_slice()[plen - 1]);\n+            if (2.0 * hi).is_infinite() {\n+                if (hi + 2.0 * lo - hi) == (2.0 * lo) && plen > 2 &&\n+                        samesign(lo, partials.as_slice()[plen - 2]) {\n+                    return 2.0 * (hi + 2.0 * lo);\n+                }\n             } else {\n-                partials[j] = x;\n-                partials.truncate(j+1);\n+                if lo != 0.0 {\n+                    partials.push(2.0 * lo);\n+                }\n+                partials.push(2.0 * hi);\n+                partials.as_mut_slice()[0] = 0.0;\n             }\n         }\n-        partials.iter().fold(0.0, |p, q| p + *q)\n+        // if there is no overflow, compute the sum of a list of\n+        // nonoverlapping floats.\n+        if partials.as_slice()[0] == 0.0 {\n+            let mut total_so_far = partials.pop().expect\n+                    (\"Impossible empty vector in Stats.sum\");","path":"src/libextra/stats.rs","position":120,"original_position":120,"commit_id":"606d5bc18583d079fb10f0ced12c536bdeb0a812","original_commit_id":"606d5bc18583d079fb10f0ced12c536bdeb0a812","user":{"login":"huonw","id":1203825,"avatar_url":"https://gravatar.com/avatar/365f3c2704e4744e237651f3c2a7c411?d=https%3A%2F%2Fidenticons.github.com%2Ffd04754df99d685827696fc1e66db286.png&r=x","gravatar_id":"365f3c2704e4744e237651f3c2a7c411","url":"https://api.github.com/users/huonw","html_url":"https://github.com/huonw","followers_url":"https://api.github.com/users/huonw/followers","following_url":"https://api.github.com/users/huonw/following{/other_user}","gists_url":"https://api.github.com/users/huonw/gists{/gist_id}","starred_url":"https://api.github.com/users/huonw/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/huonw/subscriptions","organizations_url":"https://api.github.com/users/huonw/orgs","repos_url":"https://api.github.com/users/huonw/repos","events_url":"https://api.github.com/users/huonw/events{/privacy}","received_events_url":"https://api.github.com/users/huonw/received_events","type":"User","site_admin":false},"body":"Can probably have the `(...)` on the same line, but, if that's too long, it should be\r\n\r\n```rust\r\nlet mut total_so_far = partials.pop()\r\n    .expect(\"Stats.sum: impossible empty vector\");\r\n```","created_at":"2014-02-23T06:00:43Z","updated_at":"2014-02-23T06:00:43Z","html_url":"https://github.com/mozilla/rust/pull/12355#discussion_r9974508","pull_request_url":"https://api.github.com/repos/mozilla/rust/pulls/12355","_links":{"self":{"href":"https://api.github.com/repos/mozilla/rust/pulls/comments/9974508"},"html":{"href":"https://github.com/mozilla/rust/pull/12355#discussion_r9974508"},"pull_request":{"href":"https://api.github.com/repos/mozilla/rust/pulls/12355"}}}}