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 2021/05/19 21:34:08 UTC

[GitHub] [datasketches-cpp] AlexanderSaydakov opened a new pull request #217: added seed

AlexanderSaydakov opened a new pull request #217:
URL: https://github.com/apache/datasketches-cpp/pull/217


   fixes #216 


-- 
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



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


[GitHub] [datasketches-cpp] jmalkin commented on a change in pull request #217: added seed

Posted by GitBox <gi...@apache.org>.
jmalkin commented on a change in pull request #217:
URL: https://github.com/apache/datasketches-cpp/pull/217#discussion_r635635010



##########
File path: theta/test/theta_jaccard_similarity_test.cpp
##########
@@ -140,4 +179,21 @@ TEST_CASE("theta jaccard: dissimilarity test", "[theta_sketch]") {
   REQUIRE_FALSE(theta_jaccard_similarity::dissimilarity_test(actual, actual, threshold));
 }
 
+TEST_CASE("theta jaccard: dissimilarity test custom seed", "[theta_sketch]") {
+  const int8_t min_lg_k = 12;
+  const int u1 = 1 << 20;
+  const int u2 = static_cast<int>(u1 * 0.05);
+  const double threshold = 0.061;
+  const uint64_t seed = 1234;
+
+  auto expected = update_theta_sketch::builder().set_lg_k(min_lg_k).set_seed(seed).build();
+  for (int i = 0; i < u1; ++i) expected.update(i);
+
+  auto actual = update_theta_sketch::builder().set_lg_k(min_lg_k).set_seed(seed).build();
+  for (int i = 0; i < u2; ++i) actual.update(i);
+
+  REQUIRE(theta_jaccard_similarity::dissimilarity_test(actual, expected, threshold, seed));
+  REQUIRE_FALSE(theta_jaccard_similarity::dissimilarity_test(actual, actual, threshold));

Review comment:
       should this line have the seed passed in, too?




-- 
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



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


[GitHub] [datasketches-cpp] AlexanderSaydakov merged pull request #217: added seed

Posted by GitBox <gi...@apache.org>.
AlexanderSaydakov merged pull request #217:
URL: https://github.com/apache/datasketches-cpp/pull/217


   


-- 
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



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


[GitHub] [datasketches-cpp] coveralls edited a comment on pull request #217: added seed

Posted by GitBox <gi...@apache.org>.
coveralls edited a comment on pull request #217:
URL: https://github.com/apache/datasketches-cpp/pull/217#issuecomment-844495770


   ## Pull Request Test Coverage Report for [Build 858540556](https://coveralls.io/builds/39836897)
   
   * **5** of **5**   **(100.0%)**  changed or added relevant lines in **1** file are covered.
   * No unchanged relevant lines lost coverage.
   * Overall coverage remained the same at **92.94%**
   
   ---
   
   
   
   |  Totals | [![Coverage Status](https://coveralls.io/builds/39836897/badge)](https://coveralls.io/builds/39836897) |
   | :-- | --: |
   | Change from base [Build 858101134](https://coveralls.io/builds/39832128): |  0.0% |
   | Covered Lines: | 2159 |
   | Relevant Lines: | 2323 |
   
   ---
   ##### 💛  - [Coveralls](https://coveralls.io)
   


-- 
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



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


[GitHub] [datasketches-cpp] coveralls edited a comment on pull request #217: added seed

Posted by GitBox <gi...@apache.org>.
coveralls edited a comment on pull request #217:
URL: https://github.com/apache/datasketches-cpp/pull/217#issuecomment-844495770


   ## Pull Request Test Coverage Report for [Build 858477876](https://coveralls.io/builds/39836217)
   
   * **5** of **5**   **(100.0%)**  changed or added relevant lines in **1** file are covered.
   * No unchanged relevant lines lost coverage.
   * Overall coverage remained the same at **92.94%**
   
   ---
   
   
   
   |  Totals | [![Coverage Status](https://coveralls.io/builds/39836217/badge)](https://coveralls.io/builds/39836217) |
   | :-- | --: |
   | Change from base [Build 858101134](https://coveralls.io/builds/39832128): |  0.0% |
   | Covered Lines: | 2159 |
   | Relevant Lines: | 2323 |
   
   ---
   ##### 💛  - [Coveralls](https://coveralls.io)
   


-- 
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



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


[GitHub] [datasketches-cpp] jmalkin commented on a change in pull request #217: added seed

Posted by GitBox <gi...@apache.org>.
jmalkin commented on a change in pull request #217:
URL: https://github.com/apache/datasketches-cpp/pull/217#discussion_r635635339



##########
File path: theta/test/theta_jaccard_similarity_test.cpp
##########
@@ -140,4 +179,21 @@ TEST_CASE("theta jaccard: dissimilarity test", "[theta_sketch]") {
   REQUIRE_FALSE(theta_jaccard_similarity::dissimilarity_test(actual, actual, threshold));
 }
 
+TEST_CASE("theta jaccard: dissimilarity test custom seed", "[theta_sketch]") {
+  const int8_t min_lg_k = 12;
+  const int u1 = 1 << 20;
+  const int u2 = static_cast<int>(u1 * 0.05);
+  const double threshold = 0.061;
+  const uint64_t seed = 1234;
+
+  auto expected = update_theta_sketch::builder().set_lg_k(min_lg_k).set_seed(seed).build();
+  for (int i = 0; i < u1; ++i) expected.update(i);
+
+  auto actual = update_theta_sketch::builder().set_lg_k(min_lg_k).set_seed(seed).build();
+  for (int i = 0; i < u2; ++i) actual.update(i);
+
+  REQUIRE(theta_jaccard_similarity::dissimilarity_test(actual, expected, threshold, seed));
+  REQUIRE_FALSE(theta_jaccard_similarity::dissimilarity_test(actual, actual, threshold));

Review comment:
       oh, nevermind, misread




-- 
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



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


[GitHub] [datasketches-cpp] coveralls edited a comment on pull request #217: added seed

Posted by GitBox <gi...@apache.org>.
coveralls edited a comment on pull request #217:
URL: https://github.com/apache/datasketches-cpp/pull/217#issuecomment-844495770


   ## Pull Request Test Coverage Report for [Build 858419461](https://coveralls.io/builds/39835750)
   
   * **5** of **5**   **(100.0%)**  changed or added relevant lines in **1** file are covered.
   * No unchanged relevant lines lost coverage.
   * Overall coverage remained the same at **92.94%**
   
   ---
   
   
   
   |  Totals | [![Coverage Status](https://coveralls.io/builds/39835750/badge)](https://coveralls.io/builds/39835750) |
   | :-- | --: |
   | Change from base [Build 858101134](https://coveralls.io/builds/39832128): |  0.0% |
   | Covered Lines: | 2159 |
   | Relevant Lines: | 2323 |
   
   ---
   ##### 💛  - [Coveralls](https://coveralls.io)
   


-- 
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



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


[GitHub] [datasketches-cpp] coveralls commented on pull request #217: added seed

Posted by GitBox <gi...@apache.org>.
coveralls commented on pull request #217:
URL: https://github.com/apache/datasketches-cpp/pull/217#issuecomment-844495770


   ## Pull Request Test Coverage Report for [Build 858352642](https://coveralls.io/builds/39834993)
   
   * **5** of **5**   **(100.0%)**  changed or added relevant lines in **1** file are covered.
   * No unchanged relevant lines lost coverage.
   * Overall coverage remained the same at **92.94%**
   
   ---
   
   
   
   |  Totals | [![Coverage Status](https://coveralls.io/builds/39834993/badge)](https://coveralls.io/builds/39834993) |
   | :-- | --: |
   | Change from base [Build 858101134](https://coveralls.io/builds/39832128): |  0.0% |
   | Covered Lines: | 2159 |
   | Relevant Lines: | 2323 |
   
   ---
   ##### 💛  - [Coveralls](https://coveralls.io)
   


-- 
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



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


[GitHub] [datasketches-cpp] AlexanderSaydakov commented on a change in pull request #217: added seed

Posted by GitBox <gi...@apache.org>.
AlexanderSaydakov commented on a change in pull request #217:
URL: https://github.com/apache/datasketches-cpp/pull/217#discussion_r635637581



##########
File path: theta/test/theta_jaccard_similarity_test.cpp
##########
@@ -140,4 +179,21 @@ TEST_CASE("theta jaccard: dissimilarity test", "[theta_sketch]") {
   REQUIRE_FALSE(theta_jaccard_similarity::dissimilarity_test(actual, actual, threshold));
 }
 
+TEST_CASE("theta jaccard: dissimilarity test custom seed", "[theta_sketch]") {
+  const int8_t min_lg_k = 12;
+  const int u1 = 1 << 20;
+  const int u2 = static_cast<int>(u1 * 0.05);
+  const double threshold = 0.061;
+  const uint64_t seed = 1234;
+
+  auto expected = update_theta_sketch::builder().set_lg_k(min_lg_k).set_seed(seed).build();
+  for (int i = 0; i < u1; ++i) expected.update(i);
+
+  auto actual = update_theta_sketch::builder().set_lg_k(min_lg_k).set_seed(seed).build();
+  for (int i = 0; i < u2; ++i) actual.update(i);
+
+  REQUIRE(theta_jaccard_similarity::dissimilarity_test(actual, expected, threshold, seed));
+  REQUIRE_FALSE(theta_jaccard_similarity::dissimilarity_test(actual, actual, threshold));

Review comment:
       yes. the test did not fail because the implementation checks for identical input upfront




-- 
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



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


[GitHub] [datasketches-cpp] jmalkin commented on pull request #217: added seed

Posted by GitBox <gi...@apache.org>.
jmalkin commented on pull request #217:
URL: https://github.com/apache/datasketches-cpp/pull/217#issuecomment-844551046


   👍 


-- 
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



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