You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@datasketches.apache.org by GitBox <gi...@apache.org> on 2020/03/06 00:20:15 UTC
[GitHub] [incubator-datasketches-cpp] jmalkin opened a new pull request #99:
check for NaN and inifite weights in varopt updates
jmalkin opened a new pull request #99: check for NaN and inifite weights in varopt updates
URL: https://github.com/apache/incubator-datasketches-cpp/pull/99
----------------------------------------------------------------
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
[GitHub] [incubator-datasketches-cpp] AlexanderSaydakov commented on a
change in pull request #99: check for NaN and inifite weights in varopt
updates
Posted by 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
[GitHub] [incubator-datasketches-cpp] jmalkin merged pull request #99: check
for NaN and inifite weights in varopt updates
Posted by GitBox <gi...@apache.org>.
jmalkin merged pull request #99: check for NaN and inifite weights in varopt updates
URL: https://github.com/apache/incubator-datasketches-cpp/pull/99
----------------------------------------------------------------
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
[GitHub] [incubator-datasketches-cpp] AlexanderSaydakov commented on issue
#99: check for NaN and inifite weights in varopt updates
Posted by GitBox <gi...@apache.org>.
AlexanderSaydakov commented on issue #99: check for NaN and inifite weights in varopt updates
URL: https://github.com/apache/incubator-datasketches-cpp/pull/99#issuecomment-600246601
looks good to me
----------------------------------------------------------------
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
[GitHub] [incubator-datasketches-cpp] jmalkin commented on a change in pull
request #99: check for NaN and inifite weights in varopt updates
Posted by GitBox <gi...@apache.org>.
jmalkin 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_r391990470
##########
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:
i'm ok with those -- we should probably avoid throwing where possible
----------------------------------------------------------------
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