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

Experimentation backend #7492

Merged
merged 12 commits into from
Dec 9, 2021
16 changes: 12 additions & 4 deletions .github/workflows/plugin-server-ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,9 @@ jobs:
python-version: 3.8

- name: Install SAML (python3-saml) dependencies
run: sudo apt-get install libxml2-dev libxmlsec1-dev libxmlsec1-openssl
run: |
macobo marked this conversation as resolved.
Show resolved Hide resolved
sudo apt-get update
sudo apt-get install libxml2-dev libxmlsec1-dev libxmlsec1-openssl

- name: Set up Node 14
uses: actions/setup-node@v2
Expand Down Expand Up @@ -139,7 +141,9 @@ jobs:
python-version: 3.8

- name: Install SAML (python3-saml) dependencies
run: sudo apt-get install libxml2-dev libxmlsec1-dev libxmlsec1-openssl
run: |
sudo apt-get update
sudo apt-get install libxml2-dev libxmlsec1-dev libxmlsec1-openssl

- name: Set up Node 14
uses: actions/setup-node@v2
Expand Down Expand Up @@ -219,7 +223,9 @@ jobs:
python-version: 3.8

- name: Install SAML (python3-saml) dependencies
run: sudo apt-get install libxml2-dev libxmlsec1-dev libxmlsec1-openssl
run: |
sudo apt-get update
sudo apt-get install libxml2-dev libxmlsec1-dev libxmlsec1-openssl

- name: Set up Node 14
uses: actions/setup-node@v2
Expand Down Expand Up @@ -300,7 +306,9 @@ jobs:
python-version: 3.8

- name: Install SAML (python3-saml) dependencies
run: sudo apt-get install libxml2-dev libxmlsec1-dev libxmlsec1-openssl
run: |
sudo apt-get update
sudo apt-get install libxml2-dev libxmlsec1-dev libxmlsec1-openssl

- name: Set up Node 14
uses: actions/setup-node@v2
Expand Down
Empty file.
100 changes: 100 additions & 0 deletions ee/clickhouse/queries/experiments/funnel_experiment_result.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,100 @@
import dataclasses
from datetime import datetime
from typing import List, Optional, Tuple, Type

from numpy.random import default_rng
from rest_framework.exceptions import ValidationError

from ee.clickhouse.queries.funnels import ClickhouseFunnel, funnel
from posthog.models.filters.filter import Filter
from posthog.models.team import Team


@dataclasses.dataclass
class Variant:
name: str
success_count: int
failure_count: int


SIMULATION_COUNT = 100_000


class ClickhouseFunnelExperimentResult:
def __init__(
self,
filter: Filter,
team: Team,
feature_flag: str,
experiment_start_date: datetime,
experiment_end_date: Optional[datetime] = None,
funnel_class: Type[ClickhouseFunnel] = ClickhouseFunnel,
):

breakdown_key = f"$feature/{feature_flag}"

query_filter = filter.with_data(
{
"date_from": experiment_start_date,
"date_to": experiment_end_date,
"breakdown": breakdown_key,
Copy link
Contributor

Choose a reason for hiding this comment

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

Should this use new multi-breakdowns?

"breakdown_type": "event",
"properties": [
{"key": breakdown_key, "value": ["control", "test"], "operator": "exact", "type": "event"}
],
# don't rely on properties used to set filters, taken over by $feature/X event properties
neilkakkar marked this conversation as resolved.
Show resolved Hide resolved
# instead, filter by prop names we expect to see: the variant values
}
)
self.funnel = funnel_class(query_filter, team)

def get_results(self):
funnel_results = self.funnel.run()
variants = self.get_variants(funnel_results)

probability = self.calculate_results(variants)

return {"funnel": funnel_results, "probability": probability}

def get_variants(self, funnel_results):
variants = []
for result in funnel_results:
total = sum([step["count"] for step in result])
success = result[-1]["count"]
failure = total - success
breakdown_value = result[0]["breakdown_value"][0]
Copy link
Contributor

Choose a reason for hiding this comment

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

