Skip to content

Commit

Permalink
Fixes total count bug when merging histograms with missing counts
Browse files Browse the repository at this point in the history
  • Loading branch information
ashenfad committed Nov 12, 2012
1 parent 4e02e82 commit e39421b
Show file tree
Hide file tree
Showing 3 changed files with 9 additions and 3 deletions.
2 changes: 1 addition & 1 deletion project.clj
@@ -1,5 +1,5 @@

(defproject histogram "2.1.0"
(defproject histogram "2.1.1"
:description "Dynamic/streaming histograms"
:source-path "src/clj"
:java-source-path "src/java"
Expand Down
1 change: 0 additions & 1 deletion src/java/com/bigml/histogram/Histogram.java
Expand Up @@ -574,7 +574,6 @@ public Histogram merge(Histogram<T> histogram) throws MixedInsertException {
_missingTarget.sum(histogram.getMissingTarget());
}
_missingCount += histogram.getMissingCount();
_totalCount += histogram.getMissingCount();

This comment has been minimized.

Copy link
@charleslparker

charleslparker Nov 12, 2012

Member

I think that explains a few things I've been seeing. @jdonaldson : this is also likely to help your friends at SocialLens.

This comment has been minimized.

Copy link
@jaor

jaor via email Nov 12, 2012

Member
return this;
}

Expand Down
9 changes: 8 additions & 1 deletion test/histogram/test/core.clj
Expand Up @@ -58,7 +58,14 @@
merged-hist (reduce merge! hists)]
(is (about= (sum merged-hist 0)
(/ (* points hist-count) 2)
(/ (* points hist-count) 50)))))
(/ (* points hist-count) 50))))
(let [h1 (-> (create)
(insert! 1 1)
(insert! nil 1))
h2 (-> (create)
(insert! 2 2)
(insert! nil 2))]
(is (== 2 (total-count (merge! h1 h2))))))

(deftest mixed-test
(let [insert-pair #(apply insert! (apply insert! (create) %1) %2)
Expand Down

2 comments on commit e39421b

@jdonaldson
Copy link

Choose a reason for hiding this comment

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

good deal, thanks

@charleslparker
Copy link
Member

Choose a reason for hiding this comment

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

charlie, please feel free to take care of this one... i'm under the weather today...

Will do.

Please sign in to comment.