You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@datasketches.apache.org by "AlexanderSaydakov (via GitHub)" <gi...@apache.org> on 2024/02/21 00:50:50 UTC
[PR] cross-language test [datasketches-cpp]
AlexanderSaydakov opened a new pull request, #424:
URL: https://github.com/apache/datasketches-cpp/pull/424
generate files for Java
--
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: commits-unsubscribe@datasketches.apache.org
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
Re: [PR] cross-language test [datasketches-cpp]
Posted by "coveralls (via GitHub)" <gi...@apache.org>.
coveralls commented on PR #424:
URL: https://github.com/apache/datasketches-cpp/pull/424#issuecomment-1955585704
## Pull Request Test Coverage Report for [Build 7982238743](https://coveralls.io/builds/65817549)
### Details
* **0** of **0** changed or added relevant lines in **0** files are covered.
* No unchanged relevant lines lost coverage.
* Overall coverage remained the same at **98.704%**
---
| Totals | [](https://coveralls.io/builds/65817549) |
| :-- | --: |
| Change from base [Build 7980471138](https://coveralls.io/builds/65814546): | 0.0% |
| Covered Lines: | 16379 |
| Relevant Lines: | 16594 |
---
##### 💛 - [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.
To unsubscribe, e-mail: commits-unsubscribe@datasketches.apache.org
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
Re: [PR] uint32_t weight in tdigest + cross-language test [datasketches-cpp]
Posted by "jmalkin (via GitHub)" <gi...@apache.org>.
jmalkin commented on code in PR #424:
URL: https://github.com/apache/datasketches-cpp/pull/424#discussion_r1502151245
##########
tdigest/include/tdigest.hpp:
##########
@@ -84,18 +85,20 @@ class tdigest {
static const bool USE_TWO_LEVEL_COMPRESSION = true;
static const bool USE_WEIGHT_LIMIT = true;
+ using W = typename std::conditional<std::is_same<T, double>::value, uint64_t, uint32_t>::type;
+
class centroid {
public:
- centroid(T value, uint64_t weight): mean_(value), weight_(weight) {}
+ centroid(T value, W weight): mean_(value), weight_(weight) {}
void add(const centroid& other) {
weight_ += other.weight_;
mean_ += (other.mean_ - mean_) * other.weight_ / weight_;
}
T get_mean() const { return mean_; }
- uint64_t get_weight() const { return weight_; }
+ T get_weight() const { return weight_; }
Review Comment:
return type W?
##########
tdigest/test/tdigest_test.cpp:
##########
@@ -204,6 +204,27 @@ TEST_CASE("serialize deserialize bytes non empty", "[tdigest]") {
REQUIRE(td.get_quantile(0.5) == deserialized_td.get_quantile(0.5));
}
+TEST_CASE("serialize deserialize steam and bytes equivalence empty", "[tdigest]") {
+ tdigest<double> td(100);
+ std::stringstream s(std::ios::in | std::ios::out | std::ios::binary);
+ td.serialize(s);
+ auto bytes = td.serialize();
+
+ REQUIRE(bytes.size() == static_cast<size_t>(s.tellp()));
+ for (size_t i = 0; i < bytes.size(); ++i) {
+ REQUIRE(((char*)bytes.data())[i] == (char)s.get());
+ }
+
+ s.seekg(0); // rewind
+ auto deserialized_td1 = tdigest<double>::deserialize(s);
+ auto deserialized_td2 = tdigest<double>::deserialize(bytes.data(), bytes.size());
+ REQUIRE(bytes.size() == static_cast<size_t>(s.tellg()));
+
+ REQUIRE(deserialized_td1.get_k() == deserialized_td2.get_k());
+ REQUIRE(deserialized_td1.get_total_weight() == deserialized_td2.get_total_weight());
Review Comment:
This never actually checks that the weight is 0 or that `is_empty()` returns true, only that they're equivalent
--
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: commits-unsubscribe@datasketches.apache.org
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
Re: [PR] uint32_t weight in tdigest + cross-language test [datasketches-cpp]
Posted by "AlexanderSaydakov (via GitHub)" <gi...@apache.org>.
AlexanderSaydakov merged PR #424:
URL: https://github.com/apache/datasketches-cpp/pull/424
--
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: commits-unsubscribe@datasketches.apache.org
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