[GitHub] [incubator-datasketches-cpp] AlexanderSaydakov commented on a change in pull request #99: check for NaN and inifite weights in varopt updates

[GitBox <gi...@apache.org>]

AlexanderSaydakov commented on a change in pull request #99: check for NaN and inifite weights in varopt updates
URL: https://github.com/apache/incubator-datasketches-cpp/pull/99#discussion_r388657892
 
 

 ##########
 File path: sampling/include/var_opt_sketch_impl.hpp
 ##########
 @@ -724,8 +724,8 @@ std::string var_opt_sketch<T,S,A>::items_to_string() const {
 
 template<typename T, typename S, typename A>
 void var_opt_sketch<T,S,A>::update(const T& item, double weight, bool mark) {
-  if (weight <= 0.0) { 
-    throw std::invalid_argument("Item weights must be strictly positive. Found: "
+  if (weight <= 0.0 || std::isnan(weight) || std::isinf(weight)) { 
 
 Review comment:
   Should we allow zero weight like in frequent items (and ignore the item)?
   Maybe even ignore items with negative invalid or infinite weight instead of throwing?

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@datasketches.apache.org
For additional commands, e-mail: commits-help@datasketches.apache.org