diff --git a/otava/analysis.py b/otava/analysis.py index 54cfdb9a..238468f0 100644 --- a/otava/analysis.py +++ b/otava/analysis.py @@ -213,6 +213,44 @@ def compute_change_points_orig(series: Sequence[SupportsFloat], max_pvalue: floa return change_points, None +def compute_change_points_deterministic(series: Sequence[SupportsFloat], max_pvalue: float = 0.001, min_magnitude: float = 0.0) -> Tuple[PermCPList, Optional[PermCPList]]: + """ + Same as the original algorithm but with deterministic Student T significance test at the end. + + The motivation for this variation follows from fixing the bug explained at the top of https://github.com/apache/otava/pull/96 + The intuition is that the split-merge approach introduced by Datastax is addressing the same problem that the _kappa_ variable + does in the original paper. Now that we compute correctly over all values of kappa, the split-merge part should be unnecessary, + as the original algorithm with kappa bug fixed, will find the same change points, and more. Therefore the conclusion is we want + to go back as much as possible to the original and real algorithm from the Matteson & James paper. But even then, we find that + Student T as significance test is both much faster but also qualitatively produces better results for the use case we're in at least, + that we want to continue using T test and not random permutations for the significance test. + + TBD: Whether weak change points are still helpful or not. By reading the problem they fix appears unrelated from the split-merge vs **kappa** symptoms. + + TODO: Support incremental e-divisive. This was easy to implement on top of the split-merge variation. Not clear what is the correct way here. + An easy solution is to rerun from the last change-point, but the problem is the last change point could itself be influenced by the new data + appended. + """ + tester = TTestSignificanceTester(max_pvalue=max_pvalue) + detector = ChangePointDetector(significance_tester=tester, calculator=PairDistanceCalculator) + all_change_points = detector.get_change_points(series=series) + if min_magnitude > 0.0: + above_threshold_change_points = [cp for cp in all_change_points if cp.stats.change_magnitude() >= min_magnitude] + else: + above_threshold_change_points = all_change_points + return above_threshold_change_points, all_change_points + + +def compute_change_points_split( + series: Sequence[SupportsFloat], window_len: int = 50, max_pvalue: float = 0.001, min_magnitude: float = 0.0, + new_data: Optional[int] = None, old_weak_cp: Optional[GenCPList] = None +) -> Tuple[GenCPList, Optional[GenCPList]]: + """ + This function added mainly for symmetry and anticipating a future where this is no longer the default + """ + return compute_change_points(series, window_len, max_pvalue, min_magnitude, new_data, old_weak_cp) + + def compute_change_points( series: Sequence[SupportsFloat], window_len: int = 50, max_pvalue: float = 0.001, min_magnitude: float = 0.0, new_data: Optional[int] = None, old_weak_cp: Optional[GenCPList] = None diff --git a/otava/main.py b/otava/main.py index 3bb164de..4099bfe0 100644 --- a/otava/main.py +++ b/otava/main.py @@ -422,14 +422,28 @@ def setup_analysis_options_parser(parser: argparse.ArgumentParser): "as noise so it is best to keep it short enough to include not more " "than a few change points (optimally at most 1)", ) - parser.add_argument( + ediv_group = parser.add_mutually_exclusive_group() + ediv_group.add_argument( "--orig-edivisive", action="store_true", - default=False, dest="orig_edivisive", help="use the original edivisive algorithm with no windowing " "and weak change points analysis improvements", ) + ediv_group.add_argument( + "--deterministic-edivisive", + action="store_true", + dest="deterministic_edivisive", + help="EXPERIMENTAL: use the original edivisive algorithm, but using " + "Student T for significance test. (TBD: May include weak change points later.)", + ) + ediv_group.add_argument( + "--split-edivisive", + action="store_true", + dest="split_edivisive", + help="use 'hunter' version of this algorithm, from 2023, featuring " + "split of data into smaller windows, weak change points and Student T test. (Default)", + ) def analysis_options_from_args(args: argparse.Namespace) -> AnalysisOptions: @@ -440,8 +454,14 @@ def analysis_options_from_args(args: argparse.Namespace) -> AnalysisOptions: conf.min_magnitude = args.magnitude if args.window is not None: conf.window_len = args.window - if args.orig_edivisive is not None: - conf.orig_edivisive = args.orig_edivisive + + conf.orig_edivisive = args.orig_edivisive + conf.deterministic_edivisive = args.deterministic_edivisive + conf.split_edivisive = args.split_edivisive + if not (args.split_edivisive or args.deterministic_edivisive or args.orig_edivisive): + # Default: + conf.split_edivisive = True + return conf diff --git a/otava/serialization.py b/otava/serialization.py index 1b95ad8b..db64175f 100644 --- a/otava/serialization.py +++ b/otava/serialization.py @@ -30,6 +30,8 @@ class AnalysisOptionsModel(BaseModel): max_pvalue: float = 0.001 min_magnitude: float = 0.0 orig_edivisive: bool = False + deterministic_edivisive: bool = False + split_edivisive: bool = True class MetricModel(BaseModel): diff --git a/otava/series.py b/otava/series.py index 897502db..1b932170 100644 --- a/otava/series.py +++ b/otava/series.py @@ -25,6 +25,7 @@ from otava.analysis import ( TTestStats, compute_change_points, + compute_change_points_deterministic, compute_change_points_orig, ) from otava.change_point_divisive.base import ( @@ -194,6 +195,20 @@ def __compute_change_points( changes={metric: c}, ) result.append(cpg) + elif options.deterministic_edivisive: + change_points, _ = compute_change_points_deterministic( + values, + max_pvalue=options.max_pvalue, + min_magnitude=options.min_magnitude, + ) + for c in change_points: + c.metric = metric + cpg = ChangePointGroup( + time=series.time[c.index], + attributes=series.attributes_at(c.index), + changes={metric: c}, + ) + result.append(cpg) else: change_points, weak_cps = compute_change_points( values, diff --git a/tests/cli_help_test.py b/tests/cli_help_test.py index 06cb974a..d3a1fcd5 100644 --- a/tests/cli_help_test.py +++ b/tests/cli_help_test.py @@ -185,7 +185,7 @@ def test_otava_analyze_help_output(): [--output {{log,json,regressions_only}}] [--branch [STRING]] [--metrics LIST] {usage_filter_lines} [--last COUNT] [-P, --p-value PVALUE] [-M MAGNITUDE] [--window WINDOW] - [--orig-edivisive] + [--orig-edivisive | --deterministic-edivisive | --split-edivisive] tests [tests ...] positional arguments: @@ -237,6 +237,11 @@ def test_otava_analyze_help_output(): enough to include not more than a few change points (optimally at most 1) --orig-edivisive use the original edivisive algorithm with no windowing and weak change points analysis improvements + --deterministic-edivisive + EXPERIMENTAL: use the original edivisive algorithm, but using Student T + for significance test. (TBD: May include weak change points later.) + --split-edivisive use 'hunter' version of this algorithm, from 2023, featuring split of data + into smaller windows, weak change points and Student T test. (Default) CSV Options: Options for CSV configuration diff --git a/tests/cli_options_test.py b/tests/cli_options_test.py index d5f4aebe..01678b72 100644 --- a/tests/cli_options_test.py +++ b/tests/cli_options_test.py @@ -46,18 +46,50 @@ def test_default_cli_option(self): assert otava.series.compute_change_points.call_count == 2 - # Failing due to lack of cp.metric see pull#141 - # def test_orig_cli_option(self): - # with patch('otava.series.compute_change_points') as mock_orig: - # mock_orig.return_value = ([], None) - # with tempfile.TemporaryDirectory() as td: - # td_path = Path(td) - # csv_path, timestamps, config_path, test_name = _create_files_in_temp_dir(td_path) - # # _uv_run(td_path, test_name) - # config_path_str = "" + str(config_path) - # script_main(args=["analyze", "--config", config_path_str, "--orig-edivisive", "true", test_name]) - # - # assert otava.series.compute_change_points_orig.call_count == 2 + def test_split_cli_option(self): + with patch("otava.series.compute_change_points") as mock_split: + mock_split.return_value = ([], []) + with tempfile.TemporaryDirectory() as td: + td_path = Path(td) + csv_path, timestamps, config_path, test_name = _create_files_in_temp_dir(td_path) + config_path_str = "" + str(config_path) + script_main( + args=["analyze", "--config", config_path_str, "--split-edivisive", test_name] + ) + + assert otava.series.compute_change_points.call_count == 2 + + def test_orig_cli_option(self): + with patch("otava.series.compute_change_points_orig") as mock_orig: + mock_orig.return_value = ([], None) + with tempfile.TemporaryDirectory() as td: + td_path = Path(td) + csv_path, timestamps, config_path, test_name = _create_files_in_temp_dir(td_path) + config_path_str = "" + str(config_path) + script_main( + args=["analyze", "--config", config_path_str, "--orig-edivisive", test_name] + ) + + assert otava.series.compute_change_points_orig.call_count == 2 + + def test_deterministic_cli_option(self): + with patch("otava.series.compute_change_points_deterministic") as mock_deterministic: + mock_deterministic.return_value = ([], None) + with tempfile.TemporaryDirectory() as td: + td_path = Path(td) + csv_path, timestamps, config_path, test_name = _create_files_in_temp_dir(td_path) + config_path_str = "" + str(config_path) + script_main( + args=[ + "analyze", + "--config", + config_path_str, + "--deterministic-edivisive", + test_name, + ] + ) + + assert otava.series.compute_change_points_deterministic.call_count == 2 def _create_files_in_temp_dir(td_path: Path): diff --git a/tests/deterministic_tigerbeetle_test.py b/tests/deterministic_tigerbeetle_test.py new file mode 100644 index 00000000..b07bea26 --- /dev/null +++ b/tests/deterministic_tigerbeetle_test.py @@ -0,0 +1,128 @@ + +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. + +from otava.analysis import compute_change_points_deterministic +from tests.tigerbeetle_test import tigerbeetle_demo_data as _get_series + + +def test_tb_magnitude0_p2(): + series = _get_series() + cps, weak_cps = compute_change_points_deterministic(series, max_pvalue=0.2) + indexes = [c.index for c in cps] + assert indexes == [10, 11, 15, 61, 71, 95, 117, 131, 142, 148, 192, 212, 260, 363] + + +def test_tb_magnitude0_p15(): + series = _get_series() + cps, weak_cps = compute_change_points_deterministic(series, max_pvalue=0.15) + indexes = [c.index for c in cps] + assert indexes == [10, 11, 15, 61, 71, 95, 117, 131, 142, 148, 192, 212, 260, 363] + + +def test_tb_magnitude0_p14(): + series = _get_series() + cps, weak_cps = compute_change_points_deterministic(series, max_pvalue=0.14) + indexes = [c.index for c in cps] + assert indexes == [10, 11, 15, 61, 71, 95, 117, 131, 142, 148, 192, 212, 260, 363] + + +def test_tb_magnitude0_p13(): + series = _get_series() + cps, weak_cps = compute_change_points_deterministic(series, max_pvalue=0.13) + indexes = [c.index for c in cps] + assert indexes == [10, 11, 15, 61, 71, 95, 117, 131, 142, 148, 192, 212, 260, 363] + + +def test_tb_magnitude0_p127(): + series = _get_series() + cps, weak_cps = compute_change_points_deterministic(series, max_pvalue=0.129) + indexes = [c.index for c in cps] + assert indexes == [10, 11, 15, 61, 71, 95, 117, 131, 142, 148, 192, 212, 260, 363] + + +def test_tb_magnitude0_p125(): + series = _get_series() + cps, weak_cps = compute_change_points_deterministic(series, max_pvalue=0.125) + indexes = [c.index for c in cps] + assert indexes == [15, 71, 95, 131, 192] + + +def test_tb_magnitude0_p12(): + series = _get_series() + cps, weak_cps = compute_change_points_deterministic(series, max_pvalue=0.12) + indexes = [c.index for c in cps] + assert indexes == [15, 71, 95, 131, 192] + + +def test_tb_magnitude0_p11(): + series = _get_series() + cps, weak_cps = compute_change_points_deterministic(series, max_pvalue=0.11) + indexes = [c.index for c in cps] + assert indexes == [15, 71, 95, 131, 192] + + +def test_tb_magnitude0_p1(): + series = _get_series() + cps, weak_cps = compute_change_points_deterministic(series, max_pvalue=0.1) + indexes = [c.index for c in cps] + assert indexes == [15, 71, 95, 131, 192] + + +def test_tb_magnitude0_p01(): + series = _get_series() + cps, weak_cps = compute_change_points_deterministic(series, max_pvalue=0.01) + indexes = [c.index for c in cps] + assert indexes == [15, 71, 95, 131, 192] + + +def test_tb_magnitude0_p001(): + series = _get_series() + cps, weak_cps = compute_change_points_deterministic(series, max_pvalue=0.001) + indexes = [c.index for c in cps] + assert indexes == [15, 71, 192] + + +def test_tb_magnitude0_p0001(): + series = _get_series() + cps, weak_cps = compute_change_points_deterministic(series, max_pvalue=0.0001) + indexes = [c.index for c in cps] + assert indexes == [15, 71, 192] + + +def test_tb_magnitude0_p00001(): + series = _get_series() + cps, weak_cps = compute_change_points_deterministic(series, max_pvalue=0.00001) + indexes = [c.index for c in cps] + print(cps) + assert indexes == [15, 71, 192] + + +def test_tb_magnitude0_p7x01(): + series = _get_series() + cps, weak_cps = compute_change_points_deterministic(series, max_pvalue=0.00000001) + indexes = [c.index for c in cps] + print(cps) + assert indexes == [71, 192] + + +def test_tb_magnitude0_p31x01(): + series = _get_series() + cps, weak_cps = compute_change_points_deterministic(series, max_pvalue=0.00000000000000000000000000000001) + indexes = [c.index for c in cps] + print(cps) + assert indexes == [71, 192] diff --git a/tests/resources/simple_config.yaml b/tests/resources/simple_config.yaml new file mode 100644 index 00000000..981c423d --- /dev/null +++ b/tests/resources/simple_config.yaml @@ -0,0 +1,64 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. + +# Templates define common bits shared between test definitions +templates: + common_metrics: + metrics: + throughput: + scale: 1 + direction: 1 + p50: + scale: 1.0e-6 + direction: -1 + p75: + scale: 1.0e-6 + direction: -1 + p90: + scale: 1.0e-6 + direction: -1 + p95: + scale: 1.0e-6 + direction: -1 + p99: + scale: 1.0e-6 + direction: -1 + max: + scale: 1.0e-6 + direction: -1 + + +tests: + local1: + type: csv + file: tests/resources/sample.csv + time_column: time + metrics: [metric1, metric2] + attributes: [commit] + + local2: + type: csv + file: tests/resources/sample.csv + time_column: time + metrics: + m1: + column: metric1 + direction: 1 + m2: + column: metric2 + direction: -1 + attributes: [ commit ] diff --git a/tests/series_test.py b/tests/series_test.py index cd549662..3c4b06df 100644 --- a/tests/series_test.py +++ b/tests/series_test.py @@ -20,12 +20,19 @@ from datetime import datetime from random import random +import numpy as np import pytest from pydantic import ValidationError from otava.change_point_divisive.base import ChangePointSerializer from otava.serialization import AnalysisOptionsModel, AnalyzedSeriesModel -from otava.series import AnalysisOptions, AnalyzedSeries, Metric, Series +from otava.series import ( + AnalysisOptions, + AnalyzedSeries, + Metric, + Series, + compute_change_points_deterministic, +) def test_change_point_detection(): @@ -63,6 +70,8 @@ def test_analysis_options_is_pydantic_model(): "max_pvalue": 0.05, "min_magnitude": 1.0, "orig_edivisive": True, + "deterministic_edivisive": False, + "split_edivisive": True, } @@ -530,6 +539,7 @@ def test_orig_edivisive(): options = AnalysisOptions() options.orig_edivisive = True + options.split_edivisive = False options.max_pvalue = 0.01 change_points = test.analyze(options=options).change_points_by_time @@ -769,3 +779,28 @@ def fail_compute(*args, **kwargs): assert restored.change_points_timestamp == analyzed.change_points_timestamp assert [c.index for c in restored.change_points.get_change_points_for_metric("m1")] == [10] assert len(restored.change_points_by_time) == len(analyzed.change_points_by_time) + + +# Edivisive is more sensitive close to the ends of a series +def test_tail_spike_change_point(): + series = np.random.default_rng(2).normal(loc=100.0, scale=5.0, size=303) + series[-4] = 150.0 + + cps, _ = compute_change_points_deterministic( + series, + max_pvalue=0.001, + ) + + assert len(series) - 4 in [cp.index for cp in cps] + + +def test_middle_spike_change_point(): + series = np.random.default_rng(2).normal(loc=100.0, scale=5.0, size=303) + series[150] = 150.0 + + cps, _ = compute_change_points_deterministic( + series, + max_pvalue=0.001, + ) + + assert len(cps) == 0 diff --git a/tests/tigerbeetle_test.py b/tests/tigerbeetle_test.py index 77ea5241..5a43fe9f 100644 --- a/tests/tigerbeetle_test.py +++ b/tests/tigerbeetle_test.py @@ -31,11 +31,11 @@ def _get_series(): ^ .' | ... ,..''.'...,......''','....'''''.......'...'.....,,,..'' |.. .. | |....'' - | || |,,..| + | || |,,+-' | || | ; +-------------------------------------------------------------------------------------> - 10 16 71 97 + 10 15 60 71 95 131 142 192 212 """ return [26705, 26475, 26641, 26806, 26835, 26911, 26564, 26812, 26874, 26682, 15672, 26745, 26460, 26977, 26851, 23412, 23547, 23674, 23519, 23670, 23662, 23462, 23750, 23717, 23524, 23588, 23687, 23793, 23937, 23715, 23570, 23730, 23690, 23699, 23670, 23860, 23988, 23652, 23681, 23798, 23728, 23604, 23523, 23412, 23685, 23773, 23771, 23718, 23409, 23739, 23674, 23597, 23682, 23680, 23711, 23660, 23990, 23938, 23742, 23703, 23536, 24363, 24414, 24483, 24509, 24944, 24235, 24560, 24236, 24667, 24730, 28346, 28437, 28436, 28057, 28217, 28456, 28427, 28398, 28250, 28331, 28222, 28726, 28578, 28345, 28274, 28514, 28590, 28449, 28305, 28411, 28788, 28404, 28821, 28580, 27483, 26805, 27487, 27124, 26898, 27295, 26951, 27312, 27660, 27154, 27050, 26989, 27193, 27503, 27326, 27375, 27513, 27057, 27421, 27574, 27609, 27123, 27824, 27644, 27394, 27836, 27949, 27702, 27457, 27272, 28207, 27802, 27516, 27586, 28005, 27768, 28543, 28237, 27915, 28437, 28342, 27733, 28296, 28524, 28687, 28258, 28611, 29360, 28590, 29641, 28965, 29474, 29256, 28611, 28205, 28539, 27962, 28398, 28509, 28240, 28592, 28102, 28461, 28578, 28669, 28507, 28535, 28226, 28536, 28561, 28087, 27953, 28398, 28007, 28518, 28337, 28242, 28607, 28545, 28514, 28377, 28010, 28412, 28633, 28576, 28195, 28637, 28724, 28466, 28287, 28719, 28425, 28860, 28842, 28604, 28327, 28216, 28946, 28918, 29287, 28725, 29148, 29541, 29137, 29628, 29087, 28612, 29154, 29108, 28884, 29234, 28695, 28969, 28809, 28695, 28634, 28916, 29852, 29389, 29757, 29531, 29363, 29251, 29552, 29561, 29046, 29795, 29022, 29395, 28921, 29739, 29257, 29455, 29376, 29528, 28909, 29492, 28984, 29621, 29026, 29457, 29102, 29114, 28924, 29162, 29259, 29554, 29616, 29211, 29367, 29460, 28836, 29645, 29586, 28848, 29324, 28969, 29150, 29243, 29081, 29312, 28923, 29272, 29117, 29072, 29529, 29737, 29652, 29612, 29856, 29012, 30402, 29969, 29309, 29439, 29285, 29421, 29023, 28772, 29692, 29416, 29267, 29542, 29904, 30045, 29739, 29945, 29141, 29163, 29765, 29197, 29441, 28910, 29504, 29614, 29643, 29506, 29420, 29672, 29432, 29784, 29888, 29309, 29247, 29816, 29254, 29813, 29451, 29382, 29618, 28558, 29845, 29499, 29283, 29184, 29246, 28790, 29952, 29145, 29415, 30437, 29227, 29605, 29859, 29156, 29807, 29406, 29734, 29861, 29140, 29983, 29832, 29919, 29896, 29991, 29266, 29001, 29459, 29548, 29310, 29042, 29303, 29894, 29091, 29018, 29537, 29614, 29180, 29736, 29500, 29218, 29581, 28906, 28542, 29306, 28987, 29878, 28865, 30272, 29707, 29662, 29815, 30492, 29347, 30096, 29054, 30238, 28813, 31895, 28915]