tisonkun commented on code in PR #44:
URL: https://github.com/apache/datasketches-rust/pull/44#discussion_r2653457404


##########
datasketches/src/frequencies/sketch.rs:
##########
@@ -111,22 +122,33 @@ impl<T: Eq + Hash> FrequentItemsSketch<T> {
     }
 
     /// Returns the total weight of the stream.
+    ///
+    /// This is the sum of all counts passed to `update` and 
`update_with_count`.
     pub fn total_weight(&self) -> i64 {
         self.stream_weight
     }
 
     /// Returns the estimated frequency for an item.
+    ///
+    /// If the item is tracked, this is `item_count + offset`. Otherwise it is 
zero.
     pub fn estimate(&self, item: &T) -> i64 {
         let value = self.hash_map.get(item);
         if value > 0 { value + self.offset } else { 0 }
     }
 
-    /// Returns the lower bound for an item's frequency.
-    pub fn lower_bound(&self, item: &T) -> i64 {
-        self.hash_map.get(item)
+    /// Returns the guaranteed lower bound frequency for an item.
+    ///
+    /// This value is never negative and is guaranteed to be no larger than 
the true frequency.
+    /// If the item is not tracked, the lower bound is zero.
+    pub fn lower_bound(&self, item: &T) -> u64 {
+        let value = self.hash_map.get(item);
+        value.max(0) as u64

Review Comment:
   I think the cpp impl uses u64 for reverse_purge_hash_map's value. But let's 
review this point as a follow-up.
   
   Read 
https://github.com/apache/datasketches-cpp/blob/7bb979d3ef8929e235bcd22d67579e1f695f6ecd/fi/include/reverse_purge_hash_map_impl.hpp#L346-L364
 especially for how `subtract_and_keep_positive_only` replaces:
   
   ```rust
           self.adjust_all_values_by(-median);
           self.keep_only_positive_counts();
   ```
   
   This is the only usage where i64 is needed in the current impl.
   
   



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to