refactoring

Author: oksanadanilova

topicnet/cooking_machine/cubes/controller_cube.py

@@ -19,7 +19,9 @@
     and we are allowed to change tau coefficient further; otherwise we revert back
     to the last "safe" value and stop
     
-    'sort of decreasing' perform best with PerplexityScore.
+    'sort of decreasing' performs best with `PerplexityScore`, and all scores which 
+    behave like perplexity (nonnegative, and which should decrease when a model gets better).
+    If you want to track a different kind of score, it is recommended to use `score_controller` parameter
 
     More formal definition of "sort of decreasing": if we divide a curve into two parts like so:
 
@@ -50,16 +52,14 @@
     then the right part is no higher than 5% of global minimum
     (you can change 5% if you like by adjusting `fraction_threshold` parameter)
 
-    If score_to_track is None and score_controller is None, then `ControllerAgent` will never stop
+    If `score_to_track` is None and `score_controller` is None, then `ControllerAgent` will never stop
     (useful for e.g. decaying coefficients)
 fraction_threshold: float
     Threshold to control a score by 'sort of decreasing' metric
-score_controller: ScoreControllerBase
+score_controller: BaseScoreController
     Custom score controller
     In case of 'sort of decreasing' is not proper to control score, you are able to create custom Score Controller 
-    inherited from ScoreControllerBase. New controller must contain function `is_score_out_of_control`
-     which takes TopicModel as input and return answer of type OutOfControlAnswer
-
+    inherited from `BaseScoreController`.
 tau_converter: str or callable
     Notably, def-style functions and lambda functions are allowed
     If it is function, then it should accept four arguments:
@@ -135,15 +135,15 @@
 
 @dataclass
 class OutOfControlAnswer:
-    answer: Optional[bool]
+    answer: bool
     error_message: Optional[str] = None
 
 
-class ScoreControllerBase:
+class BaseScoreController:
     def __init__(self, score_name):
         self.score_name = score_name
 
-    def get_vals(self, model):
+    def get_score_values(self, model):
         if self.score_name not in model.scores:  # case of None is handled here as well
             return None
 
@@ -154,7 +154,7 @@ def get_vals(self, model):
         return vals
 
     def __call__(self, model):
-        values = self.get_vals(model)
+        values = self.get_score_values(model)
 
         if values is None:
             return False
@@ -163,8 +163,7 @@ def __call__(self, model):
             out_of_control_result = self.is_out_of_control(values)
         except Exception as ex:
             message = (f"An error occured while controlling {self.score_name}. Message: {ex}. Score values: {values}")
-            warnings.warn(message)
-            return True
+            raise ValueError(message)
 
         if out_of_control_result.error_message is not None:
             warnings.warn(out_of_control_result.error_message)
@@ -175,9 +174,9 @@ def is_out_of_control(self, values: List[float]) -> OutOfControlAnswer:
         raise NotImplementedError
 
 
-class ScoreControllerPerplexity(ScoreControllerBase):
+class PerplexityScoreController(BaseScoreController):
     """
-        Controller is properto control the Perplexity score. For others, please ensure for yourself.
+    Controller is proper to control the Perplexity score. For others, please ensure for yourself.
     """
     DEFAULT_FRACTION_THRESHOLD = 0.05
 
@@ -195,10 +194,10 @@ def is_out_of_control(self, values: List[float]):
         minval = values[idxmin]
 
         if minval <= 0:
-            message = f"""Score {self.score_name} has min_value = {minval} which is <=0. 
+            err_message = f"""Score {self.score_name} has min_value = {minval} which is <= 0. 
                         This control scheme is using to control scores acting like Perplexity.
                         Ensure you control the Perplexity score or write your own controller"""
-            return OutOfControlAnswer(answer=True, error_message=message)
+            raise ValueError(err_message)
 
         answer = (right_maxval - minval) / minval > self.fraction_threshold
 
@@ -248,13 +247,13 @@ def __init__(self, reg_name, tau_converter, max_iters, score_to_track=None, frac
             `max_iters` could be `float("NaN")` and `float("inf")` values:
             that way agent will continue operating even outside this `RegularizationControllerCube`
         score_to_track : str, list of str or None
-            Name of score to track
+            Name of score to track.
             Please, use this definition to track only scores of type PerplexityScore.
             In other cases we recommend you to write you own ScoreController
         fraction_threshold : float, list of float of the same length as score_to_track  or None
             Uses to define threshold to control PerplexityScore
             Default value is 0.05
-        score_controller : ScoreControllerBase, list of ScoreControllerBase or None
+        score_controller : BaseScoreController, list of BaseScoreController or None
         local_dict : dict
         """
         if local_dict is None:
