feat(experiments): Apply new Trends continuous stats methods

[danielbachhuber]

See #26713

Changes

Introduces a new set of methods for calculating Trends continuous values:

• calculate_probabilities_v2_continuous
• are_results_significant_v2_continuous
• calculate_credible_intervals_v2_continuous
• calculate_expected_loss_v2_continuous

Uses the Trends continuous methods when stats_version=2.

Also introduces expected loss calculation for Trends count.

How did you test this code?

Tests should pass.

[danielbachhuber]

@andehen It would be really helpful if you could produce some sample values with your notebook. I can then implement those in tests and we can see where we're at for the log_variance value.

[danielbachhuber]

It would be really helpful if you could produce some sample values with your notebook. I can then implement those in tests and we can see where we're at for the log_variance value.

@andehen log_variance = 2 seems to work well for these test cases. I'd be curious to hear your opinion on that, and what you think the delta value should be for these tests.

[jurajmajerik]

Looks great! Why not already plug it into the query runner though?

posthog/posthog/hogql_queries/experiments/experiment_trends_query_runner.py

Lines 318 to 319 in f34bea5

	if self.stats_version == 2:
	probabilities = calculate_probabilities_v2_count(control_variant, test_variants)

[danielbachhuber]

Why not already plug it into the query runner though?

My thought on Friday was to ship what I have incrementally.

[andehen]

Looks good! 👍 Didn't spot any errors in the code it self.

As we have talked about, the key issue to address further is the assumption of a fixed variance. If one run test now, where the actual variance in the data is higher, we will claim that the results are significant when they should not be. We can use a higher value to protect against this, at the cost of saying "not significant" when we likely could (in the event that variance is lower than we assume). Really hard to say what the value should be without doing an analysis on actual data 🤷

[jurajmajerik]

Looks great, thanks for doing this!

@andehen, could you please create an issue with a detailed description to track the concern mentioned in your comment? It's important to keep track of this, though I consider it non-blocking for now. This calculation is already a big improvement over the current state of things :)

[jurajmajerik]

@danielbachhuber there's an issue here I've only spotted now:

https://github.com/PostHog/posthog/blob/master/posthog/hogql_queries/experiments/experiment_trends_query_runner.py#L134-L138

We're still overwriting the math property from AVG to SUM. So the new probability method will never receive AVG. This went unnoticed because we're only unit-testing the probability method, and not including it in a more integrated query runner test. Would be nice to have a small test within test_experiment_trends_query_runner.py to ensure the correct AVG value is received.

[danielbachhuber]

there's an issue here I've only spotted now:

Resolved with #27133