Will this work for multi-breakdowns? Seems really really suspect.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Nope, won't work for multi-breakdowns, but since we're creating the funnel ourselves, we know that there isn't a multi-breakdown.

Hmm, this is valuable feedback, I've been wrangling with this for a few days now, so much that I've forgotten the fresh-eyes look. Will add more documentation at the top of the class to explain what's going on.

I didn't want to move this yet to use breakdown vs breakdowns as there seem to be some inconsistencies / bugs around this. Will update once we get rid of that tech debt.


variants.append(Variant(breakdown_value, success, failure))

# Default variant names: control and test
return sorted(variants, key=lambda variant: variant.name, reverse=True)

@staticmethod
Copy link
Contributor

Choose a reason for hiding this comment

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

This doesn't need to be in a class. Extract this and cover it with extensive tests instead which don't need any database setup.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I do test this without any DB setup, but ok with extracting it out of the class as well!

def calculate_results(
neilkakkar marked this conversation as resolved.
Show resolved Hide resolved
variants: List[Variant], priors: Tuple[int, int] = (1, 1), simulations: int = SIMULATION_COUNT
neilkakkar marked this conversation as resolved.
Show resolved Hide resolved
):
# Calculates probability that A is better than B
if len(variants) > 2:
raise ValidationError("Can't calculate A/B test results for more than 2 variants")

if len(variants) < 2:
raise ValidationError("Can't calculate A/B test results for less than 2 variants")

prior_success, prior_failure = priors

random_sampler = default_rng()
variant_samples = []
for variant in variants:
# Get `N=simulations` samples from a Beta distribution with alpha = prior_success + variant_sucess,
# and beta = prior_failure + variant_failure
samples = random_sampler.beta(
variant.success_count + prior_success, variant.failure_count + prior_failure, simulations
)
# print(samples)
variant_samples.append(samples)

probability = sum([int(sample_a > sample_b) for (sample_a, sample_b) in zip(*variant_samples)]) / simulations
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit:

        probability = sum(sample_a > sample_b for sample_a, sample_b in zip(*variant_samples)) / simulations

It's also not obvious from the code that variant_samples has length of 2 - why is that?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Makes a lot of sense.

