fix(data): remove MinMaxPyramid::reduce, repoint its two test callers
The pyramid's public query surface is the bucket-grained trio (reduce_buckets/sum_buckets/sumsq_buckets); reduce() was the one non-*_buckets query and silently over-covered non-aligned ranges, which is exactly the misuse the exactness split exists to prevent. Exact range queries stay on Series. - delete the declaration, its doc paragraph and the definition; no helper became dead (fold_range/cover_buckets/ceil_to_bucket are all still used by the *_buckets queries) - pyramid_clear_and_rebuild: complete bucket 0 and assert its extrema via reduce_buckets(0, kBucket) -- a pyramid has no exact sub-bucket query - stream_series_acquire_publishes_snapshot: s.pyramid().reduce(0, 100) -> s.reduce(0, 100), keeping the == 99.0 assertion
This commit is contained in:
1 parent
d9909f2115
commit
5f1a4991e2
4 files changed
+8
-19
No files matched your search
@@ -28,9 +28,6 @@ inline constexpr u32 kBucket = 64;
|
|||||||
/// buckets; a range with partial edge buckets would silently pull in samples
|
/// buckets; a range with partial edge buckets would silently pull in samples
|
||||||
/// outside the range. Every query here is therefore bucket-grained and named
|
/// outside the range. Every query here is therefore bucket-grained and named
|
||||||
/// *_buckets, and exact range queries live on Series, which owns the raw samples.
|
/// *_buckets, and exact range queries live on Series, which owns the raw samples.
|
||||||
/// reduce() is the one non-*_buckets query: it is the *covering* aggregate over
|
|
||||||
/// the level-0 buckets that [begin, end) touches. Use it to ask which buckets a
|
|
||||||
/// range spans, never to measure a range.
|
|
||||||
class MinMaxPyramid {
|
class MinMaxPyramid {
|
||||||
public:
|
public:
|
||||||
MinMaxPyramid() { add_level(); }
|
MinMaxPyramid() { add_level(); }
|
||||||
@@ -55,11 +52,6 @@ class MinMaxPyramid {
|
|||||||
f64 sum_buckets(u64 begin, u64 end) const;
|
f64 sum_buckets(u64 begin, u64 end) const;
|
||||||
/// Sum of squares of the samples in those same whole buckets.
|
/// Sum of squares of the samples in those same whole buckets.
|
||||||
f64 sumsq_buckets(u64 begin, u64 end) const;
|
f64 sumsq_buckets(u64 begin, u64 end) const;
|
||||||
|
|
||||||
/// Min/max over the covering level-0 buckets: every bucket that holds a sample
|
|
||||||
/// of [begin, end). Exact only when the range is bucket-aligned.
|
|
||||||
Range reduce(u64 begin, u64 end) const;
|
|
||||||
|
|
||||||
/// Pyramid level whose buckets hold about `samples_per_pixel` samples, for a
|
/// Pyramid level whose buckets hold about `samples_per_pixel` samples, for a
|
||||||
/// window of `n` samples: 0 when a pixel covers at most one bucket. Clamped to
|
/// window of `n` samples: 0 when a pixel covers at most one bucket. Clamped to
|
||||||
/// the levels that exist, to levels with more than one bucket, and to levels
|
/// the levels that exist, to levels with more than one bucket, and to levels
|
||||||
|
|||||||
@@ -221,15 +221,6 @@ f64 MinMaxPyramid::sumsq_buckets(u64 begin, u64 end) const {
|
|||||||
return total;
|
return total;
|
||||||
}
|
}
|
||||||
|
|
||||||
Range MinMaxPyramid::reduce(u64 begin, u64 end) const {
|
|
||||||
const u64 e = std::min(end, size_);
|
|
||||||
const u64 b = std::min(begin, e);
|
|
||||||
if (e <= b) return Range{0.0, 0.0};
|
|
||||||
const u64 b0 = b / kBucket; // first bucket holding a sample
|
|
||||||
const u64 b1 = (e - 1) / kBucket + 1; // one past the last one
|
|
||||||
return fold_range(*this, b0, b1);
|
|
||||||
}
|
|
||||||
|
|
||||||
u32 MinMaxPyramid::level_for(u64 n, f64 samples_per_pixel) const {
|
u32 MinMaxPyramid::level_for(u64 n, f64 samples_per_pixel) const {
|
||||||
// The ratio test also rejects NaN, and the clamp keeps a huge (or infinite)
|
// The ratio test also rejects NaN, and the clamp keeps a huge (or infinite)
|
||||||
// samples-per-pixel from making the floor-to-u32 conversion ill-formed.
|
// samples-per-pixel from making the floor-to-u32 conversion ill-formed.
|
||||||
|
|||||||
@@ -103,7 +103,13 @@ PLOT_TEST(pyramid_clear_and_rebuild) {
|
|||||||
PLOT_CHECK(p.level_count() <= 1);
|
PLOT_CHECK(p.level_count() <= 1);
|
||||||
p.append(7.0f);
|
p.append(7.0f);
|
||||||
PLOT_CHECK(p.size() == 1);
|
PLOT_CHECK(p.size() == 1);
|
||||||
PLOT_CHECK_NEAR(p.reduce(0, 1).max, 7.0, 1e-6);
|
// A pyramid can only answer exactly for whole buckets, so complete the first
|
||||||
|
// bucket and ask the bucket-grained query for its extrema.
|
||||||
|
for (u32 i = 1; i < kBucket; ++i) p.append(1.0f);
|
||||||
|
PLOT_CHECK(p.size() == kBucket);
|
||||||
|
PLOT_CHECK(p.level(0).size() == 1);
|
||||||
|
PLOT_CHECK_NEAR(p.reduce_buckets(0, kBucket).max, 7.0, 1e-6);
|
||||||
|
PLOT_CHECK_NEAR(p.reduce_buckets(0, kBucket).min, 1.0, 1e-6);
|
||||||
}
|
}
|
||||||
|
|
||||||
PLOT_TEST(pyramid_cover_cost_is_logarithmic_for_unaligned_starts) {
|
PLOT_TEST(pyramid_cover_cost_is_logarithmic_for_unaligned_starts) {
|
||||||
|
|||||||
@@ -29,7 +29,7 @@ PLOT_TEST(stream_series_acquire_publishes_snapshot) {
|
|||||||
s.acquire();
|
s.acquire();
|
||||||
PLOT_CHECK(s.size() == 100);
|
PLOT_CHECK(s.size() == 100);
|
||||||
// Pyramid must track the published samples.
|
// Pyramid must track the published samples.
|
||||||
PLOT_CHECK_NEAR(s.pyramid().reduce(0, 100).max, 99.0, 1e-4);
|
PLOT_CHECK_NEAR(s.reduce(0, 100).max, 99.0, 1e-4);
|
||||||
for (int i = 100; i < 250; ++i) s.push(f32(i));
|
for (int i = 100; i < 250; ++i) s.push(f32(i));
|
||||||
s.acquire();
|
s.acquire();
|
||||||
PLOT_CHECK(s.size() == 250);
|
PLOT_CHECK(s.size() == 250);
|
||||||
|
|||||||
Reference in new issue
Block a user