Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Change Point Analysis #23931

Merged
merged 73 commits into from
Dec 22, 2022
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
73 commits
Select commit Hold shift + click to select a range
6448677
ADd methods to Pytorch Benchmark to run Change Point analysis
AnandInguva Oct 26, 2022
b2239e7
Add GH issue API for Python
AnandInguva Oct 28, 2022
8d1dd98
Add support for commenting on the issue
AnandInguva Oct 31, 2022
a231516
Add a description about the failing test
AnandInguva Oct 31, 2022
d947923
Add gh_issue object for perf tests
AnandInguva Nov 1, 2022
3fca01c
Update API to read from parameters from config file
AnandInguva Nov 1, 2022
5b194ea
Add find sibling change point method
AnandInguva Nov 2, 2022
156af50
Add support for storing the metadata after regression issue is created
AnandInguva Nov 4, 2022
544b8fb
Add logic for finding sibling changepoint
AnandInguva Nov 4, 2022
5f3a94c
Update test_name
AnandInguva Nov 7, 2022
fc1de36
Update GH action
AnandInguva Nov 7, 2022
f825e4e
Update Description of the issue
AnandInguva Nov 7, 2022
c2cb8ae
Add commenting on the issue feature
AnandInguva Nov 7, 2022
13c24bb
Add logic for not creating alert when its the same changepoint
AnandInguva Nov 7, 2022
bde9e14
Update config file
AnandInguva Nov 8, 2022
7711303
Change owner name
AnandInguva Nov 8, 2022
fad21a2
fix lint, pydocs
AnandInguva Nov 8, 2022
26d72cc
Add two tests in the config file
AnandInguva Nov 9, 2022
bd39279
Fix lint
AnandInguva Nov 9, 2022
c4c4486
Add labels to the config file
AnandInguva Nov 9, 2022
18861e5
Fixup lints, and typehints
AnandInguva Nov 9, 2022
3fd0899
add additional params to the config file
AnandInguva Nov 9, 2022
0612c79
Refactor change point analysis code (#82)
AnandInguva Nov 16, 2022
af22414
Fixup lint
AnandInguva Nov 16, 2022
963827c
Changes based on comments
AnandInguva Nov 20, 2022
2600e24
Refactor fetch metrics from BQ
AnandInguva Nov 23, 2022
1ae5949
Add readme on perf alerting tool
AnandInguva Nov 23, 2022
0d7c72a
Add try catch statements
AnandInguva Nov 23, 2022
6d28eb5
Update github action
AnandInguva Nov 23, 2022
00bfee0
Add awaiting triage label
AnandInguva Nov 23, 2022
929124b
Add link to edvisive algorithm
AnandInguva Nov 23, 2022
a02f275
Check with previous change point timestamp in the sibling change poin…
AnandInguva Nov 29, 2022
801d79f
Changes based on comments on PR
AnandInguva Nov 29, 2022
af44c31
Fix test name
AnandInguva Nov 30, 2022
d167500
Merge remote-tracking branch 'upstream/master' into change-point-with…
AnandInguva Nov 30, 2022
2f49383
Refactor code
AnandInguva Dec 1, 2022
ef015b8
Move constants and helper functions to separate .py file
AnandInguva Dec 1, 2022
d96460f
Merge remote-tracking branch 'upstream/master' into change-point-with…
AnandInguva Dec 1, 2022
6db24e5
Refactoring
AnandInguva Dec 2, 2022
bdbf7a5
Rename files
AnandInguva Dec 2, 2022
d66b15d
Add tests
AnandInguva Dec 2, 2022
759d3c3
Refactor and add steps in the doc string
AnandInguva Dec 2, 2022
716a388
extend finding duplicate change points to search last 10 reported CPs
AnandInguva Dec 2, 2022
01c06e2
fix tests
AnandInguva Dec 2, 2022
8842b25
Fix whitespace lint
AnandInguva Dec 2, 2022
abef62d
Fix up lint
AnandInguva Dec 2, 2022
7104e61
Add _ to internal variables
AnandInguva Dec 5, 2022
fd57d19
Remove steps and add it to the PR description
AnandInguva Dec 5, 2022
e1b59c6
Make big_query_metrics_fetcher as a function
AnandInguva Dec 5, 2022
372ebc7
Add return in doc string
AnandInguva Dec 5, 2022
e8770cd
Add perf label and awaiting triage label default while creating alert
AnandInguva Dec 5, 2022
43185c9
Refactor
AnandInguva Dec 5, 2022
1797cc1
Modify loop
AnandInguva Dec 5, 2022
d130f1f
Fix up runtime errors
AnandInguva Dec 5, 2022
433f5fe
Refactor metrics fetcher
AnandInguva Dec 6, 2022
7672b99
Add test
AnandInguva Dec 6, 2022
b0f7b9c
Changes based on comments
AnandInguva Dec 12, 2022
327ea01
Remove the confusion on sorted order of metrics and timestamps
AnandInguva Dec 12, 2022
6fdfab4
Changes issue templates and add TODOs
AnandInguva Dec 12, 2022
117f5fe
Add triaging issues section
AnandInguva Dec 12, 2022
3a453cd
Fixup lint, whitespace
AnandInguva Dec 12, 2022
397e7c7
Add notebook
AnandInguva Dec 14, 2022
41a1a00
Changes based on comments
AnandInguva Dec 14, 2022
a0c3504
Mark optional params in readme
AnandInguva Dec 14, 2022
57a4ef0
correct triage link
AnandInguva Dec 17, 2022
fad6bbe
Add unit tests and fix lints
AnandInguva Dec 17, 2022
102d1b7
Fix up lint
AnandInguva Dec 19, 2022
6b83629
Apply suggestions from code review
AnandInguva Dec 22, 2022
4d7daf8
Change default optional param
AnandInguva Dec 22, 2022
6a22e77
modify mock tests
AnandInguva Dec 22, 2022
fed8018
add docstring
AnandInguva Dec 22, 2022
748ebd5
Change default values
AnandInguva Dec 22, 2022
c12b594
Add tests to GHA
AnandInguva Dec 22, 2022
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions sdks/python/apache_beam/testing/analyzers/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -49,8 +49,8 @@ test_1:
metric_name: mean_load_model_latency_milli_secs
labels:
- run-inference
min_runs_between_change_points: 5
num_runs_in_change_point_window: 7
min_runs_between_change_points: 5 # optional parameter
num_runs_in_change_point_window: 7 # optional parameter
AnandInguva marked this conversation as resolved.
Show resolved Hide resolved
```

**NOTE**: `test_name` should be in the format `apache_beam.foo.bar`. It should point to a single test target.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@
test: `{}` for the metric: `{}`.

For more information on how to triage the alerts, please look at
`Triage performance alert issues` section of the [README](https://github.com/apache/beam/blob/3a453cd9fe2883eebed4b326d800b4cf8b2069e4/sdks/python/apache_beam/testing/analyzers/README.md).
`Triage performance alert issues` section of the [README](https://github.com/apache/beam/tree/master/sdks/python/apache_beam/testing/analyzers/README.md#triage-performance-alert-issues).
"""
_METRIC_INFO_TEMPLATE = "timestamp: {}, metric_value: `{}`"
_AWAITING_TRIAGE_LABEL = 'awaiting triage'
Expand Down Expand Up @@ -85,7 +85,7 @@ def create_issue(
'labels': [_AWAITING_TRIAGE_LABEL, _PERF_ALERT_LABEL]
}
if labels:
data['labels'].extend(labels)
data['labels'].extend(labels) # type: ignore
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For my education, why was this necessary?

Copy link
Contributor Author

@AnandInguva AnandInguva Dec 22, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IIRC, the error is Sequence[str] don't have attribute extend for pylint/mypy on 3.7. it is fixed in later version of pylint

response = requests.post(
url=url, data=json.dumps(data), headers=_HEADERS).json()
return response['number'], response['html_url']
Expand Down
6 changes: 4 additions & 2 deletions sdks/python/apache_beam/testing/analyzers/perf_analysis.py
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ def run_change_point_analysis(params, test_id, big_query_metrics_fetcher):
change_point_index = find_latest_change_point_index(
metric_values=metric_values)
if not change_point_index:
return
return False
AnandInguva marked this conversation as resolved.
Show resolved Hide resolved
# since timestamps are ordered in ascending order and
# num_runs_in_change_point_window refers to the latest runs,
# latest_change_point_run can help determine if the change point
Expand All @@ -89,7 +89,7 @@ def run_change_point_analysis(params, test_id, big_query_metrics_fetcher):
params['test_name'],
latest_change_point_run,
num_runs_in_change_point_window))
return
return False

is_alert = True
last_reported_issue_number = None
Expand Down Expand Up @@ -131,6 +131,8 @@ def run_change_point_analysis(params, test_id, big_query_metrics_fetcher):
publish_issue_metadata_to_big_query(
issue_metadata=issue_metadata, test_name=test_name)

return is_alert


def run(config_file_path: Optional[str] = None) -> None:
"""
Expand Down
131 changes: 110 additions & 21 deletions sdks/python/apache_beam/testing/analyzers/perf_analysis_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,9 @@
import time
import unittest

import mock
import pandas as pd

# pylint: disable=ungrouped-imports
try:
import apache_beam.testing.analyzers.perf_analysis as analysis
Expand All @@ -30,7 +33,30 @@
from apache_beam.testing.analyzers.perf_analysis_utils import e_divisive
from apache_beam.testing.analyzers.perf_analysis_utils import validate_config
except ImportError as e:
analysis = None
analysis = None # type: ignore


# mock methods.
def get_fake_data_with_no_change_point(**kwargs):
num_samples = 20
metric_values = [1 for i in range(num_samples)]
AnandInguva marked this conversation as resolved.
Show resolved Hide resolved
timestamps = [i for i in range(num_samples)]
AnandInguva marked this conversation as resolved.
Show resolved Hide resolved
return metric_values, timestamps


def get_fake_data_with_change_point(**kwargs):
num_samples = 20
metric_values = [0] * (num_samples // 2) + [1] * (num_samples // 2)
timestamps = [i for i in range(num_samples)]
return metric_values, timestamps


def get_existing_issue_data(**kwargs):
# change point found at index 10. So passing 10 in the
# existing issue data in mock method.
return pd.DataFrame([{
constants._CHANGE_POINT_TIMESTAMP_LABEL: 10, constants._ISSUE_NUMBER: 0
}])


@unittest.skipIf(
tvalentyn marked this conversation as resolved.
Show resolved Hide resolved
Expand All @@ -43,6 +69,15 @@ def setUp(self) -> None:
self.multiple_change_point_series = self.single_change_point_series + [
2
] * 20
self.timestamps = [time.time() + 5 for i in range(5)]
AnandInguva marked this conversation as resolved.
Show resolved Hide resolved
self.params = {
'test_name': 'fake_test',
'metrics_dataset': 'fake_dataset',
'metrics_table': 'fake_table',
'project': 'fake_project',
'metric_name': 'fake_metric_name'
}
self.test_id = 'fake_id'

def test_edivisive_means(self):
change_point_indexes = e_divisive(self.single_change_point_series)
Expand All @@ -51,23 +86,20 @@ def test_edivisive_means(self):
self.assertEqual(sorted(change_point_indexes), [10, 20])

def test_is_changepoint_in_valid_window(self):

changepoint_to_recent_run_window = 19
change_point_index = 14

is_valid = is_change_point_in_valid_window(
changepoint_to_recent_run_window, change_point_index)
self.assertEqual(is_valid, True)

changepoint_to_recent_run_window = 13
is_valid = is_change_point_in_valid_window(
changepoint_to_recent_run_window, change_point_index)
self.assertEqual(is_valid, False)
def test_is_change_point_in_invalid_window(self):
AnandInguva marked this conversation as resolved.
Show resolved Hide resolved
changepoint_to_recent_run_window = 12
change_point_index = 14

changepoint_to_recent_run_window = 14
is_valid = is_change_point_in_valid_window(
changepoint_to_recent_run_window, change_point_index)
self.assertEqual(is_valid, True)
self.assertEqual(is_valid, False)

def test_validate_config(self):
test_keys = {
Expand All @@ -80,34 +112,91 @@ def test_validate_config(self):
self.assertEqual(test_keys, constants._PERF_TEST_KEYS)
self.assertTrue(validate_config(test_keys))

def test_is_perf_alert(self):
timestamp_1 = time.time()
timestamps = [timestamp_1 + i for i in range(4, -1, -1)]

def test_duplicate_change_point(self):
change_point_index = 2
min_runs_between_change_points = 1

is_alert = is_perf_alert(
previous_change_point_timestamps=[timestamps[3]],
timestamps=timestamps,
previous_change_point_timestamps=[self.timestamps[0]],
timestamps=self.timestamps,
change_point_index=change_point_index,
min_runs_between_change_points=min_runs_between_change_points)
self.assertFalse(is_alert)
self.assertTrue(is_alert)

def test_not_duplicate_change_point(self):
AnandInguva marked this conversation as resolved.
Show resolved Hide resolved
change_point_index = 2
min_runs_between_change_points = 1
is_alert = is_perf_alert(
previous_change_point_timestamps=[timestamps[0]],
timestamps=timestamps,
previous_change_point_timestamps=[self.timestamps[3]],
timestamps=self.timestamps,
change_point_index=change_point_index,
min_runs_between_change_points=min_runs_between_change_points)
self.assertTrue(is_alert)
self.assertFalse(is_alert)

is_alert = is_perf_alert(
previous_change_point_timestamps=[timestamps[0], timestamps[3]],
timestamps=timestamps,
previous_change_point_timestamps=[
self.timestamps[0], self.timestamps[3]
],
timestamps=self.timestamps,
change_point_index=change_point_index,
min_runs_between_change_points=min_runs_between_change_points)
self.assertFalse(is_alert)

@mock.patch(
'apache_beam.testing.analyzers.perf_analysis.fetch_metric_data',
get_fake_data_with_no_change_point)
def test_alert_on_data_with_no_change_point(self):
AnandInguva marked this conversation as resolved.
Show resolved Hide resolved
is_alert = analysis.run_change_point_analysis(
params=self.params,
test_id=self.test_id,
big_query_metrics_fetcher=None)
self.assertFalse(is_alert)

@mock.patch(
'apache_beam.testing.analyzers.perf_analysis.fetch_metric_data',
get_fake_data_with_change_point)
@mock.patch(
'apache_beam.testing.analyzers.perf_analysis.get_existing_issues_data',
lambda *args,
AnandInguva marked this conversation as resolved.
Show resolved Hide resolved
**kwargs: None)
@mock.patch(
'apache_beam.testing.analyzers.perf_analysis.'
'publish_issue_metadata_to_big_query',
lambda *args,
**kwargs: None)
@mock.patch(
'apache_beam.testing.analyzers.perf_analysis'
'.create_performance_alert',
lambda *args,
**kwargs: (0, ''))
def test_alert_on_data_with_change_point(self):
is_alert = analysis.run_change_point_analysis(
params=self.params,
test_id=self.test_id,
big_query_metrics_fetcher=None)
self.assertTrue(is_alert)

@mock.patch(
'apache_beam.testing.analyzers.perf_analysis.fetch_metric_data',
get_fake_data_with_change_point)
@mock.patch(
'apache_beam.testing.analyzers.perf_analysis.get_existing_issues_data',
get_existing_issue_data)
@mock.patch(
'apache_beam.testing.analyzers.perf_analysis.'
'publish_issue_metadata_to_big_query',
lambda *args,
**kwargs: None)
@mock.patch(
'apache_beam.testing.analyzers.perf_analysis.create_performance_alert',
lambda *args,
**kwargs: (0, ''))
def test_alert_on_data_with_reported_change_point(self):
is_alert = analysis.run_change_point_analysis(
params=self.params,
test_id=self.test_id,
big_query_metrics_fetcher=None)
self.assertFalse(is_alert)


if __name__ == '__main__':
logging.getLogger().setLevel(logging.DEBUG)
Expand Down