And variant_samples is 2 because we're controlling for it at the top of this function. It doesn't work as is for more than 2 variants, the probability formula would need to change for that.
(Plus, since the funnel breakdown values is limited to 2 values (in init) we shouldn't see any more values.


# histogram_values = [ sample_a / sample_b for (sample_a, sample_b) in zip(*variant_samples)]
neilkakkar marked this conversation as resolved.
Show resolved Hide resolved

return probability
13 changes: 13 additions & 0 deletions ee/clickhouse/queries/experiments/test_funnel_experiment_result.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
import unittest

from ee.clickhouse.queries.experiments.funnel_experiment_result import ClickhouseFunnelExperimentResult, Variant


class TestFunnelExperimentCalculator(unittest.TestCase):
def test_calculate_results(self):

variant_a = Variant("A", 100, 10)
variant_b = Variant("B", 100, 18)

probability = ClickhouseFunnelExperimentResult.calculate_results([variant_a, variant_b])
self.assertTrue(probability > 0.9)
142 changes: 142 additions & 0 deletions ee/clickhouse/views/experiments.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,142 @@
from typing import Any

from rest_framework import request, serializers, viewsets
from rest_framework.decorators import action
from rest_framework.exceptions import ValidationError
from rest_framework.permissions import IsAuthenticated
from rest_framework.request import Request
from rest_framework.response import Response

from ee.clickhouse.queries.experiments.funnel_experiment_result import ClickhouseFunnelExperimentResult
from posthog.api.routing import StructuredViewSetMixin
from posthog.models.experiment import Experiment
from posthog.models.feature_flag import FeatureFlag
from posthog.models.filters.filter import Filter
from posthog.models.team import Team
from posthog.permissions import ProjectMembershipNecessaryPermissions, TeamMemberAccessPermission


class ExperimentSerializer(serializers.ModelSerializer):

feature_flag_key = serializers.CharField(source="get_feature_flag_key")
neilkakkar marked this conversation as resolved.
Show resolved Hide resolved

class Meta:
model = Experiment
fields = [
"id",
"name",
"description",
"start_date",
"end_date",
"feature_flag_key",
"parameters",
"filters",
"created_by",
"created_at",
"updated_at",
]
read_only_fields = [
"id",
"created_by",
"created_at",
"updated_at",
]

def validate_feature_flag_key(self, value):
if FeatureFlag.objects.filter(key=value, team_id=self.context["team_id"], deleted=False).exists():
raise ValidationError("Feature Flag key already exists. Please select a unique key")
neilkakkar marked this conversation as resolved.
Show resolved Hide resolved

return value

def create(self, validated_data: dict, *args: Any, **kwargs: Any) -> Experiment:
request = self.context["request"]
validated_data["created_by"] = request.user
team = Team.objects.get(id=self.context["team_id"])

feature_flag_key = validated_data.pop("get_feature_flag_key")

is_draft = "start_date" in validated_data

properties = validated_data["filters"].get("properties", [])
filters = {
"groups": [{"properties": properties, "rollout_percentage": None}],
"multivariate": {
"variants": [
{"key": "control", "name": "Control Group", "rollout_percentage": 50},
{"key": "test", "name": "Test Variant", "rollout_percentage": 50},
]
},
}

feature_flag = FeatureFlag.objects.create(
key=feature_flag_key,
name=f'Feature Flag for Experiment {validated_data["name"]}',
team=team,
created_by=request.user,
filters=filters,
is_experiment=True,
Copy link
Contributor

Choose a reason for hiding this comment

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

This field is unneeded. We could just look up which feature flags are referred to by experiments.

It's better to stay in the third normal form if possible.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Ha, makes sense! Only other consideration is that on the FF list page, we'd be checking & joining the experiments table. But I guess that's okay, this is going to be a small table forever-ish.

active=False if is_draft else True,
)

experiment = Experiment.objects.create(team=team, feature_flag=feature_flag, **validated_data)
return experiment

def update(self, instance: Experiment, validated_data: dict, *args: Any, **kwargs: Any) -> Experiment:

expected_keys = set(["name", "description", "start_date", "end_date", "parameters"])
given_keys = set(validated_data.keys())

extra_keys = given_keys - expected_keys

if extra_keys:
raise ValidationError(f"Can't update keys: {', '.join(sorted(extra_keys))} on Experiment")

is_draft = not instance.start_date
has_start_date = "start_date" in validated_data

feature_flag = instance.feature_flag

if is_draft and has_start_date:
feature_flag.active = True
feature_flag.save()
return super().update(instance, validated_data)

elif has_start_date:
raise ValidationError("Can't change experiment start date after experiment has begun")
else:
# Not a draft, doesn't have start date
# Or draft without start date
return super().update(instance, validated_data)


class ClickhouseExperimentsViewSet(StructuredViewSetMixin, viewsets.ModelViewSet):
serializer_class = ExperimentSerializer
queryset = Experiment.objects.all()
permission_classes = [IsAuthenticated, ProjectMembershipNecessaryPermissions, TeamMemberAccessPermission]

def get_queryset(self):
return super().get_queryset()

# ******************************************
# /projects/:id/experiments/:experiment_id/results
#
# Returns current results of an experiment, and graphs
# 1. Probability of success
# 2. Funnel breakdown graph to display
# 3. (?): Histogram of possible values - bucketed on backend
# ******************************************
@action(methods=["GET"], detail=True)
def results(self, request: Request, *args: Any, **kwargs: Any) -> Response:
experiment: Experiment = self.get_object()

if not experiment.filters:
raise ValidationError("Experiment has no target metric")

result = ClickhouseFunnelExperimentResult(
Filter(experiment.filters),
self.team,
experiment.feature_flag.key,
experiment.start_date,
experiment.end_date,
).get_results()
return Response(result)
Loading