@@ -266,27 +265,29 @@ def __init__(self, reg_name, tau_converter, max_iters, score_to_track=None, frac
         self.score_controllers = []
         if isinstance(score_to_track, list):
             if fraction_threshold is None:
-                scores = [(ScoreControllerPerplexity.DEFAULT_FRACTION_THRESHOLD, name) for name in score_to_track]
+                controller_params = [(name, PerplexityScoreController.DEFAULT_FRACTION_THRESHOLD) for name in
+                                     score_to_track]
             elif isinstance(fraction_threshold, list) and len(score_to_track) == len(fraction_threshold):
-                scores = list(zip(score_to_track, fraction_threshold))
+                controller_params = list(zip(score_to_track, fraction_threshold))
             else:
                 err_message = """Length of score_to_track and fraction_threshold must be same.
                 Otherwise fraction_threshold must be None"""
                 raise ControllerAgentException(err_message)
 
-            self.score_controllers.append([ScoreControllerPerplexity(name, threshold) for (name, threshold) in scores])
+            self.score_controllers.append(
+                [PerplexityScoreController(name, threshold) for (name, threshold) in controller_params])
 
         elif isinstance(score_to_track, str):
-            self.score_controllers.append([ScoreControllerPerplexity(
+            self.score_controllers.append([PerplexityScoreController(
                 score_to_track,
-                fraction_threshold or ScoreControllerPerplexity.DEFAULT_FRACTION_THRESHOLD
+                fraction_threshold or PerplexityScoreController.DEFAULT_FRACTION_THRESHOLD
             )])
 
-        if isinstance(score_controller, ScoreControllerBase):
+        if isinstance(score_controller, BaseScoreController):
             self.score_controllers.append(score_controller)
         elif isinstance(score_controller, list):
-            if not all(isinstance(score, ScoreControllerBase) for score in score_controller):
-                err_message = """score_controller must be of type ScoreControllerBase od list of ScoreControllerBase"""
+            if not all(isinstance(score, BaseScoreController) for score in score_controller):
+                err_message = """score_controller must be of type BaseScoreController or list of BaseScoreController"""
                 raise ControllerAgentException(err_message)
 
             self.score_controllers.extend(score_controller)
@@ -389,7 +390,7 @@ def __init__(self, num_iter: int, parameters,
                     >>   )
                     >>   "score_to_track": None,
                     >>   "fraction_threshold": None,
-                    >>   "score_controller": [ScoreControllerPerplexity("PerplexityScore@all", 0.1)],
+                    >>   "score_controller": [PerplexityScoreController("PerplexityScore@all", 0.1)],
                     >>   "user_value_grid": [0, 1]}
 
         reg_search : str

topicnet/tests/fixtures.py

@@ -1,12 +1,65 @@
 import pytest
 
-from topicnet.cooking_machine.cubes.controller_cube import ScoreControllerPerplexity, ControllerAgent
-from .fixtures import DATA_AGENT_CONTROLLER_LEN_CHECK, DATA_REG_CONTROLLER_SORT_OF_DECREASING
+from topicnet.cooking_machine.cubes.controller_cube import PerplexityScoreController, ControllerAgent
+
+DATA_REG_CONTROLLER_SORT_OF_DECREASING = [
+    ([246.77072143554688,
+      124.72193908691406,
+      107.95775604248047,
+      105.27597045898438,
+      112.46900939941406,
+      132.88259887695312], 0.1, True),
+    ([246.77072143554688,
+      124.72193908691406,
+      107.95775604248047,
+      105.27597045898438,
+      112.46900939941406], 0.1, False),
+    ([246.77072143554688,
+      124.72193908691406,
+      107.95775604248047,
+      105.27597045898438,
+      112.46900939941406], 0.05, True),
+
+]
+DATA_AGENT_CONTROLLER_LEN_CHECK = [
+    ({
+         "reg_name": "decorrelation",
+         "score_to_track": "PerplexityScore@all",
+         "tau_converter": "prev_tau * user_value",
+         "max_iters": float("inf")
+     }, 1),
+    ({
+         "reg_name": "decorrelation",
+         "score_to_track": ["PerplexityScore@all"],
+         "tau_converter": "prev_tau + user_value",
+         "max_iters": float("inf")
+     }, 1),
+    ({
+         "reg_name": "decorrelation",
+         "score_to_track": None,  # never stop working
+         "tau_converter": "prev_tau * user_value",
+         "max_iters": float("inf")
+     }, 0),
+    ({
+         "reg_name": "decorrelation",
+         "score_to_track": None,  # never stop working
+         "score_controller": PerplexityScoreController("PerplexityScore@all", 0.1),
+         "tau_converter": "prev_tau * user_value",
+         "max_iters": float("inf")
+     }, 1),
+    ({
+         "reg_name": "decorrelation",
+         "score_to_track": "PerplexityScore@all",  # never stop working
+         "score_controller": PerplexityScoreController("PerplexityScore@all", 0.1),
+         "tau_converter": "prev_tau * user_value",
+         "max_iters": float("inf")
+     }, 2)
+]
 
 
 @pytest.mark.parametrize('values, fraction, answer_true', DATA_REG_CONTROLLER_SORT_OF_DECREASING)
-def test_get_two_values_diff(values, fraction, answer_true):
-    score_controller = ScoreControllerPerplexity('test', fraction)
+def test_perplexity_controller(values, fraction, answer_true):
+    score_controller = PerplexityScoreController('test', fraction)
     is_out_of_control = score_controller.is_out_of_control(values)
 
     assert is_out_of_control.answer == answer_true