From d32374fab2e20ab39997dc000a035b901d5b170e Mon Sep 17 00:00:00 2001 From: Anders <754494+andehen@users.noreply.github.com> Date: Fri, 10 Jan 2025 20:14:47 +0100 Subject: [PATCH] fix(experiments): fix bug with Trends continuous new stats (#27432) Co-authored-by: Daniel Bachhuber --- .../src/scenes/experiments/Metrics/TrendsMetricForm.tsx | 2 +- .../experiments/experiment_trends_query_runner.py | 8 ++++++++ .../test/test_experiment_trends_query_runner.py | 8 ++++---- .../experiments/trends_statistics_v2_continuous.py | 7 +++++-- 4 files changed, 18 insertions(+), 7 deletions(-) diff --git a/frontend/src/scenes/experiments/Metrics/TrendsMetricForm.tsx b/frontend/src/scenes/experiments/Metrics/TrendsMetricForm.tsx index bfe5cb88ec168..2958d67b62737 100644 --- a/frontend/src/scenes/experiments/Metrics/TrendsMetricForm.tsx +++ b/frontend/src/scenes/experiments/Metrics/TrendsMetricForm.tsx @@ -81,7 +81,7 @@ export function TrendsMetricForm({ isSecondary = false }: { isSecondary?: boolea showSeriesIndicator={true} entitiesLimit={1} showNumericalPropsOnly={true} - onlyPropertyMathDefinitions={[PropertyMathType.Average]} + onlyPropertyMathDefinitions={[PropertyMathType.Sum]} {...commonActionFilterProps} />
diff --git a/posthog/hogql_queries/experiments/experiment_trends_query_runner.py b/posthog/hogql_queries/experiments/experiment_trends_query_runner.py index b09187c4453aa..f6e5fa3691036 100644 --- a/posthog/hogql_queries/experiments/experiment_trends_query_runner.py +++ b/posthog/hogql_queries/experiments/experiment_trends_query_runner.py @@ -38,6 +38,7 @@ ExperimentTrendsQueryResponse, ExperimentVariantTrendsBaseStats, DateRange, + PropertyMathType, PropertyOperator, TrendsFilter, TrendsQuery, @@ -128,6 +129,13 @@ def _prepare_count_query(self) -> TrendsQuery: """ prepared_count_query = TrendsQuery(**self.query.count_query.model_dump()) + uses_math_aggregation = self._uses_math_aggregation_by_user_or_property_value(prepared_count_query) + + # Only SUM is supported now, but some earlier experiments AVG. That does not + # make sense as input for experiment analysis, so we'll swithc that to SUM here + if uses_math_aggregation: + prepared_count_query.series[0].math = PropertyMathType.SUM + prepared_count_query.trendsFilter = TrendsFilter(display=ChartDisplayType.ACTIONS_LINE_GRAPH_CUMULATIVE) prepared_count_query.dateRange = self._get_date_range() if self._is_data_warehouse_query(prepared_count_query): diff --git a/posthog/hogql_queries/experiments/test/test_experiment_trends_query_runner.py b/posthog/hogql_queries/experiments/test/test_experiment_trends_query_runner.py index 15ec7d4fbfd38..68a5568bd6886 100644 --- a/posthog/hogql_queries/experiments/test/test_experiment_trends_query_runner.py +++ b/posthog/hogql_queries/experiments/test/test_experiment_trends_query_runner.py @@ -1505,7 +1505,7 @@ def test_query_runner_with_avg_math(self): flush_persons_and_events() prepared_count_query = query_runner.prepared_count_query - self.assertEqual(prepared_count_query.series[0].math, "avg") + self.assertEqual(prepared_count_query.series[0].math, "sum") result = query_runner.calculate() trend_result = cast(ExperimentTrendsQueryResponse, result) @@ -1538,7 +1538,7 @@ def test_query_runner_with_avg_math(self): ) # Uses the same values as test_query_runner_with_data_warehouse_series_avg_amount for easy comparison - @freeze_time("2020-01-01T12:00:00Z") + @freeze_time("2020-01-01T00:00:00Z") def test_query_runner_with_avg_math_v2_stats(self): feature_flag = self.create_feature_flag() experiment = self.create_experiment(feature_flag=feature_flag) @@ -1549,7 +1549,7 @@ def test_query_runner_with_avg_math_v2_stats(self): count_query = TrendsQuery( series=[ - EventsNode(event="purchase", math="avg", math_property="amount", math_property_type="event_properties") + EventsNode(event="purchase", math="sum", math_property="amount", math_property_type="event_properties") ], ) exposure_query = TrendsQuery(series=[EventsNode(event="$feature_flag_called")]) @@ -1619,7 +1619,7 @@ def test_query_runner_with_avg_math_v2_stats(self): flush_persons_and_events() prepared_count_query = query_runner.prepared_count_query - self.assertEqual(prepared_count_query.series[0].math, "avg") + self.assertEqual(prepared_count_query.series[0].math, "sum") result = query_runner.calculate() trend_result = cast(ExperimentTrendsQueryResponse, result) diff --git a/posthog/hogql_queries/experiments/trends_statistics_v2_continuous.py b/posthog/hogql_queries/experiments/trends_statistics_v2_continuous.py index e03a0bb3fcecf..4931d0a07212c 100644 --- a/posthog/hogql_queries/experiments/trends_statistics_v2_continuous.py +++ b/posthog/hogql_queries/experiments/trends_statistics_v2_continuous.py @@ -82,8 +82,10 @@ def calculate_probabilities_v2_continuous( if len(test_variants) < 1: raise ValidationError("Can't calculate experiment results for less than 2 variants", code="no_data") + mean_value_control = control_variant.count / control_variant.absolute_exposure + # Calculate posterior parameters for control - log_control_mean = np.log(control_variant.count + EPSILON) # Using count field to store mean value + log_control_mean = np.log(mean_value_control + EPSILON) # Update parameters for control kappa_n_control = KAPPA_0 + control_variant.absolute_exposure @@ -100,7 +102,8 @@ def calculate_probabilities_v2_continuous( # Draw samples for each test variant test_samples = [] for test in test_variants: - log_test_mean = np.log(test.count + EPSILON) # Using count field to store mean value + mean_value_test = test.count / test.absolute_exposure + log_test_mean = np.log(mean_value_test + EPSILON) kappa_n_test = KAPPA_0 + test.absolute_exposure mu_n_test = (KAPPA_0 * MU_0 + test.absolute_exposure * log_test_mean) / kappa_n_test