Skip to content

Make AnalyzedSeries change points lazy properties - #169

Open
MrlixiangWE wants to merge 2 commits into
apache:masterfrom
MrlixiangWE:fix/79-lazy-change-points
Open

Make AnalyzedSeries change points lazy properties#169
MrlixiangWE wants to merge 2 commits into
apache:masterfrom
MrlixiangWE:fix/79-lazy-change-points

Conversation

@MrlixiangWE

@MrlixiangWE MrlixiangWE commented Aug 13, 2026

Copy link
Copy Markdown

Constructing an AnalyzedSeries runs change point detection for every metric up front, even when the caller never reads the results. #79 asks for change_points and change_points_by_time to become properties instead.

change_points, weak_change_points, change_points_by_time and change_points_timestamp are now read-only properties backed by private fields. The first access computes everything once and caches it; append() and from_json() write the backing fields directly. Two behavior notes:

  • change_points_timestamp now records when the change points were actually computed rather than when the object was constructed. A timestamp stored in JSON still takes precedence on deserialization.
  • Constructing with an explicit change_points argument used to leave weak_change_points unset, so reading it raised AttributeError unless from_json patched it afterwards. It now defaults to an empty collection.

Cost measured before (master, cd5bc1e) and after (this branch) with the same script on the same machine (Linux, Xeon @ 2.8GHz, 8 GB RAM, Python 3.11), seeded random data. Every number is the median of 11 runs and is copied verbatim from the millisecond output of the script below; six decimals in milliseconds is exact nanosecond resolution, so nothing is rounded away, and the script prints the raw nanosecond medians alongside:

series construction first access repeated access
1 metric x 2000 points before 68.541473 ms 0.000726 ms 0.000298 ms
after 0.053550 ms 65.670122 ms 0.001036 ms
4 metrics x 5000 points before 640.987496 ms 0.000765 ms 0.000234 ms
after 0.439367 ms 623.203122 ms 0.000977 ms

The cost moves from the constructor to the first read; the total for callers that do read change points stays in the same range, and repeated reads stay cached. Detection results are unchanged: no expected value in the test suite needed to change. The only adjusted test is test_validate, which used to fake an uncomputed state by assigning change_points = None and now constructs with an explicitly empty collection.

Benchmark script
import random
import statistics
import time

from otava.series import AnalysisOptions, AnalyzedSeries, Metric, Series

REPS = 11


def build_series(n, m):
    random.seed(42)
    metrics = {f"m{i}": Metric(1, 1.0) for i in range(m)}
    data = {f"m{i}": [random.gauss(100, 5) for _ in range(n)] for i in range(m)}
    return Series("bench", None, list(range(n)), metrics, data, {})


def run(n, m):
    construct, first, repeat = [], [], []
    for _ in range(REPS):
        s = build_series(n, m)
        t0 = time.perf_counter_ns()
        a = AnalyzedSeries(s, AnalysisOptions())
        t1 = time.perf_counter_ns()
        _ = a.change_points
        _ = a.change_points_by_time
        t2 = time.perf_counter_ns()
        _ = a.change_points
        _ = a.change_points_by_time
        t3 = time.perf_counter_ns()
        construct.append(t1 - t0)
        first.append(t2 - t1)
        repeat.append(t3 - t2)

    def fmt(v):
        med = statistics.median(v)
        return f"{med / 1e6:.6f} ms ({med:.0f} ns)"

    print(f"{m} metrics x {n} points")
    print(f"  construction:    {fmt(construct)}")
    print(f"  first access:    {fmt(first)}")
    print(f"  repeated access: {fmt(repeat)}")


run(2000, 1)
run(5000, 4)

import otava.series
print(f"module: {otava.series.__file__}")

Verification:

  • pytest tests perf (non-container): 194 passed, including three new regression tests for laziness/caching, append() on a freshly constructed instance, and deserialization without recomputation
  • flake8, ruff check, ruff format --check, isort --check-only: clean
  • git diff --check: clean

Closes #79

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

AnalyzedSeries __compute_change_points() and __group_change_points_per_time() could be @properties instead

1 participant