Enable randomization (#184)

* Fix randomization to work with negative weights

* Fix tests to take into account randomization

* Remove experiment prob from settings

docs/randomized_tails.md

@@ -1,8 +1,5 @@
 # Randomized tail selection of addons
 
-The `TAAR_EXPERIMENT_PROB` sets a probability that a user is in an experiment
-to get randomized recommendations.
-
 Randomized recommendations does not mean that recommendations are
 fully randomized.  Weights for each recommendation are normalized to
 so that the sum of weights equals 1.0.
@@ -10,7 +7,3 @@ so that the sum of weights equals 1.0.
 Using `numpy.random.choice` - we then select a non-uniform random
 sample from the list of suggestions without replacement.  Weights are
 used to define a vector of probabilities.
-
-
-By default - TAAR_EXPERIMENT_PROB is set to 0.0 which in effect
-disables the randomization feature.

taar/recommenders/randomizer.py

@@ -28,7 +28,7 @@ def reorder_guids(guid_weight_tuples, size=None):
     if guid_weight_tuples is None or len(guid_weight_tuples) == 0:
         return []
 
-    weight_list = [weight for (guid, weight) in guid_weight_tuples]
+    weights = np.array([weight for (guid, weight) in guid_weight_tuples])
     guids = [guid for (guid, weight) in guid_weight_tuples]
     guid_map = dict(zip(guids, guid_weight_tuples))
 
@@ -36,8 +36,9 @@ def reorder_guids(guid_weight_tuples, size=None):
         size = len(guids)
 
     # Normalize the weights so that they're probabilities
-    total_weight = sum(weight_list)
-    probabilities = [w * 1.0 / total_weight for w in weight_list]
+    # Scale first, weights can be negative (for example, collaborative filtering similarity scores)
+    scaled_weights = weights - np.min(weights) + np.finfo(float).eps
+    probabilities = scaled_weights / np.sum(scaled_weights)
 
     choices = np.random.choice(guids, size=size, replace=False, p=probabilities)
     return [guid_map[guid] for guid in choices]

taar/recommenders/recommendation_manager.py

@@ -6,16 +6,13 @@ from taar.recommenders.ensemble_recommender import (
     EnsembleRecommender,
     is_test_client,
 )
-from taar.recommenders.randomizer import in_experiment, reorder_guids
+from taar.recommenders.randomizer import reorder_guids
 from taar.recommenders.debug import log_timer_info
 from srgutil.interfaces import IMozLogging
 from taar.recommenders.redis_cache import TAARCache
 
-from taar.settings import TAAR_EXPERIMENT_PROB
-
 import markus
 
-
 metrics = markus.get_metrics("taar")
 
 
@@ -60,8 +57,6 @@ class RecommendationManager:
 
         self._redis_cache = TAARCache.get_instance(self._ctx)
 
-        self._experiment_prob = ctx.get("TAAR_EXPERIMENT_PROB", TAAR_EXPERIMENT_PROB)
-
     @metrics.timer_decorator("profile_recommendation")
     def recommend(self, client_id, limit, extra_data={}):
         """Return recommendations for the given client.
@@ -79,8 +74,6 @@ class RecommendationManager:
             with log_timer_info("redis read", self.logger):
                 extra_data["cache"] = self._redis_cache.cache_context()
 
-            results = None
-
             if is_test_client(client_id):
                 # Just create a stub client_info blob
                 client_info = {
@@ -96,21 +89,14 @@ class RecommendationManager:
                     )
                     return []
 
-            if in_experiment(client_id, self._experiment_prob):
-                if results is None:
-                    # Fetch back all possible whitelisted addons for this
-                    # client
-                    extra_data["guid_randomization"] = True
-                    whitelist = extra_data["cache"]["whitelist"]
-                    results = self._ensemble_recommender.recommend(
-                        client_info, len(whitelist), extra_data
-                    )
+            # Fetch back all possible whitelisted addons for this
+            # client
+            extra_data["guid_randomization"] = True
+            whitelist = extra_data["cache"]["whitelist"]
+            results = self._ensemble_recommender.recommend(
+                client_info, len(whitelist), extra_data
+            )
 
-                results = reorder_guids(results, limit)
-            else:
-                if results is None:
-                    results = self._ensemble_recommender.recommend(
-                        client_info, limit, extra_data
-                    )
+            results = reorder_guids(results, limit)
 
             return results

taar/settings.py

@@ -45,8 +45,6 @@ TAAR_SIMILARITY_LRCURVES_KEY = config(
     "TAAR_SIMILARITY_LRCURVES_KEY", default="test_similarity_lrcurves_key"
 )
 
-TAAR_EXPERIMENT_PROB = config("TAAR_EXPERIMENT_PROB", default=0.0)
-
 
 # TAAR-lite configuration below
 

tests/test_randomizer.py

@@ -26,20 +26,23 @@ def test_reorder_guids():
     np.random.seed(seed=42)
 
     guid_weight_tuples = [
-        ("guid1", 0.01),
+        ("guid0", -0.60),
+        ("guid1", -0.30),
         ("guid2", 0.09),
         ("guid3", 0.30),
-        ("guid4", 0.60),
+        ("guid4", 2.5),
     ]
 
     # Run this 100 times to get the average ordering
     results = []
+    limit = 4
     for i in range(100):
-        results.append(reorder_guids(guid_weight_tuples))
+        results.append(reorder_guids(guid_weight_tuples, size=limit))
 
     best_result = []
-    for i in range(4):
+    for i in range(limit):
         best_result.append(most_frequent([row[i] for row in results])[0])
+
     assert best_result == ["guid4", "guid3", "guid2", "guid1"]
 
 

tests/test_recommendation_manager.py

@@ -17,6 +17,7 @@ from .mocks import MockRecommenderFactory
 import operator
 from functools import reduce
 
+import numpy as np
 from markus import TIMING
 from markus.testing import MetricsMock
 
@@ -116,19 +117,22 @@ def test_none_profile_returns_empty_list(test_ctx):
 
 
 def test_simple_recommendation(test_ctx):
-    with mock_install_mock_curated_data(test_ctx):
+    # Fix the random seed so that we get stable results between test
+    # runs
+    np.random.seed(seed=42)
 
+    with mock_install_mock_curated_data(test_ctx):
         EXPECTED_RESULTS = [
-            ("ghi", 3430.0),
             ("def", 3320.0),
-            ("ijk", 3200.0),
-            ("hij", 3100.0),
-            ("lmn", 420.0),
             ("klm", 409.99999999999994),
+            ("hij", 3100.0),
+            ("ijk", 3200.0),
+            ("ghi", 3430.0),
+            ("lmn", 420.0),
             ("jkl", 400.0),
             ("abc", 23.0),
             ("fgh", 22.0),
-            ("efg", 21.0),
+            ("efg", 21.0)
         ]
 
         with MetricsMock() as mm: