From c63f592a8202fbba948f7f9479f13c505eba6757 Mon Sep 17 00:00:00 2001 From: Thomas Werkmeister Date: Tue, 17 Aug 2021 17:21:26 +0200 Subject: [PATCH 01/41] Adapted memoization policies for graph --- rasa/core/policies/_memoization.py | 369 +++++++++++++++++++++++++++++ rasa/core/policies/memoization.py | 115 +++++++-- 2 files changed, 459 insertions(+), 25 deletions(-) create mode 100644 rasa/core/policies/_memoization.py diff --git a/rasa/core/policies/_memoization.py b/rasa/core/policies/_memoization.py new file mode 100644 index 000000000000..107da9a94433 --- /dev/null +++ b/rasa/core/policies/_memoization.py @@ -0,0 +1,369 @@ +# WARNING: This module will be dropped before Rasa Open Source 3.0 is released. +# Please don't do any changes in this module and rather adapt `MemoizationPolicyGraphComponent` from +# the regular `rasa.core.policies.memoization` module. This module is a +# workaround to defer breaking changes due to the architecture revamp in 3.0. +# flake8: noqa +import zlib + +import base64 +import json +import logging + +from tqdm import tqdm +from typing import Optional, Any, Dict, List, Text + +import rasa.utils.io +import rasa.shared.utils.io +from rasa.shared.core.domain import State, Domain +from rasa.shared.core.events import ActionExecuted +from rasa.core.featurizers.tracker_featurizers import ( + TrackerFeaturizer, + MaxHistoryTrackerFeaturizer, +) +from rasa.shared.nlu.interpreter import NaturalLanguageInterpreter +from rasa.core.policies.policy import Policy, PolicyPrediction +from rasa.shared.core.trackers import DialogueStateTracker +from rasa.shared.core.generator import TrackerWithCachedStates +from rasa.shared.utils.io import is_logging_disabled +from rasa.core.constants import MEMOIZATION_POLICY_PRIORITY, DEFAULT_MAX_HISTORY + +logger = logging.getLogger(__name__) + + +class MemoizationPolicy(Policy): + """A policy that follows exact examples of `max_history` turns in training stories. + + Since `slots` that are set some time in the past are + preserved in all future feature vectors until they are set + to None, this policy implicitly remembers and most importantly + recalls examples in the context of the current dialogue + longer than `max_history`. + + This policy is not supposed to be the only policy in an ensemble, + it is optimized for precision and not recall. + It should get a 100% precision because it emits probabilities of 1.1 + along it's predictions, which makes every mistake fatal as + no other policy can overrule it. + + If it is needed to recall turns from training dialogues where + some slots might not be set during prediction time, and there are + training stories for this, use AugmentedMemoizationPolicy. + """ + + ENABLE_FEATURE_STRING_COMPRESSION = True + + USE_NLU_CONFIDENCE_AS_SCORE = False + + @staticmethod + def _standard_featurizer( + max_history: Optional[int] = None, + ) -> MaxHistoryTrackerFeaturizer: + # Memoization policy always uses MaxHistoryTrackerFeaturizer + # without state_featurizer + return MaxHistoryTrackerFeaturizer( + state_featurizer=None, max_history=max_history + ) + + def __init__( + self, + featurizer: Optional[TrackerFeaturizer] = None, + priority: int = MEMOIZATION_POLICY_PRIORITY, + max_history: Optional[int] = DEFAULT_MAX_HISTORY, + lookup: Optional[Dict] = None, + **kwargs: Any, + ) -> None: + """Initialize the policy. + + Args: + featurizer: tracker featurizer + priority: the priority of the policy + max_history: maximum history to take into account when featurizing trackers + lookup: a dictionary that stores featurized tracker states and + predicted actions for them + """ + if not featurizer: + featurizer = self._standard_featurizer(max_history) + + super().__init__(featurizer, priority, **kwargs) + + self.max_history = self.featurizer.max_history + self.lookup = lookup if lookup is not None else {} + + def _create_lookup_from_states( + self, + trackers_as_states: List[List[State]], + trackers_as_actions: List[List[Text]], + ) -> Dict[Text, Text]: + """Creates lookup dictionary from the tracker represented as states. + + Args: + trackers_as_states: representation of the trackers as a list of states + trackers_as_actions: representation of the trackers as a list of actions + + Returns: + lookup dictionary + """ + + lookup = {} + + if not trackers_as_states: + return lookup + + assert len(trackers_as_actions[0]) == 1, ( + f"The second dimension of trackers_as_action should be 1, " + f"instead of {len(trackers_as_actions[0])}" + ) + + ambiguous_feature_keys = set() + + pbar = tqdm( + zip(trackers_as_states, trackers_as_actions), + desc="Processed actions", + disable=is_logging_disabled(), + ) + for states, actions in pbar: + action = actions[0] + + feature_key = self._create_feature_key(states) + if not feature_key: + continue + + if feature_key not in ambiguous_feature_keys: + if feature_key in lookup.keys(): + if lookup[feature_key] != action: + # delete contradicting example created by + # partial history augmentation from memory + ambiguous_feature_keys.add(feature_key) + del lookup[feature_key] + else: + lookup[feature_key] = action + pbar.set_postfix({"# examples": "{:d}".format(len(lookup))}) + + return lookup + + def _create_feature_key(self, states: List[State]) -> Text: + # we sort keys to make sure that the same states + # represented as dictionaries have the same json strings + # quotes are removed for aesthetic reasons + feature_str = json.dumps(states, sort_keys=True).replace('"', "") + if self.ENABLE_FEATURE_STRING_COMPRESSION: + compressed = zlib.compress( + bytes(feature_str, rasa.shared.utils.io.DEFAULT_ENCODING) + ) + return base64.b64encode(compressed).decode( + rasa.shared.utils.io.DEFAULT_ENCODING + ) + else: + return feature_str + + def train( + self, + training_trackers: List[TrackerWithCachedStates], + domain: Domain, + interpreter: NaturalLanguageInterpreter, + **kwargs: Any, + ) -> None: + # only considers original trackers (no augmented ones) + training_trackers = [ + t + for t in training_trackers + if not hasattr(t, "is_augmented") or not t.is_augmented + ] + ( + trackers_as_states, + trackers_as_actions, + ) = self.featurizer.training_states_and_labels(training_trackers, domain) + self.lookup = self._create_lookup_from_states( + trackers_as_states, trackers_as_actions + ) + logger.debug(f"Memorized {len(self.lookup)} unique examples.") + + def _recall_states(self, states: List[State]) -> Optional[Text]: + return self.lookup.get(self._create_feature_key(states)) + + def recall( + self, states: List[State], tracker: DialogueStateTracker, domain: Domain, + ) -> Optional[Text]: + """Finds the action based on the given states. + + Args: + states: List of states. + tracker: The tracker. + domain: The Domain. + + Returns: + The name of the action. + """ + return self._recall_states(states) + + def _prediction_result( + self, action_name: Text, tracker: DialogueStateTracker, domain: Domain + ) -> List[float]: + result = self._default_predictions(domain) + if action_name: + if self.USE_NLU_CONFIDENCE_AS_SCORE: + # the memoization will use the confidence of NLU on the latest + # user message to set the confidence of the action + score = tracker.latest_message.intent.get("confidence", 1.0) + else: + score = 1.0 + + result[domain.index_for_action(action_name)] = score + + return result + + def predict_action_probabilities( + self, + tracker: DialogueStateTracker, + domain: Domain, + interpreter: NaturalLanguageInterpreter, + **kwargs: Any, + ) -> PolicyPrediction: + """Predicts the next action the bot should take after seeing the tracker. + + Args: + tracker: the :class:`rasa.core.trackers.DialogueStateTracker` + domain: the :class:`rasa.shared.core.domain.Domain` + interpreter: Interpreter which may be used by the policies to create + additional features. + + Returns: + The policy's prediction (e.g. the probabilities for the actions). + """ + result = self._default_predictions(domain) + + states = self._prediction_states(tracker, domain) + logger.debug(f"Current tracker state:{self.format_tracker_states(states)}") + predicted_action_name = self.recall(states, tracker, domain) + if predicted_action_name is not None: + logger.debug(f"There is a memorised next action '{predicted_action_name}'") + result = self._prediction_result(predicted_action_name, tracker, domain) + else: + logger.debug("There is no memorised next action") + + return self._prediction(result) + + def _metadata(self) -> Dict[Text, Any]: + return { + "priority": self.priority, + "max_history": self.max_history, + "lookup": self.lookup, + } + + @classmethod + def _metadata_filename(cls) -> Text: + return "memorized_turns.json" + + +class AugmentedMemoizationPolicy(MemoizationPolicy): + """The policy that remembers examples from training stories + for `max_history` turns. + + If it is needed to recall turns from training dialogues + where some slots might not be set during prediction time, + add relevant stories without such slots to training data. + E.g. reminder stories. + + Since `slots` that are set some time in the past are + preserved in all future feature vectors until they are set + to None, this policy has a capability to recall the turns + up to `max_history` from training stories during prediction + even if additional slots were filled in the past + for current dialogue. + """ + + @staticmethod + def _back_to_the_future( + tracker: DialogueStateTracker, again: bool = False + ) -> Optional[DialogueStateTracker]: + """Send Marty to the past to get + the new featurization for the future""" + + idx_of_first_action = None + idx_of_second_action = None + + # we need to find second executed action + for e_i, event in enumerate(tracker.applied_events()): + # find second ActionExecuted + if isinstance(event, ActionExecuted): + if idx_of_first_action is None: + idx_of_first_action = e_i + else: + idx_of_second_action = e_i + break + + # use first action, if we went first time and second action, if we went again + idx_to_use = idx_of_second_action if again else idx_of_first_action + if idx_to_use is None: + return None + + # make second ActionExecuted the first one + events = tracker.applied_events()[idx_to_use:] + if not events: + return None + + mcfly_tracker = tracker.init_copy() + for e in events: + mcfly_tracker.update(e) + + return mcfly_tracker + + def _recall_using_delorean( + self, old_states: List[State], tracker: DialogueStateTracker, domain: Domain, + ) -> Optional[Text]: + """Applies to the future idea to change the past and get the new future. + + Recursively go to the past to correctly forget slots, + and then back to the future to recall. + + Args: + old_states: List of states. + tracker: The tracker. + domain: The Domain. + + Returns: + The name of the action. + """ + logger.debug("Launch DeLorean...") + + mcfly_tracker = self._back_to_the_future(tracker) + while mcfly_tracker is not None: + states = self._prediction_states(mcfly_tracker, domain,) + + if old_states != states: + # check if we like new futures + memorised = self._recall_states(states) + if memorised is not None: + logger.debug(f"Current tracker state {states}") + return memorised + old_states = states + + # go back again + mcfly_tracker = self._back_to_the_future(mcfly_tracker, again=True) + + # No match found + logger.debug(f"Current tracker state {old_states}") + return None + + def recall( + self, states: List[State], tracker: DialogueStateTracker, domain: Domain, + ) -> Optional[Text]: + """Finds the action based on the given states. + + Uses back to the future idea to change the past and check whether the new future + can be used to recall the action. + + Args: + states: List of states. + tracker: The tracker. + domain: The Domain. + + Returns: + The name of the action. + """ + predicted_action_name = self._recall_states(states) + if predicted_action_name is None: + # let's try a different method to recall that tracker + return self._recall_using_delorean(states, tracker, domain,) + else: + return predicted_action_name diff --git a/rasa/core/policies/memoization.py b/rasa/core/policies/memoization.py index 155b5981516b..94896e1600ce 100644 --- a/rasa/core/policies/memoization.py +++ b/rasa/core/policies/memoization.py @@ -6,26 +6,46 @@ from tqdm import tqdm from typing import Optional, Any, Dict, List, Text +from pathlib import Path import rasa.utils.io import rasa.shared.utils.io +from rasa.engine.graph import ExecutionContext +from rasa.engine.storage.resource import Resource +from rasa.engine.storage.storage import ModelStorage from rasa.shared.core.domain import State, Domain from rasa.shared.core.events import ActionExecuted from rasa.core.featurizers.tracker_featurizers import ( TrackerFeaturizer, MaxHistoryTrackerFeaturizer, + FEATURIZER_FILE, ) +from rasa.shared.exceptions import FileIOException from rasa.shared.nlu.interpreter import NaturalLanguageInterpreter -from rasa.core.policies.policy import Policy, PolicyPrediction +from rasa.core.policies.policy import PolicyPrediction, PolicyGraphComponent from rasa.shared.core.trackers import DialogueStateTracker from rasa.shared.core.generator import TrackerWithCachedStates from rasa.shared.utils.io import is_logging_disabled -from rasa.core.constants import MEMOIZATION_POLICY_PRIORITY, DEFAULT_MAX_HISTORY +from rasa.core.constants import ( + MEMOIZATION_POLICY_PRIORITY, + DEFAULT_MAX_HISTORY, + POLICY_MAX_HISTORY, + POLICY_PRIORITY, +) +from rasa.core.policies._memoization import ( + MemoizationPolicy, + AugmentedMemoizationPolicy, +) + +# TODO: This is a workaround around until we have all components migrated to +# `GraphComponent`. +MemoizationPolicy = MemoizationPolicy +AugmentedMemoizationPolicy = AugmentedMemoizationPolicy logger = logging.getLogger(__name__) -class MemoizationPolicy(Policy): +class MemoizationPolicyGraphComponent(PolicyGraphComponent): """A policy that follows exact examples of `max_history` turns in training stories. Since `slots` that are set some time in the past are @@ -45,25 +65,33 @@ class MemoizationPolicy(Policy): training stories for this, use AugmentedMemoizationPolicy. """ - ENABLE_FEATURE_STRING_COMPRESSION = True + @staticmethod + def get_default_config() -> Dict[Text, Any]: + """Returns the default config (see parent class for full docstring).""" + # please make sure to update the docs when changing a default parameter + return { + "enable_feature_string_compression": True, + "use_nlu_confidence_as_score": False, + POLICY_PRIORITY: MEMOIZATION_POLICY_PRIORITY, + POLICY_MAX_HISTORY: DEFAULT_MAX_HISTORY, + } - USE_NLU_CONFIDENCE_AS_SCORE = False + required_packages = [] - @staticmethod - def _standard_featurizer( - max_history: Optional[int] = None, - ) -> MaxHistoryTrackerFeaturizer: + def _standard_featurizer(self) -> MaxHistoryTrackerFeaturizer: # Memoization policy always uses MaxHistoryTrackerFeaturizer # without state_featurizer return MaxHistoryTrackerFeaturizer( - state_featurizer=None, max_history=max_history + state_featurizer=None, max_history=self.config[POLICY_MAX_HISTORY] ) def __init__( self, + config: Dict[Text, Any], + model_storage: ModelStorage, + resource: Resource, + execution_context: ExecutionContext, featurizer: Optional[TrackerFeaturizer] = None, - priority: int = MEMOIZATION_POLICY_PRIORITY, - max_history: Optional[int] = DEFAULT_MAX_HISTORY, lookup: Optional[Dict] = None, **kwargs: Any, ) -> None: @@ -77,12 +105,13 @@ def __init__( predicted actions for them """ if not featurizer: - featurizer = self._standard_featurizer(max_history) + featurizer = self._standard_featurizer() - super().__init__(featurizer, priority, **kwargs) - - self.max_history = self.featurizer.max_history - self.lookup = lookup if lookup is not None else {} + super().__init__( + config, model_storage, resource, execution_context, featurizer, **kwargs + ) + self.config = config + self.lookup = lookup or {} def _create_lookup_from_states( self, @@ -239,20 +268,56 @@ def predict_action_probabilities( return self._prediction(result) def _metadata(self) -> Dict[Text, Any]: - return { - "priority": self.priority, - "max_history": self.max_history, - "lookup": self.lookup, - } + return {"config": self.config, "lookup": self.lookup} @classmethod def _metadata_filename(cls) -> Text: return "memorized_turns.json" + @classmethod + def load( + cls, + config: Dict[Text, Any], + model_storage: ModelStorage, + resource: Resource, + execution_context: ExecutionContext, + **kwargs: Any, + ) -> "PolicyGraphComponent": + """Loads a trained policy (see parent class for full docstring).""" + config = {} + featurizer = None + lookup = None + + try: + with model_storage.read_from(resource) as path: + metadata_file = Path(path) / cls._metadata_filename() + metadata = json.loads(rasa.shared.utils.io.read_file(metadata_file)) + config = metadata["config"] + lookup = metadata["lookup"] + + if (Path(path) / FEATURIZER_FILE).is_file(): + featurizer = TrackerFeaturizer.load(path) + + config.update(kwargs) + + except (ValueError, FileNotFoundError, FileIOException): + logger.info( + f"Couldn't load metadata for policy '{cls.__name__}' as the persisted " + f"metadata couldn't be loaded." + ) + + return cls( + config, + model_storage, + resource, + execution_context, + featurizer=featurizer, + lookup=lookup, + ) + -class AugmentedMemoizationPolicy(MemoizationPolicy): - """The policy that remembers examples from training stories - for `max_history` turns. +class AugmentedMemoizationPolicyGraphComponent(MemoizationPolicyGraphComponent): + """The policy that remembers examples from training stories for `max_history` turns. If it is needed to recall turns from training dialogues where some slots might not be set during prediction time, From 8e7be7ebbf409510d9cb7a4dff5bb24098325b00 Mon Sep 17 00:00:00 2001 From: Thomas Werkmeister Date: Wed, 18 Aug 2021 09:50:21 +0200 Subject: [PATCH 02/41] Fixed conftest import --- tests/core/conftest.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/core/conftest.py b/tests/core/conftest.py index 06495d89aecf..e0a0edc3b76c 100644 --- a/tests/core/conftest.py +++ b/tests/core/conftest.py @@ -17,7 +17,7 @@ from rasa.shared.core.domain import Domain from rasa.shared.core.events import ReminderScheduled, UserUttered, ActionExecuted from rasa.core.nlg import TemplatedNaturalLanguageGenerator, NaturalLanguageGenerator -from rasa.core.policies.memoization import Policy +from rasa.core.policies import Policy from rasa.core.processor import MessageProcessor from rasa.shared.core.slots import Slot from rasa.core.tracker_store import InMemoryTrackerStore, MongoTrackerStore From 0209f73810f211f9aa64c994a0ae9198620034ee Mon Sep 17 00:00:00 2001 From: Thomas Werkmeister Date: Wed, 18 Aug 2021 10:17:04 +0200 Subject: [PATCH 03/41] Code quality --- rasa/core/policies/memoization.py | 16 ++-------------- 1 file changed, 2 insertions(+), 14 deletions(-) diff --git a/rasa/core/policies/memoization.py b/rasa/core/policies/memoization.py index 94896e1600ce..185b40fd84af 100644 --- a/rasa/core/policies/memoization.py +++ b/rasa/core/policies/memoization.py @@ -93,23 +93,12 @@ def __init__( execution_context: ExecutionContext, featurizer: Optional[TrackerFeaturizer] = None, lookup: Optional[Dict] = None, - **kwargs: Any, ) -> None: - """Initialize the policy. - - Args: - featurizer: tracker featurizer - priority: the priority of the policy - max_history: maximum history to take into account when featurizing trackers - lookup: a dictionary that stores featurized tracker states and - predicted actions for them - """ + """Initialize the policy.""" if not featurizer: featurizer = self._standard_featurizer() - super().__init__( - config, model_storage, resource, execution_context, featurizer, **kwargs - ) + super().__init__(config, model_storage, resource, execution_context, featurizer) self.config = config self.lookup = lookup or {} @@ -127,7 +116,6 @@ def _create_lookup_from_states( Returns: lookup dictionary """ - lookup = {} if not trackers_as_states: From d809ae5f43ff345a48f65ef3cfe11bcb64a19bfc Mon Sep 17 00:00:00 2001 From: Thomas Werkmeister Date: Wed, 18 Aug 2021 15:29:58 +0200 Subject: [PATCH 04/41] Removed required packages var --- rasa/core/policies/memoization.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/rasa/core/policies/memoization.py b/rasa/core/policies/memoization.py index 185b40fd84af..b9a1cfdf9c5e 100644 --- a/rasa/core/policies/memoization.py +++ b/rasa/core/policies/memoization.py @@ -76,8 +76,6 @@ def get_default_config() -> Dict[Text, Any]: POLICY_MAX_HISTORY: DEFAULT_MAX_HISTORY, } - required_packages = [] - def _standard_featurizer(self) -> MaxHistoryTrackerFeaturizer: # Memoization policy always uses MaxHistoryTrackerFeaturizer # without state_featurizer From fdb0413301dcd3021ae3742d0d91528dc94a9f8d Mon Sep 17 00:00:00 2001 From: Thomas Werkmeister Date: Wed, 18 Aug 2021 15:35:52 +0200 Subject: [PATCH 05/41] Delegating a couple things to the super class --- rasa/core/policies/memoization.py | 4 ---- rasa/core/policies/policy.py | 1 + 2 files changed, 1 insertion(+), 4 deletions(-) diff --git a/rasa/core/policies/memoization.py b/rasa/core/policies/memoization.py index b9a1cfdf9c5e..3adc11248466 100644 --- a/rasa/core/policies/memoization.py +++ b/rasa/core/policies/memoization.py @@ -93,11 +93,7 @@ def __init__( lookup: Optional[Dict] = None, ) -> None: """Initialize the policy.""" - if not featurizer: - featurizer = self._standard_featurizer() - super().__init__(config, model_storage, resource, execution_context, featurizer) - self.config = config self.lookup = lookup or {} def _create_lookup_from_states( diff --git a/rasa/core/policies/policy.py b/rasa/core/policies/policy.py index 3a8b736683a6..0704c2c6540e 100644 --- a/rasa/core/policies/policy.py +++ b/rasa/core/policies/policy.py @@ -127,6 +127,7 @@ def __init__( featurizer = self._create_featurizer(config) self.__featurizer = featurizer + self.config = config self.priority = config.get(POLICY_PRIORITY, DEFAULT_POLICY_PRIORITY) self.finetune_mode = execution_context.is_finetuning From 967a28b432ceec85685c3f10ecc3a8064e527960 Mon Sep 17 00:00:00 2001 From: Thomas Werkmeister Date: Mon, 23 Aug 2021 09:32:05 +0200 Subject: [PATCH 06/41] Not persisting config anymore for graph policy components --- rasa/core/policies/memoization.py | 4 +--- rasa/core/policies/policy.py | 4 ---- 2 files changed, 1 insertion(+), 7 deletions(-) diff --git a/rasa/core/policies/memoization.py b/rasa/core/policies/memoization.py index 3adc11248466..f17ab83b5452 100644 --- a/rasa/core/policies/memoization.py +++ b/rasa/core/policies/memoization.py @@ -250,7 +250,7 @@ def predict_action_probabilities( return self._prediction(result) def _metadata(self) -> Dict[Text, Any]: - return {"config": self.config, "lookup": self.lookup} + return {"lookup": self.lookup} @classmethod def _metadata_filename(cls) -> Text: @@ -266,7 +266,6 @@ def load( **kwargs: Any, ) -> "PolicyGraphComponent": """Loads a trained policy (see parent class for full docstring).""" - config = {} featurizer = None lookup = None @@ -274,7 +273,6 @@ def load( with model_storage.read_from(resource) as path: metadata_file = Path(path) / cls._metadata_filename() metadata = json.loads(rasa.shared.utils.io.read_file(metadata_file)) - config = metadata["config"] lookup = metadata["lookup"] if (Path(path) / FEATURIZER_FILE).is_file(): diff --git a/rasa/core/policies/policy.py b/rasa/core/policies/policy.py index 0704c2c6540e..f2222851c75f 100644 --- a/rasa/core/policies/policy.py +++ b/rasa/core/policies/policy.py @@ -435,14 +435,10 @@ def load( **kwargs: Any, ) -> "PolicyGraphComponent": """Loads a trained policy (see parent class for full docstring).""" - config = {} featurizer = None try: with model_storage.read_from(resource) as path: - metadata_file = Path(path) / cls._metadata_filename() - config = json.loads(rasa.shared.utils.io.read_file(metadata_file)) - if (Path(path) / FEATURIZER_FILE).is_file(): featurizer = TrackerFeaturizer.load(path) From ccfea81ae172014977b36ec9935db687a0777f0c Mon Sep 17 00:00:00 2001 From: Thomas Werkmeister Date: Mon, 23 Aug 2021 14:00:36 +0200 Subject: [PATCH 07/41] adjusted unit testing for memoization graph component --- rasa/core/policies/memoization.py | 7 +- rasa/core/policies/policy.py | 14 +- rasa/core/policies/ted_policy.py | 9 +- tests/core/policies/test_ted_policy.py | 48 +++++- tests/core/test_policies.py | 226 +++++++++++++++++++------ 5 files changed, 231 insertions(+), 73 deletions(-) diff --git a/rasa/core/policies/memoization.py b/rasa/core/policies/memoization.py index f17ab83b5452..2d27daa40424 100644 --- a/rasa/core/policies/memoization.py +++ b/rasa/core/policies/memoization.py @@ -152,7 +152,7 @@ def _create_feature_key(self, states: List[State]) -> Text: # represented as dictionaries have the same json strings # quotes are removed for aesthetic reasons feature_str = json.dumps(states, sort_keys=True).replace('"', "") - if self.ENABLE_FEATURE_STRING_COMPRESSION: + if self.config["enable_feature_string_compression"]: compressed = zlib.compress( bytes(feature_str, rasa.shared.utils.io.DEFAULT_ENCODING) ) @@ -166,7 +166,6 @@ def train( self, training_trackers: List[TrackerWithCachedStates], domain: Domain, - interpreter: NaturalLanguageInterpreter, **kwargs: Any, ) -> None: # only considers original trackers (no augmented ones) @@ -207,7 +206,7 @@ def _prediction_result( ) -> List[float]: result = self._default_predictions(domain) if action_name: - if self.USE_NLU_CONFIDENCE_AS_SCORE: + if self.config["use_nlu_confidence_as_score"]: # the memoization will use the confidence of NLU on the latest # user message to set the confidence of the action score = tracker.latest_message.intent.get("confidence", 1.0) @@ -264,7 +263,7 @@ def load( resource: Resource, execution_context: ExecutionContext, **kwargs: Any, - ) -> "PolicyGraphComponent": + ) -> "MemoizationPolicyGraphComponent": """Loads a trained policy (see parent class for full docstring).""" featurizer = None lookup = None diff --git a/rasa/core/policies/policy.py b/rasa/core/policies/policy.py index f2222851c75f..916e3e89136d 100644 --- a/rasa/core/policies/policy.py +++ b/rasa/core/policies/policy.py @@ -123,11 +123,11 @@ def __init__( featurizer: Optional[TrackerFeaturizer] = None, ) -> None: """Constructs a new Policy object.""" + self.config = config if featurizer is None: featurizer = self._create_featurizer(config) self.__featurizer = featurizer - self.config = config self.priority = config.get(POLICY_PRIORITY, DEFAULT_POLICY_PRIORITY) self.finetune_mode = execution_context.is_finetuning @@ -148,18 +148,17 @@ def create( """Creates a new untrained policy (see parent class for full docstring).""" return cls(config, model_storage, resource, execution_context) - @classmethod - def _create_featurizer(cls, policy_config: Dict[Text, Any]) -> TrackerFeaturizer: + def _create_featurizer(self, policy_config: Dict[Text, Any]) -> TrackerFeaturizer: policy_config = copy.deepcopy(policy_config) featurizer_configs = policy_config.get("featurizer") if not featurizer_configs: - return cls._standard_featurizer() + return self._standard_featurizer() featurizer_func = _get_featurizer_from_config( featurizer_configs, - cls.__name__, + type(self).__name__, lookup_path="rasa.core.featurizers.tracker_featurizers", ) featurizer_config = featurizer_configs[0] @@ -168,7 +167,7 @@ def _create_featurizer(cls, policy_config: Dict[Text, Any]) -> TrackerFeaturizer if state_featurizer_configs: state_featurizer_func = _get_featurizer_from_config( state_featurizer_configs, - cls.__name__, + type(self).__name__, lookup_path="rasa.core.featurizers.single_state_featurizer", ) state_featurizer_config = state_featurizer_configs[0] @@ -179,8 +178,7 @@ def _create_featurizer(cls, policy_config: Dict[Text, Any]) -> TrackerFeaturizer return featurizer_func(**featurizer_config) - @staticmethod - def _standard_featurizer() -> MaxHistoryTrackerFeaturizer: + def _standard_featurizer(self) -> MaxHistoryTrackerFeaturizer: return MaxHistoryTrackerFeaturizer(SingleStateFeaturizer()) @property diff --git a/rasa/core/policies/ted_policy.py b/rasa/core/policies/ted_policy.py index c9b61c157161..94994d8bc72d 100644 --- a/rasa/core/policies/ted_policy.py +++ b/rasa/core/policies/ted_policy.py @@ -41,7 +41,7 @@ ) from rasa.shared.nlu.interpreter import NaturalLanguageInterpreter, RegexInterpreter from rasa.core.policies.policy import PolicyPrediction, PolicyGraphComponent -from rasa.core.constants import DIALOGUE, POLICY_MAX_HISTORY +from rasa.core.constants import DIALOGUE, POLICY_MAX_HISTORY, DEFAULT_MAX_HISTORY from rasa.shared.constants import DIAGNOSTIC_DATA from rasa.shared.core.constants import ACTIVE_LOOP, SLOTS, ACTION_LISTEN_NAME from rasa.shared.core.trackers import DialogueStateTracker @@ -332,12 +332,13 @@ def get_default_config() -> Dict[Text, Any]: # ingredients in a recipe, but it doesn't make sense for the parts of # an address SPLIT_ENTITIES_BY_COMMA: SPLIT_ENTITIES_BY_COMMA_DEFAULT_VALUE, + # Max history of the policy, unbounded by default + POLICY_MAX_HISTORY: DEFAULT_MAX_HISTORY, } - @staticmethod - def _standard_featurizer(max_history: Optional[int] = None) -> TrackerFeaturizer: + def _standard_featurizer(self) -> TrackerFeaturizer: return MaxHistoryTrackerFeaturizer( - SingleStateFeaturizer(), max_history=max_history + SingleStateFeaturizer(), max_history=self.config[POLICY_MAX_HISTORY] ) def __init__( diff --git a/tests/core/policies/test_ted_policy.py b/tests/core/policies/test_ted_policy.py index e3bad2b79b1c..58020d7694c5 100644 --- a/tests/core/policies/test_ted_policy.py +++ b/tests/core/policies/test_ted_policy.py @@ -6,7 +6,11 @@ import tests.core.test_policies from _pytest.monkeypatch import MonkeyPatch from _pytest.logging import LogCaptureFixture -from rasa.core.constants import POLICY_PRIORITY, POLICY_MAX_HISTORY +from rasa.core.constants import ( + POLICY_PRIORITY, + POLICY_MAX_HISTORY, + DEFAULT_POLICY_PRIORITY, +) from rasa.core.featurizers.single_state_featurizer import SingleStateFeaturizer from rasa.core.featurizers.tracker_featurizers import ( @@ -571,6 +575,48 @@ def test_ignore_action_unlikely_intent( == prediction_without_action.probabilities ) + @pytest.mark.parametrize( + "featurizer_config, tracker_featurizer, state_featurizer", + [ + (None, MaxHistoryTrackerFeaturizer(), SingleStateFeaturizer), + ([], MaxHistoryTrackerFeaturizer(), SingleStateFeaturizer), + ], + ) + def test_empty_featurizer_configs( + self, + featurizer_config: Optional[Dict[Text, Any]], + model_storage: ModelStorage, + resource: Resource, + execution_context: ExecutionContext, + tracker_featurizer: MaxHistoryTrackerFeaturizer, + state_featurizer: Type[SingleStateFeaturizer], + ): + featurizer_config_override = ( + {"featurizer": featurizer_config} if featurizer_config else {} + ) + policy = self.create_policy( + None, + priority=DEFAULT_POLICY_PRIORITY, + model_storage=model_storage, + resource=resource, + execution_context=execution_context, + config=self._config(DEFAULT_POLICY_PRIORITY, featurizer_config_override), + ) + + featurizer = policy.featurizer + assert isinstance(featurizer, tracker_featurizer.__class__) + + if featurizer_config: + expected_max_history = featurizer_config[0].get(POLICY_MAX_HISTORY) + else: + expected_max_history = self._config(DEFAULT_POLICY_PRIORITY).get( + POLICY_MAX_HISTORY + ) + + assert featurizer.max_history == expected_max_history + + assert isinstance(featurizer.state_featurizer, state_featurizer) + class TestTEDPolicyMargin(TestTEDPolicy): def _config( diff --git a/tests/core/test_policies.py b/tests/core/test_policies.py index b19b52fa4cff..809721c5ca27 100644 --- a/tests/core/test_policies.py +++ b/tests/core/test_policies.py @@ -1,12 +1,16 @@ import uuid from pathlib import Path -from typing import Type, List, Text, Optional, Dict, Any +from typing import Type, List, Text, Optional, Dict, Any, Union import dataclasses import numpy as np import pytest from _pytest.tmpdir import TempPathFactory -from rasa.core.constants import DEFAULT_POLICY_PRIORITY, POLICY_MAX_HISTORY +from rasa.core.constants import ( + DEFAULT_POLICY_PRIORITY, + POLICY_MAX_HISTORY, + POLICY_PRIORITY, +) from rasa.engine.graph import ExecutionContext, GraphSchema, GraphComponent from rasa.engine.storage.local_model_storage import LocalModelStorage from rasa.engine.storage.resource import Resource @@ -38,10 +42,19 @@ IntentMaxHistoryTrackerFeaturizer, ) from rasa.shared.nlu.interpreter import RegexInterpreter -from rasa.core.policies.policy import SupportedData, Policy, InvalidPolicyConfig +from rasa.core.policies.policy import ( + SupportedData, + Policy, + InvalidPolicyConfig, + PolicyGraphComponent, +) from rasa.core.policies.rule_policy import RulePolicy from rasa.core.policies.ted_policy import TEDPolicy -from rasa.core.policies.memoization import AugmentedMemoizationPolicy, MemoizationPolicy +from rasa.core.policies.memoization import ( + AugmentedMemoizationPolicyGraphComponent, + MemoizationPolicyGraphComponent, +) + from rasa.shared.core.trackers import DialogueStateTracker from tests.dialogues import TEST_DEFAULT_DIALOGUE from tests.core.utilities import get_tracker, tracker_from_dialogue @@ -55,6 +68,9 @@ def train_trackers( ) +EitherPolicy = Union[Policy, PolicyGraphComponent] + + # We are going to use class style testing here since unfortunately pytest # doesn't support using fixtures as arguments to its own parameterize yet # (hence, we can't train a policy, declare it as a fixture and use the @@ -94,7 +110,7 @@ def create_policy( resource: Resource, execution_context: ExecutionContext, config: Optional[Dict[Text, Any]] = None, - ) -> Policy: + ) -> EitherPolicy: raise NotImplementedError @pytest.fixture(scope="class") @@ -126,19 +142,23 @@ def trained_policy( model_storage: ModelStorage, resource: Resource, execution_context: ExecutionContext, - ) -> Policy: + ) -> EitherPolicy: policy = self.create_policy( featurizer, priority, model_storage, resource, execution_context ) training_trackers = train_trackers( default_domain, stories_path, augmentation_factor=20 ) - policy.train(training_trackers, default_domain, RegexInterpreter()) + if isinstance(policy, GraphComponent): + policy.train(training_trackers, default_domain) + else: + # TODO: Drop after all policies are migrated to `GraphComponent` + policy.train(training_trackers, default_domain, RegexInterpreter()) return policy def test_featurizer( self, - trained_policy: Policy, + trained_policy: EitherPolicy, resource: Resource, model_storage: ModelStorage, tmp_path: Path, @@ -169,7 +189,7 @@ def test_featurizer( @pytest.mark.parametrize("should_finetune", [False, True]) def test_persist_and_load( self, - trained_policy: Policy, + trained_policy: EitherPolicy, default_domain: Domain, tmp_path: Path, should_finetune: bool, @@ -228,15 +248,15 @@ def test_persist_and_load_empty_policy( ): resource = Resource(uuid.uuid4().hex) empty_policy = self.create_policy( - DEFAULT_POLICY_PRIORITY, None, + DEFAULT_POLICY_PRIORITY, default_model_storage, resource, execution_context, ) - empty_policy.train([], default_domain, RegexInterpreter()) - if isinstance(empty_policy, GraphComponent): + if isinstance(empty_policy, PolicyGraphComponent): + empty_policy.train([], default_domain) loaded = empty_policy.__class__.load( self._config(DEFAULT_POLICY_PRIORITY), default_model_storage, @@ -245,13 +265,16 @@ def test_persist_and_load_empty_policy( ) else: # TODO: Drop after all policies are migrated to `GraphComponent` + empty_policy.train([], default_domain, RegexInterpreter()) empty_policy.persist(str(tmp_path)) loaded = empty_policy.__class__.load(str(tmp_path)) assert loaded is not None @staticmethod - def _get_next_action(policy: Policy, events: List[Event], domain: Domain) -> Text: + def _get_next_action( + policy: EitherPolicy, events: List[Event], domain: Domain + ) -> Text: tracker = get_tracker(events) scores = policy.predict_action_probabilities( @@ -260,16 +283,17 @@ def _get_next_action(policy: Policy, events: List[Event], domain: Domain) -> Tex index = scores.index(max(scores)) return domain.action_names_or_texts[index] + # TODO: make these two commented out parametrizations testable in subclasses @pytest.mark.parametrize( "featurizer_config, tracker_featurizer, state_featurizer", [ - (None, MaxHistoryTrackerFeaturizer(), SingleStateFeaturizer), - ([], MaxHistoryTrackerFeaturizer(), SingleStateFeaturizer), + # (None, MaxHistoryTrackerFeaturizer(), SingleStateFeaturizer), + # ([], MaxHistoryTrackerFeaturizer(), SingleStateFeaturizer), ( [ { "name": "MaxHistoryTrackerFeaturizer", - "max_history": 12, + POLICY_MAX_HISTORY: 12, "state_featurizer": [], } ], @@ -277,7 +301,7 @@ def _get_next_action(policy: Policy, events: List[Event], domain: Domain) -> Tex type(None), ), ( - [{"name": "MaxHistoryTrackerFeaturizer", "max_history": 12}], + [{"name": "MaxHistoryTrackerFeaturizer", POLICY_MAX_HISTORY: 12}], MaxHistoryTrackerFeaturizer(max_history=12), type(None), ), @@ -285,7 +309,7 @@ def _get_next_action(policy: Policy, events: List[Event], domain: Domain) -> Tex [ { "name": "IntentMaxHistoryTrackerFeaturizer", - "max_history": 12, + POLICY_MAX_HISTORY: 12, "state_featurizer": [ {"name": "IntentTokenizerSingleStateFeaturizer"} ], @@ -305,13 +329,16 @@ def test_different_featurizer_configs( tracker_featurizer: MaxHistoryTrackerFeaturizer, state_featurizer: Type[SingleStateFeaturizer], ): + featurizer_config_override = ( + {"featurizer": featurizer_config} if featurizer_config else {} + ) policy = self.create_policy( None, - priority=1, + priority=DEFAULT_POLICY_PRIORITY, model_storage=model_storage, resource=resource, execution_context=execution_context, - config={"featurizer": featurizer_config}, + config=self._config(DEFAULT_POLICY_PRIORITY, featurizer_config_override), ) if not isinstance(policy, GraphComponent): @@ -321,9 +348,13 @@ def test_different_featurizer_configs( featurizer = policy.featurizer assert isinstance(featurizer, tracker_featurizer.__class__) - expected_max_history = self._config(DEFAULT_POLICY_PRIORITY).get( - POLICY_MAX_HISTORY, tracker_featurizer.max_history - ) + if featurizer_config: + expected_max_history = featurizer_config[0].get(POLICY_MAX_HISTORY) + else: + expected_max_history = self._config(DEFAULT_POLICY_PRIORITY).get( + POLICY_MAX_HISTORY + ) + assert featurizer.max_history == expected_max_history assert isinstance(featurizer.state_featurizer, state_featurizer) @@ -332,13 +363,13 @@ def test_different_featurizer_configs( "featurizer_config", [ [ - {"name": "MaxHistoryTrackerFeaturizer", "max_history": 12}, - {"name": "MaxHistoryTrackerFeaturizer", "max_history": 12}, + {"name": "MaxHistoryTrackerFeaturizer", POLICY_MAX_HISTORY: 12}, + {"name": "MaxHistoryTrackerFeaturizer", POLICY_MAX_HISTORY: 12}, ], [ { "name": "IntentMaxHistoryTrackerFeaturizer", - "max_history": 12, + POLICY_MAX_HISTORY: 12, "state_featurizer": [ {"name": "IntentTokenizerSingleStateFeaturizer"}, {"name": "IntentTokenizerSingleStateFeaturizer"}, @@ -349,7 +380,7 @@ def test_different_featurizer_configs( ) def test_different_invalid_featurizer_configs( self, - trained_policy: Policy, + trained_policy: EitherPolicy, featurizer_config: Optional[Dict[Text, Any]], model_storage: ModelStorage, resource: Resource, @@ -371,6 +402,19 @@ def test_different_invalid_featurizer_configs( class TestMemoizationPolicy(PolicyTestCollection): + @pytest.fixture(scope="class") + def featurizer(self) -> TrackerFeaturizer: + featurizer = MaxHistoryTrackerFeaturizer(None, max_history=self.max_history) + return featurizer + + def _config( + self, priority: int, config_override: Optional[Dict[Text, Any]] = None + ) -> Dict[Text, Any]: + config_override = config_override or {} + config = MemoizationPolicyGraphComponent.get_default_config() + config[POLICY_PRIORITY] = priority + return {**config, **config_override} + def create_policy( self, featurizer: Optional[TrackerFeaturizer], @@ -379,28 +423,42 @@ def create_policy( resource: Resource, execution_context: ExecutionContext, config: Optional[Dict[Text, Any]] = None, - ) -> Policy: - max_history = None - if isinstance(featurizer, MaxHistoryTrackerFeaturizer): - max_history = featurizer.max_history - return MemoizationPolicy(priority=priority, max_history=max_history) + ) -> EitherPolicy: + config = config or MemoizationPolicyGraphComponent.get_default_config() + config[POLICY_PRIORITY] = priority + return MemoizationPolicyGraphComponent( + config, model_storage, resource, execution_context, featurizer + ) - def test_featurizer(self, trained_policy: Policy, tmp_path: Path): + def test_featurizer( + self, + trained_policy: EitherPolicy, + resource: Resource, + model_storage: ModelStorage, + tmp_path: Path, + execution_context: ExecutionContext, + ): assert isinstance(trained_policy.featurizer, MaxHistoryTrackerFeaturizer) assert trained_policy.featurizer.state_featurizer is None - trained_policy.persist(str(tmp_path)) - loaded = trained_policy.__class__.load(str(tmp_path)) + config = self._config(DEFAULT_POLICY_PRIORITY) + trained_policy.persist() + loaded = type(trained_policy).load( + config, model_storage, resource, execution_context + ) assert isinstance(loaded.featurizer, MaxHistoryTrackerFeaturizer) assert loaded.featurizer.state_featurizer is None def test_memorise( self, - trained_policy: MemoizationPolicy, + trained_policy: MemoizationPolicyGraphComponent, default_domain: Domain, stories_path: Text, ): trackers = train_trackers(default_domain, stories_path, augmentation_factor=20) - trained_policy.train(trackers, default_domain, RegexInterpreter()) + if isinstance(trained_policy, PolicyGraphComponent): + trained_policy.train(trackers, default_domain) + else: + trained_policy.train(trackers, default_domain, RegexInterpreter()) lookup_with_augmentation = trained_policy.lookup trackers = [ @@ -426,15 +484,18 @@ def test_memorise( trackers_no_augmentation = train_trackers( default_domain, stories_path, augmentation_factor=0 ) - trained_policy.train( - trackers_no_augmentation, default_domain, RegexInterpreter() - ) + if isinstance(trained_policy, PolicyGraphComponent): + trained_policy.train(trackers_no_augmentation, default_domain) + else: + trained_policy.train( + trackers_no_augmentation, default_domain, RegexInterpreter() + ) lookup_no_augmentation = trained_policy.lookup assert lookup_no_augmentation == lookup_with_augmentation def test_memorise_with_nlu( - self, trained_policy: MemoizationPolicy, default_domain: Domain + self, trained_policy: MemoizationPolicyGraphComponent, default_domain: Domain ): tracker = tracker_from_dialogue(TEST_DEFAULT_DIALOGUE, default_domain) states = trained_policy._prediction_states(tracker, default_domain) @@ -444,15 +505,19 @@ def test_memorise_with_nlu( def test_finetune_after_load( self, - trained_policy: MemoizationPolicy, + trained_policy: MemoizationPolicyGraphComponent, + resource: Resource, + model_storage: ModelStorage, + execution_context: ExecutionContext, default_domain: Domain, - tmp_path: Path, stories_path: Text, ): - trained_policy.persist(tmp_path) - - loaded_policy = MemoizationPolicy.load(tmp_path, should_finetune=True) + trained_policy.persist() + execution_context = dataclasses.replace(execution_context, is_finetuning=True) + loaded_policy = MemoizationPolicyGraphComponent.load( + trained_policy.config, model_storage, resource, execution_context + ) assert loaded_policy.finetune_mode @@ -470,9 +535,7 @@ def test_finetune_after_load( original_train_data = train_trackers( default_domain, stories_path, augmentation_factor=20 ) - loaded_policy.train( - original_train_data + [new_story], default_domain, RegexInterpreter() - ) + loaded_policy.train(original_train_data + [new_story], default_domain) # Get the hash of the tracker state of new story new_story_states, _ = loaded_policy.featurizer.training_states_and_labels( @@ -517,7 +580,7 @@ def test_finetune_after_load( ) def test_ignore_action_unlikely_intent( self, - trained_policy: MemoizationPolicy, + trained_policy: MemoizationPolicyGraphComponent, default_domain: Domain, tracker_events_with_action: List[Event], tracker_events_without_action: List[Event], @@ -543,8 +606,58 @@ def test_ignore_action_unlikely_intent( == prediction_without_action.probabilities ) + @pytest.mark.parametrize( + "featurizer_config, tracker_featurizer, state_featurizer", + [ + (None, MaxHistoryTrackerFeaturizer(), type(None)), + ([], MaxHistoryTrackerFeaturizer(), type(None)), + ], + ) + def test_empty_featurizer_configs( + self, + featurizer_config: Optional[Dict[Text, Any]], + model_storage: ModelStorage, + resource: Resource, + execution_context: ExecutionContext, + tracker_featurizer: MaxHistoryTrackerFeaturizer, + state_featurizer: Type[SingleStateFeaturizer], + ): + featurizer_config_override = ( + {"featurizer": featurizer_config} if featurizer_config else {} + ) + policy = self.create_policy( + None, + priority=DEFAULT_POLICY_PRIORITY, + model_storage=model_storage, + resource=resource, + execution_context=execution_context, + config=self._config(DEFAULT_POLICY_PRIORITY, featurizer_config_override), + ) + + featurizer = policy.featurizer + assert isinstance(featurizer, tracker_featurizer.__class__) + + if featurizer_config: + expected_max_history = featurizer_config[0].get(POLICY_MAX_HISTORY) + else: + expected_max_history = self._config(DEFAULT_POLICY_PRIORITY).get( + POLICY_MAX_HISTORY + ) + + assert featurizer.max_history == expected_max_history + + assert isinstance(featurizer.state_featurizer, state_featurizer) + class TestAugmentedMemoizationPolicy(TestMemoizationPolicy): + def _config( + self, priority: int, config_override: Optional[Dict[Text, Any]] = None + ) -> Dict[Text, Any]: + config_override = config_override or {} + config = AugmentedMemoizationPolicyGraphComponent.get_default_config() + config[POLICY_PRIORITY] = priority + return {**config, **config_override} + def create_policy( self, featurizer: Optional[TrackerFeaturizer], @@ -553,11 +666,12 @@ def create_policy( resource: Resource, execution_context: ExecutionContext, config: Optional[Dict[Text, Any]] = None, - ) -> Policy: - max_history = None - if isinstance(featurizer, MaxHistoryTrackerFeaturizer): - max_history = featurizer.max_history - return AugmentedMemoizationPolicy(priority=priority, max_history=max_history) + ) -> EitherPolicy: + config = config or AugmentedMemoizationPolicyGraphComponent.get_default_config() + config[POLICY_PRIORITY] = priority + return AugmentedMemoizationPolicyGraphComponent( + config, model_storage, resource, execution_context, featurizer + ) @pytest.mark.parametrize( @@ -565,7 +679,7 @@ def create_policy( [ (TEDPolicy, SupportedData.ML_DATA), (RulePolicy, SupportedData.ML_AND_RULE_DATA), - (MemoizationPolicy, SupportedData.ML_DATA), + (MemoizationPolicyGraphComponent, SupportedData.ML_DATA), ], ) def test_supported_data(policy: Type[Policy], supported_data: SupportedData): From 52857e6c2a0fce11b0f9add34ecca9d641775490 Mon Sep 17 00:00:00 2001 From: Thomas Werkmeister Date: Mon, 23 Aug 2021 14:23:06 +0200 Subject: [PATCH 08/41] removed unused imports --- rasa/core/policies/policy.py | 1 - 1 file changed, 1 deletion(-) diff --git a/rasa/core/policies/policy.py b/rasa/core/policies/policy.py index 916e3e89136d..9c136ff17df5 100644 --- a/rasa/core/policies/policy.py +++ b/rasa/core/policies/policy.py @@ -1,7 +1,6 @@ from __future__ import annotations import abc import copy -import json import logging from enum import Enum from pathlib import Path From eadd3cb31739f7fe42951c5734c05ae9602c1b9b Mon Sep 17 00:00:00 2001 From: Thomas Werkmeister Date: Tue, 24 Aug 2021 10:26:26 +0200 Subject: [PATCH 09/41] Removed max history hack in ted policy --- rasa/core/policies/policy.py | 1 + rasa/core/policies/ted_policy.py | 13 ++++++------- tests/core/test_policies.py | 3 --- 3 files changed, 7 insertions(+), 10 deletions(-) diff --git a/rasa/core/policies/policy.py b/rasa/core/policies/policy.py index 9c136ff17df5..4f4f0cfa5fdf 100644 --- a/rasa/core/policies/policy.py +++ b/rasa/core/policies/policy.py @@ -178,6 +178,7 @@ def _create_featurizer(self, policy_config: Dict[Text, Any]) -> TrackerFeaturize return featurizer_func(**featurizer_config) def _standard_featurizer(self) -> MaxHistoryTrackerFeaturizer: + """Initializes the standard featurizer for this policy.""" return MaxHistoryTrackerFeaturizer(SingleStateFeaturizer()) @property diff --git a/rasa/core/policies/ted_policy.py b/rasa/core/policies/ted_policy.py index 94994d8bc72d..ccd4093ea2c6 100644 --- a/rasa/core/policies/ted_policy.py +++ b/rasa/core/policies/ted_policy.py @@ -361,13 +361,6 @@ def __init__( config[SPLIT_ENTITIES_BY_COMMA], SPLIT_ENTITIES_BY_COMMA_DEFAULT_VALUE ) - # TODO: check if this statement can be removed. - # More context here - - # https://github.com/RasaHQ/rasa/issues/5786#issuecomment-840762751 - max_history = config.get(POLICY_MAX_HISTORY) - if isinstance(self.featurizer, MaxHistoryTrackerFeaturizer) and max_history: - self.featurizer.max_history = max_history - self._load_params(config) self.model = model @@ -386,6 +379,12 @@ def __init__( if self.config[CHECKPOINT_MODEL]: self.tmp_checkpoint_dir = Path(rasa.utils.io.create_temporary_directory()) + def _standard_featurizer(self) -> MaxHistoryTrackerFeaturizer: + """Initializes the standard featurizer for this policy.""" + return MaxHistoryTrackerFeaturizer( + SingleStateFeaturizer(), self.config.get(POLICY_MAX_HISTORY) + ) + @staticmethod def model_class() -> Type[TED]: """Gets the class of the model architecture to be used by the policy. diff --git a/tests/core/test_policies.py b/tests/core/test_policies.py index 809721c5ca27..0589176c2eff 100644 --- a/tests/core/test_policies.py +++ b/tests/core/test_policies.py @@ -283,12 +283,9 @@ def _get_next_action( index = scores.index(max(scores)) return domain.action_names_or_texts[index] - # TODO: make these two commented out parametrizations testable in subclasses @pytest.mark.parametrize( "featurizer_config, tracker_featurizer, state_featurizer", [ - # (None, MaxHistoryTrackerFeaturizer(), SingleStateFeaturizer), - # ([], MaxHistoryTrackerFeaturizer(), SingleStateFeaturizer), ( [ { From d1013a66faf4eac9c77825942c49e6c8398df57b Mon Sep 17 00:00:00 2001 From: Thomas Werkmeister Date: Tue, 24 Aug 2021 10:55:59 +0200 Subject: [PATCH 10/41] Making sure policy config is used for initializing featurizers if necessary --- rasa/core/policies/policy.py | 18 +++++++++++++++--- 1 file changed, 15 insertions(+), 3 deletions(-) diff --git a/rasa/core/policies/policy.py b/rasa/core/policies/policy.py index 4f4f0cfa5fdf..26f8d13a505c 100644 --- a/rasa/core/policies/policy.py +++ b/rasa/core/policies/policy.py @@ -37,7 +37,11 @@ from rasa.shared.nlu.interpreter import NaturalLanguageInterpreter from rasa.shared.core.trackers import DialogueStateTracker from rasa.shared.core.generator import TrackerWithCachedStates -from rasa.core.constants import DEFAULT_POLICY_PRIORITY, POLICY_PRIORITY +from rasa.core.constants import ( + DEFAULT_POLICY_PRIORITY, + POLICY_PRIORITY, + POLICY_MAX_HISTORY, +) from rasa.shared.core.constants import ( USER, SLOTS, @@ -155,9 +159,17 @@ def _create_featurizer(self, policy_config: Dict[Text, Any]) -> TrackerFeaturize if not featurizer_configs: return self._standard_featurizer() + if ( + POLICY_MAX_HISTORY not in featurizer_configs[0] + and POLICY_MAX_HISTORY in policy_config + ): + featurizer_configs[0][POLICY_MAX_HISTORY] = policy_config[ + POLICY_MAX_HISTORY + ] + featurizer_func = _get_featurizer_from_config( featurizer_configs, - type(self).__name__, + self.__class__.__name__, lookup_path="rasa.core.featurizers.tracker_featurizers", ) featurizer_config = featurizer_configs[0] @@ -166,7 +178,7 @@ def _create_featurizer(self, policy_config: Dict[Text, Any]) -> TrackerFeaturize if state_featurizer_configs: state_featurizer_func = _get_featurizer_from_config( state_featurizer_configs, - type(self).__name__, + self.__class__.__name__, lookup_path="rasa.core.featurizers.single_state_featurizer", ) state_featurizer_config = state_featurizer_configs[0] From 75b06785b3b1b2e752d3a81ef129ae6c2cc9a99f Mon Sep 17 00:00:00 2001 From: Thomas Werkmeister Date: Tue, 24 Aug 2021 11:00:15 +0200 Subject: [PATCH 11/41] Removed duplicate method --- rasa/core/policies/ted_policy.py | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/rasa/core/policies/ted_policy.py b/rasa/core/policies/ted_policy.py index ccd4093ea2c6..3edb5154efcf 100644 --- a/rasa/core/policies/ted_policy.py +++ b/rasa/core/policies/ted_policy.py @@ -338,7 +338,7 @@ def get_default_config() -> Dict[Text, Any]: def _standard_featurizer(self) -> TrackerFeaturizer: return MaxHistoryTrackerFeaturizer( - SingleStateFeaturizer(), max_history=self.config[POLICY_MAX_HISTORY] + SingleStateFeaturizer(), max_history=self.config.get(POLICY_MAX_HISTORY) ) def __init__( @@ -379,12 +379,6 @@ def __init__( if self.config[CHECKPOINT_MODEL]: self.tmp_checkpoint_dir = Path(rasa.utils.io.create_temporary_directory()) - def _standard_featurizer(self) -> MaxHistoryTrackerFeaturizer: - """Initializes the standard featurizer for this policy.""" - return MaxHistoryTrackerFeaturizer( - SingleStateFeaturizer(), self.config.get(POLICY_MAX_HISTORY) - ) - @staticmethod def model_class() -> Type[TED]: """Gets the class of the model architecture to be used by the policy. From be45a0177bf983291b458cc345a7f5cabc1173b6 Mon Sep 17 00:00:00 2001 From: Thomas Werkmeister Date: Tue, 24 Aug 2021 13:05:03 +0200 Subject: [PATCH 12/41] Added policy priority to ted policy default config --- rasa/core/policies/ted_policy.py | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/rasa/core/policies/ted_policy.py b/rasa/core/policies/ted_policy.py index 3edb5154efcf..a1f4958523b2 100644 --- a/rasa/core/policies/ted_policy.py +++ b/rasa/core/policies/ted_policy.py @@ -41,7 +41,13 @@ ) from rasa.shared.nlu.interpreter import NaturalLanguageInterpreter, RegexInterpreter from rasa.core.policies.policy import PolicyPrediction, PolicyGraphComponent -from rasa.core.constants import DIALOGUE, POLICY_MAX_HISTORY, DEFAULT_MAX_HISTORY +from rasa.core.constants import ( + DIALOGUE, + POLICY_MAX_HISTORY, + DEFAULT_MAX_HISTORY, + DEFAULT_POLICY_PRIORITY, + POLICY_PRIORITY, +) from rasa.shared.constants import DIAGNOSTIC_DATA from rasa.shared.core.constants import ACTIVE_LOOP, SLOTS, ACTION_LISTEN_NAME from rasa.shared.core.trackers import DialogueStateTracker @@ -334,6 +340,8 @@ def get_default_config() -> Dict[Text, Any]: SPLIT_ENTITIES_BY_COMMA: SPLIT_ENTITIES_BY_COMMA_DEFAULT_VALUE, # Max history of the policy, unbounded by default POLICY_MAX_HISTORY: DEFAULT_MAX_HISTORY, + # Determines the importance of policies, higher values take precedence + POLICY_PRIORITY: DEFAULT_POLICY_PRIORITY, } def _standard_featurizer(self) -> TrackerFeaturizer: From 716238af10e0da039898cf4f0c9afd0144b949de Mon Sep 17 00:00:00 2001 From: Thomas Werkmeister Date: Tue, 24 Aug 2021 13:07:06 +0200 Subject: [PATCH 13/41] Removed unecessary type check --- tests/core/test_policies.py | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/tests/core/test_policies.py b/tests/core/test_policies.py index 0589176c2eff..1f0115723a4c 100644 --- a/tests/core/test_policies.py +++ b/tests/core/test_policies.py @@ -452,10 +452,7 @@ def test_memorise( stories_path: Text, ): trackers = train_trackers(default_domain, stories_path, augmentation_factor=20) - if isinstance(trained_policy, PolicyGraphComponent): - trained_policy.train(trackers, default_domain) - else: - trained_policy.train(trackers, default_domain, RegexInterpreter()) + trained_policy.train(trackers, default_domain) lookup_with_augmentation = trained_policy.lookup trackers = [ From df0fd0c6dbf2345de2f599d24a220fcb3f8693f2 Mon Sep 17 00:00:00 2001 From: Thomas Werkmeister Date: Tue, 24 Aug 2021 13:22:44 +0200 Subject: [PATCH 14/41] Removed kwargs overwrite of policy config --- rasa/core/policies/memoization.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/rasa/core/policies/memoization.py b/rasa/core/policies/memoization.py index 2d27daa40424..c70deec017e9 100644 --- a/rasa/core/policies/memoization.py +++ b/rasa/core/policies/memoization.py @@ -277,8 +277,6 @@ def load( if (Path(path) / FEATURIZER_FILE).is_file(): featurizer = TrackerFeaturizer.load(path) - config.update(kwargs) - except (ValueError, FileNotFoundError, FileIOException): logger.info( f"Couldn't load metadata for policy '{cls.__name__}' as the persisted " From 1eb211d2a94ef0f30946c56d39948eb32f37ed8c Mon Sep 17 00:00:00 2001 From: Thomas Werkmeister Date: Tue, 24 Aug 2021 13:25:26 +0200 Subject: [PATCH 15/41] Made policy config a privat var --- rasa/core/policies/memoization.py | 6 +++--- rasa/core/policies/policy.py | 2 +- tests/core/policies/test_unexpected_intent_policy.py | 4 ++-- tests/core/test_policies.py | 2 +- tests/test_model_training.py | 4 ++-- 5 files changed, 9 insertions(+), 9 deletions(-) diff --git a/rasa/core/policies/memoization.py b/rasa/core/policies/memoization.py index c70deec017e9..4d1597860022 100644 --- a/rasa/core/policies/memoization.py +++ b/rasa/core/policies/memoization.py @@ -80,7 +80,7 @@ def _standard_featurizer(self) -> MaxHistoryTrackerFeaturizer: # Memoization policy always uses MaxHistoryTrackerFeaturizer # without state_featurizer return MaxHistoryTrackerFeaturizer( - state_featurizer=None, max_history=self.config[POLICY_MAX_HISTORY] + state_featurizer=None, max_history=self._config[POLICY_MAX_HISTORY] ) def __init__( @@ -152,7 +152,7 @@ def _create_feature_key(self, states: List[State]) -> Text: # represented as dictionaries have the same json strings # quotes are removed for aesthetic reasons feature_str = json.dumps(states, sort_keys=True).replace('"', "") - if self.config["enable_feature_string_compression"]: + if self._config["enable_feature_string_compression"]: compressed = zlib.compress( bytes(feature_str, rasa.shared.utils.io.DEFAULT_ENCODING) ) @@ -206,7 +206,7 @@ def _prediction_result( ) -> List[float]: result = self._default_predictions(domain) if action_name: - if self.config["use_nlu_confidence_as_score"]: + if self._config["use_nlu_confidence_as_score"]: # the memoization will use the confidence of NLU on the latest # user message to set the confidence of the action score = tracker.latest_message.intent.get("confidence", 1.0) diff --git a/rasa/core/policies/policy.py b/rasa/core/policies/policy.py index 26f8d13a505c..9f91009b2741 100644 --- a/rasa/core/policies/policy.py +++ b/rasa/core/policies/policy.py @@ -126,7 +126,7 @@ def __init__( featurizer: Optional[TrackerFeaturizer] = None, ) -> None: """Constructs a new Policy object.""" - self.config = config + self._config = config if featurizer is None: featurizer = self._create_featurizer(config) self.__featurizer = featurizer diff --git a/tests/core/policies/test_unexpected_intent_policy.py b/tests/core/policies/test_unexpected_intent_policy.py index ef080e0d234d..d1016bf71bed 100644 --- a/tests/core/policies/test_unexpected_intent_policy.py +++ b/tests/core/policies/test_unexpected_intent_policy.py @@ -453,7 +453,7 @@ def test_should_check_for_intent( default_domain.intents[intent_index], default_domain ) - loaded_policy.config[IGNORE_INTENTS_LIST] = [ + loaded_policy._config[IGNORE_INTENTS_LIST] = [ default_domain.intents[intent_index] ] assert ( @@ -790,7 +790,7 @@ def test_individual_label_metadata( # Monkey-patch certain attributes of the policy to make the testing easier. label_thresholds = {0: 1.2, 1: -0.3, 4: -2.3, 5: 0.2} loaded_policy.label_thresholds = label_thresholds - loaded_policy.config[RANKING_LENGTH] = ranking_length + loaded_policy._config[RANKING_LENGTH] = ranking_length # Some dummy similarities similarities = np.array([[3.2, 0.2, -1.2, -4.3, -5.1, 2.3]]) diff --git a/tests/core/test_policies.py b/tests/core/test_policies.py index 1f0115723a4c..5d4013f6dec0 100644 --- a/tests/core/test_policies.py +++ b/tests/core/test_policies.py @@ -510,7 +510,7 @@ def test_finetune_after_load( trained_policy.persist() execution_context = dataclasses.replace(execution_context, is_finetuning=True) loaded_policy = MemoizationPolicyGraphComponent.load( - trained_policy.config, model_storage, resource, execution_context + trained_policy._config, model_storage, resource, execution_context ) assert loaded_policy.finetune_mode diff --git a/tests/test_model_training.py b/tests/test_model_training.py index 03318d501c36..7b67b3785401 100644 --- a/tests/test_model_training.py +++ b/tests/test_model_training.py @@ -695,7 +695,7 @@ def test_model_finetuning_core( assert isinstance(model_to_finetune, Agent) ted = model_to_finetune.policy_ensemble.policies[0] - assert ted.config[EPOCHS] == 2 + assert ted._config[EPOCHS] == 2 assert ted.finetune_mode @@ -727,7 +727,7 @@ def test_model_finetuning_core_with_default_epochs( model_to_finetune = kwargs["model_to_finetune"] ted = model_to_finetune.policy_ensemble.policies[0] - assert ted.config[EPOCHS] == TEDPolicy.defaults[EPOCHS] * 2 + assert ted._config[EPOCHS] == TEDPolicy.defaults[EPOCHS] * 2 def test_model_finetuning_core_new_domain_label( From 9be669a67724aa5151f31480e41a4b8f60b01821 Mon Sep 17 00:00:00 2001 From: Thomas Werkmeister Date: Tue, 24 Aug 2021 16:01:48 +0200 Subject: [PATCH 16/41] Revert "Made policy config a privat var" This reverts commit 1eb211d2a94ef0f30946c56d39948eb32f37ed8c. --- rasa/core/policies/memoization.py | 6 +++--- rasa/core/policies/policy.py | 2 +- tests/core/policies/test_unexpected_intent_policy.py | 4 ++-- tests/core/test_policies.py | 2 +- tests/test_model_training.py | 4 ++-- 5 files changed, 9 insertions(+), 9 deletions(-) diff --git a/rasa/core/policies/memoization.py b/rasa/core/policies/memoization.py index 4d1597860022..c70deec017e9 100644 --- a/rasa/core/policies/memoization.py +++ b/rasa/core/policies/memoization.py @@ -80,7 +80,7 @@ def _standard_featurizer(self) -> MaxHistoryTrackerFeaturizer: # Memoization policy always uses MaxHistoryTrackerFeaturizer # without state_featurizer return MaxHistoryTrackerFeaturizer( - state_featurizer=None, max_history=self._config[POLICY_MAX_HISTORY] + state_featurizer=None, max_history=self.config[POLICY_MAX_HISTORY] ) def __init__( @@ -152,7 +152,7 @@ def _create_feature_key(self, states: List[State]) -> Text: # represented as dictionaries have the same json strings # quotes are removed for aesthetic reasons feature_str = json.dumps(states, sort_keys=True).replace('"', "") - if self._config["enable_feature_string_compression"]: + if self.config["enable_feature_string_compression"]: compressed = zlib.compress( bytes(feature_str, rasa.shared.utils.io.DEFAULT_ENCODING) ) @@ -206,7 +206,7 @@ def _prediction_result( ) -> List[float]: result = self._default_predictions(domain) if action_name: - if self._config["use_nlu_confidence_as_score"]: + if self.config["use_nlu_confidence_as_score"]: # the memoization will use the confidence of NLU on the latest # user message to set the confidence of the action score = tracker.latest_message.intent.get("confidence", 1.0) diff --git a/rasa/core/policies/policy.py b/rasa/core/policies/policy.py index 9f91009b2741..26f8d13a505c 100644 --- a/rasa/core/policies/policy.py +++ b/rasa/core/policies/policy.py @@ -126,7 +126,7 @@ def __init__( featurizer: Optional[TrackerFeaturizer] = None, ) -> None: """Constructs a new Policy object.""" - self._config = config + self.config = config if featurizer is None: featurizer = self._create_featurizer(config) self.__featurizer = featurizer diff --git a/tests/core/policies/test_unexpected_intent_policy.py b/tests/core/policies/test_unexpected_intent_policy.py index d1016bf71bed..ef080e0d234d 100644 --- a/tests/core/policies/test_unexpected_intent_policy.py +++ b/tests/core/policies/test_unexpected_intent_policy.py @@ -453,7 +453,7 @@ def test_should_check_for_intent( default_domain.intents[intent_index], default_domain ) - loaded_policy._config[IGNORE_INTENTS_LIST] = [ + loaded_policy.config[IGNORE_INTENTS_LIST] = [ default_domain.intents[intent_index] ] assert ( @@ -790,7 +790,7 @@ def test_individual_label_metadata( # Monkey-patch certain attributes of the policy to make the testing easier. label_thresholds = {0: 1.2, 1: -0.3, 4: -2.3, 5: 0.2} loaded_policy.label_thresholds = label_thresholds - loaded_policy._config[RANKING_LENGTH] = ranking_length + loaded_policy.config[RANKING_LENGTH] = ranking_length # Some dummy similarities similarities = np.array([[3.2, 0.2, -1.2, -4.3, -5.1, 2.3]]) diff --git a/tests/core/test_policies.py b/tests/core/test_policies.py index 5d4013f6dec0..1f0115723a4c 100644 --- a/tests/core/test_policies.py +++ b/tests/core/test_policies.py @@ -510,7 +510,7 @@ def test_finetune_after_load( trained_policy.persist() execution_context = dataclasses.replace(execution_context, is_finetuning=True) loaded_policy = MemoizationPolicyGraphComponent.load( - trained_policy._config, model_storage, resource, execution_context + trained_policy.config, model_storage, resource, execution_context ) assert loaded_policy.finetune_mode diff --git a/tests/test_model_training.py b/tests/test_model_training.py index 7b67b3785401..03318d501c36 100644 --- a/tests/test_model_training.py +++ b/tests/test_model_training.py @@ -695,7 +695,7 @@ def test_model_finetuning_core( assert isinstance(model_to_finetune, Agent) ted = model_to_finetune.policy_ensemble.policies[0] - assert ted._config[EPOCHS] == 2 + assert ted.config[EPOCHS] == 2 assert ted.finetune_mode @@ -727,7 +727,7 @@ def test_model_finetuning_core_with_default_epochs( model_to_finetune = kwargs["model_to_finetune"] ted = model_to_finetune.policy_ensemble.policies[0] - assert ted._config[EPOCHS] == TEDPolicy.defaults[EPOCHS] * 2 + assert ted.config[EPOCHS] == TEDPolicy.defaults[EPOCHS] * 2 def test_model_finetuning_core_new_domain_label( From 4491703cc514fdbb98d2ff10710d9728dc8f4a47 Mon Sep 17 00:00:00 2001 From: Thomas Werkmeister Date: Tue, 24 Aug 2021 16:31:11 +0200 Subject: [PATCH 17/41] Simplified policy test suites --- tests/core/policies/test_ted_policy.py | 236 ++----------------------- tests/core/test_policies.py | 191 +++++++------------- 2 files changed, 76 insertions(+), 351 deletions(-) diff --git a/tests/core/policies/test_ted_policy.py b/tests/core/policies/test_ted_policy.py index 58020d7694c5..b7f663ab5801 100644 --- a/tests/core/policies/test_ted_policy.py +++ b/tests/core/policies/test_ted_policy.py @@ -124,33 +124,6 @@ class TestTEDPolicy(PolicyTestCollection): def _policy_class_to_test() -> Type[TEDPolicy]: return TEDPolicy - def _config( - self, priority: int, config_override: Optional[Dict[Text, Any]] = None - ) -> Dict[Text, Any]: - config_override = config_override or {} - return { - **TEDPolicy.get_default_config(), - POLICY_PRIORITY: priority, - **config_override, - } - - def create_policy( - self, - featurizer: Optional[TrackerFeaturizer], - priority: int, - model_storage: ModelStorage, - resource: Resource, - execution_context: ExecutionContext, - config: Optional[Dict[Text, Any]] = None, - ) -> TEDPolicy: - return TEDPolicy( - self._config(priority, config), - featurizer=featurizer, - model_storage=model_storage, - resource=resource, - execution_context=execution_context, - ) - def test_train_model_checkpointing(self, tmp_path: Path): checkpoint_dir = get_checkpoint_dir_path(tmp_path) assert not checkpoint_dir.is_dir() @@ -224,7 +197,7 @@ def test_epoch_override_when_loaded( ): execution_context.is_finetuning = should_finetune loaded_policy = trained_policy.__class__.load( - {**self._config(trained_policy.priority), EPOCH_OVERRIDE: epoch_override}, + {**self._config(), EPOCH_OVERRIDE: epoch_override}, model_storage, resource, execution_context, @@ -275,7 +248,6 @@ def test_training_with_no_intent( ) policy = self.create_policy( featurizer=featurizer, - priority=priority, model_storage=model_storage, resource=resource, execution_context=execution_context, @@ -596,11 +568,10 @@ def test_empty_featurizer_configs( ) policy = self.create_policy( None, - priority=DEFAULT_POLICY_PRIORITY, model_storage=model_storage, resource=resource, execution_context=execution_context, - config=self._config(DEFAULT_POLICY_PRIORITY, featurizer_config_override), + config=self._config(featurizer_config_override), ) featurizer = policy.featurizer @@ -609,9 +580,7 @@ def test_empty_featurizer_configs( if featurizer_config: expected_max_history = featurizer_config[0].get(POLICY_MAX_HISTORY) else: - expected_max_history = self._config(DEFAULT_POLICY_PRIORITY).get( - POLICY_MAX_HISTORY - ) + expected_max_history = self._config().get(POLICY_MAX_HISTORY) assert featurizer.max_history == expected_max_history @@ -620,33 +589,15 @@ def test_empty_featurizer_configs( class TestTEDPolicyMargin(TestTEDPolicy): def _config( - self, priority: int, config_override: Optional[Dict[Text, Any]] = None + self, config_override: Optional[Dict[Text, Any]] = None ) -> Dict[Text, Any]: config_override = config_override or {} return { **TEDPolicy.get_default_config(), - POLICY_PRIORITY: priority, LOSS_TYPE: "margin", **config_override, } - def create_policy( - self, - featurizer: Optional[TrackerFeaturizer], - priority: int, - model_storage: ModelStorage, - resource: Resource, - execution_context: ExecutionContext, - config: Optional[Dict[Text, Any]] = None, - ) -> Policy: - return TEDPolicy( - self._config(priority, config), - featurizer=featurizer, - model_storage=model_storage, - resource=resource, - execution_context=execution_context, - ) - def test_similarity_type(self, trained_policy: TEDPolicy): assert trained_policy.config[SIMILARITY_TYPE] == COSINE @@ -685,64 +636,28 @@ def test_prediction_on_empty_tracker( class TestTEDPolicyWithEval(TestTEDPolicy): def _config( - self, priority: int, config_override: Optional[Dict[Text, Any]] = None + self, config_override: Optional[Dict[Text, Any]] = None ) -> Dict[Text, Any]: config_override = config_override or {} return { **TEDPolicy.get_default_config(), - POLICY_PRIORITY: priority, SCALE_LOSS: False, EVAL_NUM_EXAMPLES: 4, **config_override, } - def create_policy( - self, - featurizer: Optional[TrackerFeaturizer], - priority: int, - model_storage: ModelStorage, - resource: Resource, - execution_context: ExecutionContext, - config: Optional[Dict[Text, Any]] = None, - ) -> Policy: - return TEDPolicy( - featurizer=featurizer, - config=self._config(priority, config), - model_storage=model_storage, - resource=resource, - execution_context=execution_context, - ) - class TestTEDPolicyNoNormalization(TestTEDPolicy): def _config( - self, priority: int, config_override: Optional[Dict[Text, Any]] = None + self, config_override: Optional[Dict[Text, Any]] = None ) -> Dict[Text, Any]: config_override = config_override or {} return { **TEDPolicy.get_default_config(), RANKING_LENGTH: 0, - POLICY_PRIORITY: priority, **config_override, } - def create_policy( - self, - featurizer: Optional[TrackerFeaturizer], - priority: int, - model_storage: ModelStorage, - resource: Resource, - execution_context: ExecutionContext, - config: Optional[Dict[Text, Any]] = None, - ) -> Policy: - return TEDPolicy( - featurizer=featurizer, - config=self._config(priority, config), - model_storage=model_storage, - resource=resource, - execution_context=execution_context, - ) - def test_ranking_length(self, trained_policy: TEDPolicy): assert trained_policy.config[RANKING_LENGTH] == 0 @@ -772,33 +687,15 @@ def test_normalization( class TestTEDPolicyLinearNormConfidence(TestTEDPolicy): def _config( - self, priority: int, config_override: Optional[Dict[Text, Any]] = None + self, config_override: Optional[Dict[Text, Any]] = None ) -> Dict[Text, Any]: config_override = config_override or {} return { **TEDPolicy.get_default_config(), - POLICY_PRIORITY: priority, MODEL_CONFIDENCE: LINEAR_NORM, **config_override, } - def create_policy( - self, - featurizer: Optional[TrackerFeaturizer], - priority: int, - model_storage: ModelStorage, - resource: Resource, - execution_context: ExecutionContext, - config: Optional[Dict[Text, Any]] = None, - ) -> Policy: - return TEDPolicy( - featurizer=featurizer, - config=self._config(priority, config), - model_storage=model_storage, - resource=resource, - execution_context=execution_context, - ) - def test_confidence_type(self, trained_policy: TEDPolicy): assert trained_policy.config[MODEL_CONFIDENCE] == LINEAR_NORM @@ -841,85 +738,47 @@ def test_prediction_on_empty_tracker( class TestTEDPolicyLowRankingLength(TestTEDPolicy): def _config( - self, priority: int, config_override: Optional[Dict[Text, Any]] = None + self, config_override: Optional[Dict[Text, Any]] = None ) -> Dict[Text, Any]: config_override = config_override or {} return { **TEDPolicy.get_default_config(), - POLICY_PRIORITY: priority, RANKING_LENGTH: 3, **config_override, } - def create_policy( - self, - featurizer: Optional[TrackerFeaturizer], - priority: int, - model_storage: ModelStorage, - resource: Resource, - execution_context: ExecutionContext, - config: Optional[Dict[Text, Any]] = None, - ) -> Policy: - return TEDPolicy( - featurizer=featurizer, - config=self._config(priority, config), - model_storage=model_storage, - resource=resource, - execution_context=execution_context, - ) - def test_ranking_length(self, trained_policy: TEDPolicy): assert trained_policy.config[RANKING_LENGTH] == 3 class TestTEDPolicyHighRankingLength(TestTEDPolicy): def _config( - self, priority: int, config_override: Optional[Dict[Text, Any]] = None + self, config_override: Optional[Dict[Text, Any]] = None ) -> Dict[Text, Any]: config_override = config_override or {} return { **TEDPolicy.get_default_config(), - POLICY_PRIORITY: priority, RANKING_LENGTH: 11, **config_override, } - def create_policy( - self, - featurizer: Optional[TrackerFeaturizer], - priority: int, - model_storage: ModelStorage, - resource: Resource, - execution_context: ExecutionContext, - config: Optional[Dict[Text, Any]] = None, - ) -> Policy: - return TEDPolicy( - featurizer=featurizer, - config=self._config(priority, config), - model_storage=model_storage, - resource=resource, - execution_context=execution_context, - ) - def test_ranking_length(self, trained_policy: TEDPolicy): assert trained_policy.config[RANKING_LENGTH] == 11 class TestTEDPolicyWithStandardFeaturizer(TestTEDPolicy): def _config( - self, priority: int, config_override: Optional[Dict[Text, Any]] = None + self, config_override: Optional[Dict[Text, Any]] = None ) -> Dict[Text, Any]: config_override = config_override or {} return { **TEDPolicy.get_default_config(), - POLICY_PRIORITY: priority, **config_override, } def create_policy( self, featurizer: Optional[TrackerFeaturizer], - priority: int, model_storage: ModelStorage, resource: Resource, execution_context: ExecutionContext, @@ -929,7 +788,7 @@ def create_policy( # since it is using MaxHistoryTrackerFeaturizer # if max_history is not specified return TEDPolicy( - config=self._config(priority, config), + config=self._config(config), model_storage=model_storage, resource=resource, execution_context=execution_context, @@ -949,7 +808,7 @@ def test_featurizer( ) loaded = trained_policy.__class__.load( - self._config(trained_policy.priority), + self._config(trained_policy.config), model_storage, resource, execution_context, @@ -961,12 +820,11 @@ def test_featurizer( class TestTEDPolicyWithMaxHistory(TestTEDPolicy): def _config( - self, priority: int, config_override: Optional[Dict[Text, Any]] = None + self, config_override: Optional[Dict[Text, Any]] = None ) -> Dict[Text, Any]: config_override = config_override or {} return { **TEDPolicy.get_default_config(), - POLICY_PRIORITY: priority, POLICY_MAX_HISTORY: self.max_history, **config_override, } @@ -974,7 +832,6 @@ def _config( def create_policy( self, featurizer: Optional[TrackerFeaturizer], - priority: int, model_storage: ModelStorage, resource: Resource, execution_context: ExecutionContext, @@ -984,100 +841,39 @@ def create_policy( # since it is using MaxHistoryTrackerFeaturizer # if max_history is specified return TEDPolicy( - config=self._config(priority, config), + config=self._config(config), model_storage=model_storage, resource=resource, execution_context=execution_context, ) - def test_featurizer( - self, - trained_policy: Policy, - resource: Resource, - model_storage: ModelStorage, - tmp_path: Path, - execution_context: ExecutionContext, - ): - assert isinstance(trained_policy.featurizer, MaxHistoryTrackerFeaturizer) - assert trained_policy.featurizer.max_history == self.max_history - assert isinstance( - trained_policy.featurizer.state_featurizer, SingleStateFeaturizer - ) - - loaded = trained_policy.__class__.load( - self._config(trained_policy.priority), - model_storage, - resource, - execution_context, - ) - - assert isinstance(loaded.featurizer, MaxHistoryTrackerFeaturizer) - assert loaded.featurizer.max_history == self.max_history - assert isinstance(loaded.featurizer.state_featurizer, SingleStateFeaturizer) - class TestTEDPolicyWithRelativeAttention(TestTEDPolicy): def _config( - self, priority: int, config_override: Optional[Dict[Text, Any]] = None + self, config_override: Optional[Dict[Text, Any]] = None ) -> Dict[Text, Any]: config_override = config_override or {} return { **TEDPolicy.get_default_config(), - POLICY_PRIORITY: priority, KEY_RELATIVE_ATTENTION: True, VALUE_RELATIVE_ATTENTION: True, MAX_RELATIVE_POSITION: 5, **config_override, } - def create_policy( - self, - featurizer: Optional[TrackerFeaturizer], - priority: int, - model_storage: ModelStorage, - resource: Resource, - execution_context: ExecutionContext, - config: Optional[Dict[Text, Any]] = None, - ) -> Policy: - return TEDPolicy( - featurizer=featurizer, - config=self._config(priority, config), - model_storage=model_storage, - resource=resource, - execution_context=execution_context, - ) - class TestTEDPolicyWithRelativeAttentionMaxHistoryOne(TestTEDPolicy): max_history = 1 def _config( - self, priority: int, config_override: Optional[Dict[Text, Any]] = None + self, config_override: Optional[Dict[Text, Any]] = None ) -> Dict[Text, Any]: config_override = config_override or {} return { **TEDPolicy.get_default_config(), - POLICY_PRIORITY: priority, KEY_RELATIVE_ATTENTION: True, VALUE_RELATIVE_ATTENTION: True, MAX_RELATIVE_POSITION: 5, **config_override, } - - def create_policy( - self, - featurizer: Optional[TrackerFeaturizer], - priority: int, - model_storage: ModelStorage, - resource: Resource, - execution_context: ExecutionContext, - config: Optional[Dict[Text, Any]] = None, - ) -> Policy: - return TEDPolicy( - featurizer=featurizer, - config=self._config(priority, config), - model_storage=model_storage, - resource=resource, - execution_context=execution_context, - ) diff --git a/tests/core/test_policies.py b/tests/core/test_policies.py index 1f0115723a4c..1ddea47b863b 100644 --- a/tests/core/test_policies.py +++ b/tests/core/test_policies.py @@ -1,16 +1,12 @@ import uuid from pathlib import Path -from typing import Type, List, Text, Optional, Dict, Any, Union +from typing import Type, List, Text, Optional, Dict, Any import dataclasses import numpy as np import pytest from _pytest.tmpdir import TempPathFactory -from rasa.core.constants import ( - DEFAULT_POLICY_PRIORITY, - POLICY_MAX_HISTORY, - POLICY_PRIORITY, -) +from rasa.core.constants import POLICY_MAX_HISTORY from rasa.engine.graph import ExecutionContext, GraphSchema, GraphComponent from rasa.engine.storage.local_model_storage import LocalModelStorage from rasa.engine.storage.resource import Resource @@ -68,9 +64,6 @@ def train_trackers( ) -EitherPolicy = Union[Policy, PolicyGraphComponent] - - # We are going to use class style testing here since unfortunately pytest # doesn't support using fixtures as arguments to its own parameterize yet # (hence, we can't train a policy, declare it as a fixture and use the @@ -83,6 +76,10 @@ class PolicyTestCollection: Each policy can declare further tests on its own.""" + @staticmethod + def _policy_class_to_test() -> Type[PolicyGraphComponent]: + raise NotImplementedError + max_history = 3 # this is the amount of history we test on @pytest.fixture(scope="class") @@ -98,20 +95,27 @@ def execution_context(self) -> ExecutionContext: return ExecutionContext(GraphSchema({}), uuid.uuid4().hex) def _config( - self, priority: int, config_override: Optional[Dict[Text, Any]] = None + self, config_override: Optional[Dict[Text, Any]] = None ) -> Dict[Text, Any]: - raise NotImplementedError + config_override = config_override or {} + config = self._policy_class_to_test().get_default_config() + return {**config, **config_override} def create_policy( self, featurizer: Optional[TrackerFeaturizer], - priority: Optional[int], model_storage: ModelStorage, resource: Resource, execution_context: ExecutionContext, config: Optional[Dict[Text, Any]] = None, - ) -> EitherPolicy: - raise NotImplementedError + ) -> PolicyGraphComponent: + return self._policy_class_to_test()( + config=self._config(config), + model_storage=model_storage, + resource=resource, + execution_context=execution_context, + featurizer=featurizer, + ) @pytest.fixture(scope="class") def featurizer(self) -> TrackerFeaturizer: @@ -136,29 +140,24 @@ def tracker(self, default_domain: Domain) -> DialogueStateTracker: def trained_policy( self, featurizer: Optional[TrackerFeaturizer], - priority: int, stories_path: Text, default_domain: Domain, model_storage: ModelStorage, resource: Resource, execution_context: ExecutionContext, - ) -> EitherPolicy: + ) -> PolicyGraphComponent: policy = self.create_policy( - featurizer, priority, model_storage, resource, execution_context + featurizer, model_storage, resource, execution_context ) training_trackers = train_trackers( default_domain, stories_path, augmentation_factor=20 ) - if isinstance(policy, GraphComponent): - policy.train(training_trackers, default_domain) - else: - # TODO: Drop after all policies are migrated to `GraphComponent` - policy.train(training_trackers, default_domain, RegexInterpreter()) + policy.train(training_trackers, default_domain) return policy def test_featurizer( self, - trained_policy: EitherPolicy, + trained_policy: PolicyGraphComponent, resource: Resource, model_storage: ModelStorage, tmp_path: Path, @@ -170,17 +169,12 @@ def test_featurizer( trained_policy.featurizer.state_featurizer, SingleStateFeaturizer ) - if isinstance(trained_policy, GraphComponent): - loaded = trained_policy.__class__.load( - self._config(trained_policy.priority), - model_storage, - resource, - execution_context, - ) - else: - # TODO: Drop after all policies are migrated to `GraphComponent` - trained_policy.persist(str(tmp_path)) - loaded = trained_policy.__class__.load(str(tmp_path)) + loaded = trained_policy.__class__.load( + self._config(trained_policy.config), + model_storage, + resource, + execution_context, + ) assert isinstance(loaded.featurizer, MaxHistoryTrackerFeaturizer) assert loaded.featurizer.max_history == self.max_history @@ -189,28 +183,21 @@ def test_featurizer( @pytest.mark.parametrize("should_finetune", [False, True]) def test_persist_and_load( self, - trained_policy: EitherPolicy, + trained_policy: PolicyGraphComponent, default_domain: Domain, - tmp_path: Path, should_finetune: bool, stories_path: Text, model_storage: ModelStorage, resource: Resource, execution_context: ExecutionContext, ): - if isinstance(trained_policy, GraphComponent): - loaded = trained_policy.__class__.load( - self._config(trained_policy.priority), - model_storage, - resource, - dataclasses.replace(execution_context, is_finetuning=should_finetune), - ) - else: - # TODO: Drop after all policies are migrated to `GraphComponent` - trained_policy.persist(str(tmp_path)) - loaded = trained_policy.__class__.load( - str(tmp_path), should_finetune=should_finetune - ) + loaded = trained_policy.__class__.load( + self._config(trained_policy.config), + model_storage, + resource, + dataclasses.replace(execution_context, is_finetuning=should_finetune), + ) + assert loaded.finetune_mode == should_finetune trackers = train_trackers(default_domain, stories_path, augmentation_factor=20) @@ -241,39 +228,25 @@ def test_prediction_on_empty_tracker( ) def test_persist_and_load_empty_policy( self, - tmp_path: Path, default_domain: Domain, default_model_storage: ModelStorage, execution_context: ExecutionContext, ): resource = Resource(uuid.uuid4().hex) empty_policy = self.create_policy( - None, - DEFAULT_POLICY_PRIORITY, - default_model_storage, - resource, - execution_context, + None, default_model_storage, resource, execution_context, ) - if isinstance(empty_policy, PolicyGraphComponent): - empty_policy.train([], default_domain) - loaded = empty_policy.__class__.load( - self._config(DEFAULT_POLICY_PRIORITY), - default_model_storage, - resource, - execution_context, - ) - else: - # TODO: Drop after all policies are migrated to `GraphComponent` - empty_policy.train([], default_domain, RegexInterpreter()) - empty_policy.persist(str(tmp_path)) - loaded = empty_policy.__class__.load(str(tmp_path)) + empty_policy.train([], default_domain) + loaded = empty_policy.__class__.load( + self._config(), default_model_storage, resource, execution_context, + ) assert loaded is not None @staticmethod def _get_next_action( - policy: EitherPolicy, events: List[Event], domain: Domain + policy: PolicyGraphComponent, events: List[Event], domain: Domain ) -> Text: tracker = get_tracker(events) @@ -331,11 +304,10 @@ def test_different_featurizer_configs( ) policy = self.create_policy( None, - priority=DEFAULT_POLICY_PRIORITY, model_storage=model_storage, resource=resource, execution_context=execution_context, - config=self._config(DEFAULT_POLICY_PRIORITY, featurizer_config_override), + config=self._config(featurizer_config_override), ) if not isinstance(policy, GraphComponent): @@ -348,9 +320,7 @@ def test_different_featurizer_configs( if featurizer_config: expected_max_history = featurizer_config[0].get(POLICY_MAX_HISTORY) else: - expected_max_history = self._config(DEFAULT_POLICY_PRIORITY).get( - POLICY_MAX_HISTORY - ) + expected_max_history = self._config().get(POLICY_MAX_HISTORY) assert featurizer.max_history == expected_max_history @@ -377,7 +347,7 @@ def test_different_featurizer_configs( ) def test_different_invalid_featurizer_configs( self, - trained_policy: EitherPolicy, + trained_policy: PolicyGraphComponent, featurizer_config: Optional[Dict[Text, Any]], model_storage: ModelStorage, resource: Resource, @@ -390,7 +360,6 @@ def test_different_invalid_featurizer_configs( with pytest.raises(InvalidPolicyConfig): self.create_policy( None, - priority=1, model_storage=model_storage, resource=resource, execution_context=execution_context, @@ -399,37 +368,18 @@ def test_different_invalid_featurizer_configs( class TestMemoizationPolicy(PolicyTestCollection): + @staticmethod + def _policy_class_to_test() -> Type[PolicyGraphComponent]: + return MemoizationPolicyGraphComponent + @pytest.fixture(scope="class") def featurizer(self) -> TrackerFeaturizer: featurizer = MaxHistoryTrackerFeaturizer(None, max_history=self.max_history) return featurizer - def _config( - self, priority: int, config_override: Optional[Dict[Text, Any]] = None - ) -> Dict[Text, Any]: - config_override = config_override or {} - config = MemoizationPolicyGraphComponent.get_default_config() - config[POLICY_PRIORITY] = priority - return {**config, **config_override} - - def create_policy( - self, - featurizer: Optional[TrackerFeaturizer], - priority: int, - model_storage: ModelStorage, - resource: Resource, - execution_context: ExecutionContext, - config: Optional[Dict[Text, Any]] = None, - ) -> EitherPolicy: - config = config or MemoizationPolicyGraphComponent.get_default_config() - config[POLICY_PRIORITY] = priority - return MemoizationPolicyGraphComponent( - config, model_storage, resource, execution_context, featurizer - ) - def test_featurizer( self, - trained_policy: EitherPolicy, + trained_policy: PolicyGraphComponent, resource: Resource, model_storage: ModelStorage, tmp_path: Path, @@ -437,10 +387,11 @@ def test_featurizer( ): assert isinstance(trained_policy.featurizer, MaxHistoryTrackerFeaturizer) assert trained_policy.featurizer.state_featurizer is None - config = self._config(DEFAULT_POLICY_PRIORITY) - trained_policy.persist() - loaded = type(trained_policy).load( - config, model_storage, resource, execution_context + loaded = trained_policy.__class__.load( + self._config(trained_policy.config), + model_storage, + resource, + execution_context, ) assert isinstance(loaded.featurizer, MaxHistoryTrackerFeaturizer) assert loaded.featurizer.state_featurizer is None @@ -621,11 +572,10 @@ def test_empty_featurizer_configs( ) policy = self.create_policy( None, - priority=DEFAULT_POLICY_PRIORITY, model_storage=model_storage, resource=resource, execution_context=execution_context, - config=self._config(DEFAULT_POLICY_PRIORITY, featurizer_config_override), + config=self._config(featurizer_config_override), ) featurizer = policy.featurizer @@ -634,9 +584,7 @@ def test_empty_featurizer_configs( if featurizer_config: expected_max_history = featurizer_config[0].get(POLICY_MAX_HISTORY) else: - expected_max_history = self._config(DEFAULT_POLICY_PRIORITY).get( - POLICY_MAX_HISTORY - ) + expected_max_history = self._config().get(POLICY_MAX_HISTORY) assert featurizer.max_history == expected_max_history @@ -644,28 +592,9 @@ def test_empty_featurizer_configs( class TestAugmentedMemoizationPolicy(TestMemoizationPolicy): - def _config( - self, priority: int, config_override: Optional[Dict[Text, Any]] = None - ) -> Dict[Text, Any]: - config_override = config_override or {} - config = AugmentedMemoizationPolicyGraphComponent.get_default_config() - config[POLICY_PRIORITY] = priority - return {**config, **config_override} - - def create_policy( - self, - featurizer: Optional[TrackerFeaturizer], - priority: int, - model_storage: ModelStorage, - resource: Resource, - execution_context: ExecutionContext, - config: Optional[Dict[Text, Any]] = None, - ) -> EitherPolicy: - config = config or AugmentedMemoizationPolicyGraphComponent.get_default_config() - config[POLICY_PRIORITY] = priority - return AugmentedMemoizationPolicyGraphComponent( - config, model_storage, resource, execution_context, featurizer - ) + @staticmethod + def _policy_class_to_test() -> Type[PolicyGraphComponent]: + return AugmentedMemoizationPolicyGraphComponent @pytest.mark.parametrize( From 14c5b0560d7119c611d3880ab98d90f9889ffa79 Mon Sep 17 00:00:00 2001 From: Thomas Werkmeister Date: Tue, 24 Aug 2021 17:23:11 +0200 Subject: [PATCH 18/41] Code quality + unexpecTED adjustment --- tests/core/policies/test_ted_policy.py | 7 +--- .../policies/test_unexpected_intent_policy.py | 32 +------------------ tests/core/test_policies.py | 4 --- 3 files changed, 2 insertions(+), 41 deletions(-) diff --git a/tests/core/policies/test_ted_policy.py b/tests/core/policies/test_ted_policy.py index b7f663ab5801..f5cf1ab1c763 100644 --- a/tests/core/policies/test_ted_policy.py +++ b/tests/core/policies/test_ted_policy.py @@ -6,11 +6,7 @@ import tests.core.test_policies from _pytest.monkeypatch import MonkeyPatch from _pytest.logging import LogCaptureFixture -from rasa.core.constants import ( - POLICY_PRIORITY, - POLICY_MAX_HISTORY, - DEFAULT_POLICY_PRIORITY, -) +from rasa.core.constants import POLICY_MAX_HISTORY from rasa.core.featurizers.single_state_featurizer import SingleStateFeaturizer from rasa.core.featurizers.tracker_featurizers import ( @@ -228,7 +224,6 @@ def test_train_fails_with_checkpoint_zero_eval_num_epochs(self, tmp_path: Path): def test_training_with_no_intent( self, featurizer: Optional[TrackerFeaturizer], - priority: int, default_domain: Domain, tmp_path: Path, caplog: LogCaptureFixture, diff --git a/tests/core/policies/test_unexpected_intent_policy.py b/tests/core/policies/test_unexpected_intent_policy.py index e1726493b799..4269def42ab1 100644 --- a/tests/core/policies/test_unexpected_intent_policy.py +++ b/tests/core/policies/test_unexpected_intent_policy.py @@ -1,5 +1,5 @@ from pathlib import Path -from typing import Optional, List, Dict, Text, Type, Any +from typing import Optional, List, Dict, Text, Type import tensorflow as tf import numpy as np import pytest @@ -7,7 +7,6 @@ from _pytest.logging import LogCaptureFixture import logging -from rasa.core.constants import POLICY_PRIORITY from rasa.core.featurizers.single_state_featurizer import ( IntentTokenizerSingleStateFeaturizer, ) @@ -63,33 +62,6 @@ class TestUnexpecTEDIntentPolicy(TestTEDPolicy): def _policy_class_to_test() -> Type[UnexpecTEDIntentPolicy]: return UnexpecTEDIntentPolicy - def _config( - self, priority: int, config_override: Optional[Dict[Text, Any]] = None - ) -> Dict[Text, Any]: - config_override = config_override or {} - return { - **UnexpecTEDIntentPolicy.get_default_config(), - POLICY_PRIORITY: priority, - **config_override, - } - - def create_policy( - self, - featurizer: Optional[TrackerFeaturizer], - priority: int, - model_storage: ModelStorage, - resource: Resource, - execution_context: ExecutionContext, - config: Optional[Dict[Text, Any]] = None, - ) -> UnexpecTEDIntentPolicy: - return UnexpecTEDIntentPolicy( - self._config(priority, config), - featurizer=featurizer, - model_storage=model_storage, - resource=resource, - execution_context=execution_context, - ) - @pytest.fixture(scope="class") def featurizer(self) -> TrackerFeaturizer: featurizer = IntentMaxHistoryTrackerFeaturizer( @@ -154,7 +126,6 @@ def test_label_data_assembly( def test_training_with_no_intent( self, featurizer: Optional[TrackerFeaturizer], - priority: int, default_domain: Domain, tmp_path: Path, caplog: LogCaptureFixture, @@ -174,7 +145,6 @@ def test_training_with_no_intent( ) policy = self.create_policy( featurizer=featurizer, - priority=priority, model_storage=model_storage, resource=resource, execution_context=execution_context, diff --git a/tests/core/test_policies.py b/tests/core/test_policies.py index 1ddea47b863b..38b809aea9ed 100644 --- a/tests/core/test_policies.py +++ b/tests/core/test_policies.py @@ -124,10 +124,6 @@ def featurizer(self) -> TrackerFeaturizer: ) return featurizer - @pytest.fixture(scope="class") - def priority(self) -> int: - return 1 - @pytest.fixture(scope="class") def default_domain(self, domain_path: Text) -> Domain: return Domain.load(domain_path) From f77066144c558e8f6cd7d11f1068bf4118cc0147 Mon Sep 17 00:00:00 2001 From: Thomas Werkmeister Date: Wed, 25 Aug 2021 09:18:57 +0200 Subject: [PATCH 19/41] Persisting policy by default when training --- rasa/core/policies/memoization.py | 2 ++ tests/core/test_policies.py | 1 - 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/rasa/core/policies/memoization.py b/rasa/core/policies/memoization.py index c70deec017e9..1302c8b8eae7 100644 --- a/rasa/core/policies/memoization.py +++ b/rasa/core/policies/memoization.py @@ -183,6 +183,8 @@ def train( ) logger.debug(f"Memorized {len(self.lookup)} unique examples.") + self.persist() + def _recall_states(self, states: List[State]) -> Optional[Text]: return self.lookup.get(self._create_feature_key(states)) diff --git a/tests/core/test_policies.py b/tests/core/test_policies.py index 38b809aea9ed..3ffda0854d6b 100644 --- a/tests/core/test_policies.py +++ b/tests/core/test_policies.py @@ -454,7 +454,6 @@ def test_finetune_after_load( stories_path: Text, ): - trained_policy.persist() execution_context = dataclasses.replace(execution_context, is_finetuning=True) loaded_policy = MemoizationPolicyGraphComponent.load( trained_policy.config, model_storage, resource, execution_context From f53b21943506d6142f382011b5ee551d6b447db5 Mon Sep 17 00:00:00 2001 From: Thomas Werkmeister Date: Wed, 25 Aug 2021 09:20:54 +0200 Subject: [PATCH 20/41] Keeping original policy name in unit tests --- tests/core/test_policies.py | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/tests/core/test_policies.py b/tests/core/test_policies.py index 3ffda0854d6b..5407e535a97b 100644 --- a/tests/core/test_policies.py +++ b/tests/core/test_policies.py @@ -47,8 +47,8 @@ from rasa.core.policies.rule_policy import RulePolicy from rasa.core.policies.ted_policy import TEDPolicy from rasa.core.policies.memoization import ( - AugmentedMemoizationPolicyGraphComponent, - MemoizationPolicyGraphComponent, + AugmentedMemoizationPolicyGraphComponent as AugmentedMemoizationPolicy, + MemoizationPolicyGraphComponent as MemoizationPolicy, ) from rasa.shared.core.trackers import DialogueStateTracker @@ -366,7 +366,7 @@ def test_different_invalid_featurizer_configs( class TestMemoizationPolicy(PolicyTestCollection): @staticmethod def _policy_class_to_test() -> Type[PolicyGraphComponent]: - return MemoizationPolicyGraphComponent + return MemoizationPolicy @pytest.fixture(scope="class") def featurizer(self) -> TrackerFeaturizer: @@ -394,7 +394,7 @@ def test_featurizer( def test_memorise( self, - trained_policy: MemoizationPolicyGraphComponent, + trained_policy: MemoizationPolicy, default_domain: Domain, stories_path: Text, ): @@ -436,7 +436,7 @@ def test_memorise( assert lookup_no_augmentation == lookup_with_augmentation def test_memorise_with_nlu( - self, trained_policy: MemoizationPolicyGraphComponent, default_domain: Domain + self, trained_policy: MemoizationPolicy, default_domain: Domain ): tracker = tracker_from_dialogue(TEST_DEFAULT_DIALOGUE, default_domain) states = trained_policy._prediction_states(tracker, default_domain) @@ -446,7 +446,7 @@ def test_memorise_with_nlu( def test_finetune_after_load( self, - trained_policy: MemoizationPolicyGraphComponent, + trained_policy: MemoizationPolicy, resource: Resource, model_storage: ModelStorage, execution_context: ExecutionContext, @@ -455,7 +455,7 @@ def test_finetune_after_load( ): execution_context = dataclasses.replace(execution_context, is_finetuning=True) - loaded_policy = MemoizationPolicyGraphComponent.load( + loaded_policy = MemoizationPolicy.load( trained_policy.config, model_storage, resource, execution_context ) @@ -520,7 +520,7 @@ def test_finetune_after_load( ) def test_ignore_action_unlikely_intent( self, - trained_policy: MemoizationPolicyGraphComponent, + trained_policy: MemoizationPolicy, default_domain: Domain, tracker_events_with_action: List[Event], tracker_events_without_action: List[Event], @@ -589,7 +589,7 @@ def test_empty_featurizer_configs( class TestAugmentedMemoizationPolicy(TestMemoizationPolicy): @staticmethod def _policy_class_to_test() -> Type[PolicyGraphComponent]: - return AugmentedMemoizationPolicyGraphComponent + return AugmentedMemoizationPolicy @pytest.mark.parametrize( @@ -597,7 +597,7 @@ def _policy_class_to_test() -> Type[PolicyGraphComponent]: [ (TEDPolicy, SupportedData.ML_DATA), (RulePolicy, SupportedData.ML_AND_RULE_DATA), - (MemoizationPolicyGraphComponent, SupportedData.ML_DATA), + (MemoizationPolicy, SupportedData.ML_DATA), ], ) def test_supported_data(policy: Type[Policy], supported_data: SupportedData): From 0607458313d4dfe84bfac9ac6f8bfe296d8da527 Mon Sep 17 00:00:00 2001 From: Thomas Werkmeister Date: Wed, 25 Aug 2021 09:39:36 +0200 Subject: [PATCH 21/41] Only carrying over max_history from policy to featurizer for MaxHistoryTrackerFeaturizers --- rasa/core/policies/policy.py | 17 ++++++++--------- 1 file changed, 8 insertions(+), 9 deletions(-) diff --git a/rasa/core/policies/policy.py b/rasa/core/policies/policy.py index 26f8d13a505c..60b99f7939a1 100644 --- a/rasa/core/policies/policy.py +++ b/rasa/core/policies/policy.py @@ -159,14 +159,6 @@ def _create_featurizer(self, policy_config: Dict[Text, Any]) -> TrackerFeaturize if not featurizer_configs: return self._standard_featurizer() - if ( - POLICY_MAX_HISTORY not in featurizer_configs[0] - and POLICY_MAX_HISTORY in policy_config - ): - featurizer_configs[0][POLICY_MAX_HISTORY] = policy_config[ - POLICY_MAX_HISTORY - ] - featurizer_func = _get_featurizer_from_config( featurizer_configs, self.__class__.__name__, @@ -187,7 +179,14 @@ def _create_featurizer(self, policy_config: Dict[Text, Any]) -> TrackerFeaturize **state_featurizer_config ) - return featurizer_func(**featurizer_config) + featurizer = featurizer_func(**featurizer_config) + if ( + isinstance(featurizer, MaxHistoryTrackerFeaturizer) + and POLICY_MAX_HISTORY in policy_config + and POLICY_MAX_HISTORY not in featurizer_config + ): + featurizer.max_history = policy_config[POLICY_MAX_HISTORY] + return featurizer def _standard_featurizer(self) -> MaxHistoryTrackerFeaturizer: """Initializes the standard featurizer for this policy.""" From d7543e14c8efd754c372353746ffa7cd3f7c9d17 Mon Sep 17 00:00:00 2001 From: Thomas Werkmeister Date: Wed, 25 Aug 2021 10:51:28 +0200 Subject: [PATCH 22/41] Turned load fail logging to warning from info --- rasa/core/policies/memoization.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rasa/core/policies/memoization.py b/rasa/core/policies/memoization.py index 1302c8b8eae7..f8aae4437787 100644 --- a/rasa/core/policies/memoization.py +++ b/rasa/core/policies/memoization.py @@ -280,7 +280,7 @@ def load( featurizer = TrackerFeaturizer.load(path) except (ValueError, FileNotFoundError, FileIOException): - logger.info( + logger.warning( f"Couldn't load metadata for policy '{cls.__name__}' as the persisted " f"metadata couldn't be loaded." ) From e6e9fca67fc33f37d0f5e0cc0775649eed854187 Mon Sep 17 00:00:00 2001 From: Thomas Werkmeister Date: Wed, 25 Aug 2021 10:55:21 +0200 Subject: [PATCH 23/41] Removed forward reference --- rasa/core/policies/memoization.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/rasa/core/policies/memoization.py b/rasa/core/policies/memoization.py index f8aae4437787..bce59552c8aa 100644 --- a/rasa/core/policies/memoization.py +++ b/rasa/core/policies/memoization.py @@ -1,3 +1,4 @@ +from __future__ import annotations import zlib import base64 @@ -265,7 +266,7 @@ def load( resource: Resource, execution_context: ExecutionContext, **kwargs: Any, - ) -> "MemoizationPolicyGraphComponent": + ) -> MemoizationPolicyGraphComponent: """Loads a trained policy (see parent class for full docstring).""" featurizer = None lookup = None From 63074ea85b8269ad13a067a6ab80d0e36b6fc799 Mon Sep 17 00:00:00 2001 From: Thomas Werkmeister Date: Wed, 25 Aug 2021 10:55:34 +0200 Subject: [PATCH 24/41] Accessing POLICY_MAX_HISTORY directly --- rasa/core/policies/ted_policy.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rasa/core/policies/ted_policy.py b/rasa/core/policies/ted_policy.py index a1f4958523b2..e356360a0aae 100644 --- a/rasa/core/policies/ted_policy.py +++ b/rasa/core/policies/ted_policy.py @@ -346,7 +346,7 @@ def get_default_config() -> Dict[Text, Any]: def _standard_featurizer(self) -> TrackerFeaturizer: return MaxHistoryTrackerFeaturizer( - SingleStateFeaturizer(), max_history=self.config.get(POLICY_MAX_HISTORY) + SingleStateFeaturizer(), max_history=self.config[POLICY_MAX_HISTORY] ) def __init__( From d1c2bb8465f9bc195d530249d92f659fd24d789a Mon Sep 17 00:00:00 2001 From: Thomas Werkmeister Date: Wed, 25 Aug 2021 10:58:20 +0200 Subject: [PATCH 25/41] Removed compatibility code for old policy component types --- tests/core/test_policies.py | 16 ++-------------- 1 file changed, 2 insertions(+), 14 deletions(-) diff --git a/tests/core/test_policies.py b/tests/core/test_policies.py index 5407e535a97b..219c70fa60fa 100644 --- a/tests/core/test_policies.py +++ b/tests/core/test_policies.py @@ -306,10 +306,6 @@ def test_different_featurizer_configs( config=self._config(featurizer_config_override), ) - if not isinstance(policy, GraphComponent): - # TODO: Drop this after all policies have been migration to graph components - return - featurizer = policy.featurizer assert isinstance(featurizer, tracker_featurizer.__class__) @@ -349,10 +345,6 @@ def test_different_invalid_featurizer_configs( resource: Resource, execution_context: ExecutionContext, ): - if not isinstance(trained_policy, GraphComponent): - # TODO: Drop this after all policies have been migration to graph components - return - with pytest.raises(InvalidPolicyConfig): self.create_policy( None, @@ -425,12 +417,8 @@ def test_memorise( trackers_no_augmentation = train_trackers( default_domain, stories_path, augmentation_factor=0 ) - if isinstance(trained_policy, PolicyGraphComponent): - trained_policy.train(trackers_no_augmentation, default_domain) - else: - trained_policy.train( - trackers_no_augmentation, default_domain, RegexInterpreter() - ) + trained_policy.train(trackers_no_augmentation, default_domain) + lookup_no_augmentation = trained_policy.lookup assert lookup_no_augmentation == lookup_with_augmentation From 70be73bd0d6edf89807f8273f53fa08f55297613 Mon Sep 17 00:00:00 2001 From: Thomas Werkmeister Date: Wed, 25 Aug 2021 11:04:07 +0200 Subject: [PATCH 26/41] Removed unused import --- tests/core/test_policies.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/core/test_policies.py b/tests/core/test_policies.py index 219c70fa60fa..642ad132a012 100644 --- a/tests/core/test_policies.py +++ b/tests/core/test_policies.py @@ -7,7 +7,7 @@ import pytest from _pytest.tmpdir import TempPathFactory from rasa.core.constants import POLICY_MAX_HISTORY -from rasa.engine.graph import ExecutionContext, GraphSchema, GraphComponent +from rasa.engine.graph import ExecutionContext, GraphSchema from rasa.engine.storage.local_model_storage import LocalModelStorage from rasa.engine.storage.resource import Resource from rasa.engine.storage.storage import ModelStorage From 18cad3ebc5457da781240dd298c35ada34916417 Mon Sep 17 00:00:00 2001 From: Thomas Werkmeister Date: Wed, 25 Aug 2021 11:31:28 +0200 Subject: [PATCH 27/41] Moved persist implementation from Policy class to MemoizationPolicy --- rasa/core/policies/memoization.py | 12 ++++++++++++ rasa/core/policies/policy.py | 10 +--------- 2 files changed, 13 insertions(+), 9 deletions(-) diff --git a/rasa/core/policies/memoization.py b/rasa/core/policies/memoization.py index bce59552c8aa..d4a9894c0812 100644 --- a/rasa/core/policies/memoization.py +++ b/rasa/core/policies/memoization.py @@ -258,6 +258,18 @@ def _metadata(self) -> Dict[Text, Any]: def _metadata_filename(cls) -> Text: return "memorized_turns.json" + def persist(self) -> None: + """Persists the policy to storage.""" + with self._model_storage.write_to(self._resource) as path: + # not all policies have a featurizer + if self.featurizer is not None: + self.featurizer.persist(path) + + file = Path(path) / self._metadata_filename() + + rasa.shared.utils.io.create_directory_for_file(file) + rasa.shared.utils.io.dump_obj_as_json_to_file(file, self._metadata()) + @classmethod def load( cls, diff --git a/rasa/core/policies/policy.py b/rasa/core/policies/policy.py index 60b99f7939a1..a25bfb620d9c 100644 --- a/rasa/core/policies/policy.py +++ b/rasa/core/policies/policy.py @@ -424,15 +424,7 @@ def _metadata_filename(cls) -> Text: def persist(self) -> None: """Persists the policy to storage.""" - with self._model_storage.write_to(self._resource) as path: - # not all policies have a featurizer - if self.featurizer is not None: - self.featurizer.persist(path) - - file = Path(path) / self._metadata_filename() - - rasa.shared.utils.io.create_directory_for_file(file) - rasa.shared.utils.io.dump_obj_as_json_to_file(file, self._metadata()) + pass @classmethod def load( From cf64b39f4c5f57b9f57f247e7330c40238944ea1 Mon Sep 17 00:00:00 2001 From: Thomas Werkmeister Date: Mon, 30 Aug 2021 10:15:05 +0200 Subject: [PATCH 28/41] Removed metadata and persist abstract methods from policy class --- rasa/core/policies/policy.py | 27 --------------------------- 1 file changed, 27 deletions(-) diff --git a/rasa/core/policies/policy.py b/rasa/core/policies/policy.py index a25bfb620d9c..0c7a76a76c3e 100644 --- a/rasa/core/policies/policy.py +++ b/rasa/core/policies/policy.py @@ -399,33 +399,6 @@ def _prediction( action_metadata=action_metadata, ) - def _metadata(self) -> Optional[Dict[Text, Any]]: - """Returns this policy's attributes that should be persisted. - - Policies using the default `persist()` and `load()` implementations must - implement the `_metadata()` method." - - Returns: - The policy metadata. - """ - pass - - @classmethod - def _metadata_filename(cls) -> Text: - """Returns the filename of the persisted policy metadata. - - Policies using the default `persist()` and `load()` implementations must - implement the `_metadata_filename()` method. - - Returns: - The filename of the persisted policy metadata. - """ - pass - - def persist(self) -> None: - """Persists the policy to storage.""" - pass - @classmethod def load( cls, From a1f2cbf715108f791d5a1becdadaa9ee5e1d6ba2 Mon Sep 17 00:00:00 2001 From: Thomas Werkmeister Date: Mon, 30 Aug 2021 13:39:06 +0200 Subject: [PATCH 29/41] Fixed access to policy.max_history --- rasa/core/policies/memoization.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/rasa/core/policies/memoization.py b/rasa/core/policies/memoization.py index c8f1384563ae..d2fec4cb861b 100644 --- a/rasa/core/policies/memoization.py +++ b/rasa/core/policies/memoization.py @@ -381,7 +381,9 @@ def _recall_using_delorean( logger.debug("Launch DeLorean...") # Truncate the tracker based on `max_history` - mcfly_tracker = _trim_tracker_by_max_history(tracker, self.max_history) + mcfly_tracker = _trim_tracker_by_max_history( + tracker, self.config[POLICY_MAX_HISTORY] + ) mcfly_tracker = self._back_to_the_future(tracker) while mcfly_tracker is not None: states = self._prediction_states(mcfly_tracker, domain,) From 7bda727f19959f22259905e6bb7ade91b589d2f5 Mon Sep 17 00:00:00 2001 From: Thomas Werkmeister Date: Mon, 30 Aug 2021 14:31:45 +0200 Subject: [PATCH 30/41] Test commit --- tests/core/test_policies.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tests/core/test_policies.py b/tests/core/test_policies.py index 642ad132a012..d7d67f8b2a17 100644 --- a/tests/core/test_policies.py +++ b/tests/core/test_policies.py @@ -575,6 +575,8 @@ def test_empty_featurizer_configs( class TestAugmentedMemoizationPolicy(TestMemoizationPolicy): + """Test suite for AugmentedMemoizationPolicy.""" + @staticmethod def _policy_class_to_test() -> Type[PolicyGraphComponent]: return AugmentedMemoizationPolicy From 3958782166e88186421c3acfe39101ae2b029ccf Mon Sep 17 00:00:00 2001 From: Thomas Werkmeister Date: Mon, 30 Aug 2021 15:30:06 +0200 Subject: [PATCH 31/41] Joined all memoization policy tests --- tests/core/policies/test_memoization.py | 195 ------------------------ tests/core/test_policies.py | 145 ++++++++++++++++++ 2 files changed, 145 insertions(+), 195 deletions(-) delete mode 100644 tests/core/policies/test_memoization.py diff --git a/tests/core/policies/test_memoization.py b/tests/core/policies/test_memoization.py deleted file mode 100644 index 9eb8ad415322..000000000000 --- a/tests/core/policies/test_memoization.py +++ /dev/null @@ -1,195 +0,0 @@ -import pytest - -from rasa.engine.graph import ExecutionContext -from rasa.engine.storage.resource import Resource -from rasa.engine.storage.storage import ModelStorage -from tests.core.test_policies import PolicyTestCollection -from typing import Optional, Dict, Text, Any -from rasa.core.featurizers.tracker_featurizers import ( - TrackerFeaturizer, - MaxHistoryTrackerFeaturizer, -) -from rasa.shared.core.generator import TrackerWithCachedStates -from rasa.core.policies.memoization import AugmentedMemoizationPolicy, MemoizationPolicy -from rasa.shared.core.domain import Domain -from rasa.shared.core.events import ( - ActionExecuted, - UserUttered, - SlotSet, -) -from rasa.shared.nlu.interpreter import RegexInterpreter - - -class TestMemoizationPolicy(PolicyTestCollection): - def create_policy( - self, - featurizer: Optional[TrackerFeaturizer], - priority: Optional[int], - model_storage: ModelStorage, - resource: Resource, - execution_context: ExecutionContext, - config: Optional[Dict[Text, Any]] = None, - ) -> MemoizationPolicy: - return MemoizationPolicy(featurizer=featurizer, priority=priority) - - @pytest.mark.parametrize("max_history", [1, 2, 3, 4, None]) - def test_prediction( - self, - max_history: Optional[int], - model_storage: ModelStorage, - resource: Resource, - execution_context: ExecutionContext, - ): - policy = self.create_policy( - featurizer=MaxHistoryTrackerFeaturizer(max_history=max_history), - priority=1, - model_storage=model_storage, - resource=resource, - execution_context=execution_context, - ) - - GREET_INTENT_NAME = "greet" - UTTER_GREET_ACTION = "utter_greet" - UTTER_BYE_ACTION = "utter_goodbye" - domain = Domain.from_yaml( - f""" - intents: - - {GREET_INTENT_NAME} - actions: - - {UTTER_GREET_ACTION} - - {UTTER_BYE_ACTION} - slots: - slot_1: - type: bool - slot_2: - type: bool - slot_3: - type: bool - slot_4: - type: bool - """ - ) - events = [ - UserUttered(intent={"name": GREET_INTENT_NAME}), - ActionExecuted(UTTER_GREET_ACTION), - SlotSet("slot_1", True), - ActionExecuted(UTTER_GREET_ACTION), - SlotSet("slot_2", True), - SlotSet("slot_3", True), - ActionExecuted(UTTER_GREET_ACTION), - ActionExecuted(UTTER_GREET_ACTION), - UserUttered(intent={"name": GREET_INTENT_NAME}), - ActionExecuted(UTTER_GREET_ACTION), - SlotSet("slot_4", True), - ActionExecuted(UTTER_BYE_ACTION), - ] - training_story = TrackerWithCachedStates.from_events( - "training story", evts=events, domain=domain, slots=domain.slots, - ) - test_story = TrackerWithCachedStates.from_events( - "training story", events[:-1], domain=domain, slots=domain.slots, - ) - interpreter = RegexInterpreter() - policy.train([training_story], domain, interpreter) - prediction = policy.predict_action_probabilities( - test_story, domain, interpreter - ) - assert ( - domain.action_names_or_texts[ - prediction.probabilities.index(max(prediction.probabilities)) - ] - == UTTER_BYE_ACTION - ) - - -class TestAugmentedMemoizationPolicy(TestMemoizationPolicy): - def create_policy( - self, - featurizer: Optional[TrackerFeaturizer], - priority: Optional[int], - model_storage: ModelStorage, - resource: Resource, - execution_context: ExecutionContext, - config: Optional[Dict[Text, Any]] = None, - ) -> MemoizationPolicy: - return AugmentedMemoizationPolicy(featurizer=featurizer, priority=priority) - - @pytest.mark.parametrize("max_history", [1, 2, 3, 4, None]) - def test_augmented_prediction( - self, - max_history: Optional[int], - model_storage: ModelStorage, - resource: Resource, - execution_context: ExecutionContext, - ): - policy = self.create_policy( - featurizer=MaxHistoryTrackerFeaturizer(max_history=max_history), - priority=1, - model_storage=model_storage, - resource=resource, - execution_context=execution_context, - ) - - GREET_INTENT_NAME = "greet" - UTTER_GREET_ACTION = "utter_greet" - UTTER_BYE_ACTION = "utter_goodbye" - domain = Domain.from_yaml( - f""" - intents: - - {GREET_INTENT_NAME} - actions: - - {UTTER_GREET_ACTION} - - {UTTER_BYE_ACTION} - slots: - slot_1: - type: bool - initial_value: true - slot_2: - type: bool - slot_3: - type: bool - """ - ) - training_story = TrackerWithCachedStates.from_events( - "training story", - [ - ActionExecuted(UTTER_GREET_ACTION), - UserUttered(intent={"name": GREET_INTENT_NAME}), - ActionExecuted(UTTER_GREET_ACTION), - SlotSet("slot_3", True), - ActionExecuted(UTTER_BYE_ACTION), - ], - domain=domain, - slots=domain.slots, - ) - test_story = TrackerWithCachedStates.from_events( - "test story", - [ - UserUttered(intent={"name": GREET_INTENT_NAME}), - ActionExecuted(UTTER_GREET_ACTION), - SlotSet("slot_1", False), - ActionExecuted(UTTER_GREET_ACTION), - ActionExecuted(UTTER_GREET_ACTION), - UserUttered(intent={"name": GREET_INTENT_NAME}), - ActionExecuted(UTTER_GREET_ACTION), - SlotSet("slot_2", True), - ActionExecuted(UTTER_GREET_ACTION), - UserUttered(intent={"name": GREET_INTENT_NAME}), - ActionExecuted(UTTER_GREET_ACTION), - SlotSet("slot_3", True), - # ActionExecuted(UTTER_BYE_ACTION), - ], - domain=domain, - slots=domain.slots, - ) - interpreter = RegexInterpreter() - policy.train([training_story], domain, interpreter) - prediction = policy.predict_action_probabilities( - test_story, domain, interpreter - ) - assert ( - domain.action_names_or_texts[ - prediction.probabilities.index(max(prediction.probabilities)) - ] - == UTTER_BYE_ACTION - ) diff --git a/tests/core/test_policies.py b/tests/core/test_policies.py index d7d67f8b2a17..45dc7a2d0e9b 100644 --- a/tests/core/test_policies.py +++ b/tests/core/test_policies.py @@ -573,6 +573,73 @@ def test_empty_featurizer_configs( assert isinstance(featurizer.state_featurizer, state_featurizer) + @pytest.mark.parametrize("max_history", [1, 2, 3, 4, None]) + def test_prediction( + self, + max_history: Optional[int], + model_storage: ModelStorage, + resource: Resource, + execution_context: ExecutionContext, + ): + policy = self.create_policy( + featurizer=MaxHistoryTrackerFeaturizer(max_history=max_history), + model_storage=model_storage, + resource=resource, + execution_context=execution_context, + ) + + GREET_INTENT_NAME = "greet" + UTTER_GREET_ACTION = "utter_greet" + UTTER_BYE_ACTION = "utter_goodbye" + domain = Domain.from_yaml( + f""" + intents: + - {GREET_INTENT_NAME} + actions: + - {UTTER_GREET_ACTION} + - {UTTER_BYE_ACTION} + slots: + slot_1: + type: bool + slot_2: + type: bool + slot_3: + type: bool + slot_4: + type: bool + """ + ) + events = [ + UserUttered(intent={"name": GREET_INTENT_NAME}), + ActionExecuted(UTTER_GREET_ACTION), + SlotSet("slot_1", True), + ActionExecuted(UTTER_GREET_ACTION), + SlotSet("slot_2", True), + SlotSet("slot_3", True), + ActionExecuted(UTTER_GREET_ACTION), + ActionExecuted(UTTER_GREET_ACTION), + UserUttered(intent={"name": GREET_INTENT_NAME}), + ActionExecuted(UTTER_GREET_ACTION), + SlotSet("slot_4", True), + ActionExecuted(UTTER_BYE_ACTION), + ] + training_story = TrackerWithCachedStates.from_events( + "training story", evts=events, domain=domain, slots=domain.slots, + ) + test_story = TrackerWithCachedStates.from_events( + "training story", events[:-1], domain=domain, slots=domain.slots, + ) + policy.train([training_story], domain) + prediction = policy.predict_action_probabilities( + test_story, domain, RegexInterpreter() + ) + assert ( + domain.action_names_or_texts[ + prediction.probabilities.index(max(prediction.probabilities)) + ] + == UTTER_BYE_ACTION + ) + class TestAugmentedMemoizationPolicy(TestMemoizationPolicy): """Test suite for AugmentedMemoizationPolicy.""" @@ -581,6 +648,84 @@ class TestAugmentedMemoizationPolicy(TestMemoizationPolicy): def _policy_class_to_test() -> Type[PolicyGraphComponent]: return AugmentedMemoizationPolicy + @pytest.mark.parametrize("max_history", [1, 2, 3, 4, None]) + def test_augmented_prediction( + self, + max_history: Optional[int], + model_storage: ModelStorage, + resource: Resource, + execution_context: ExecutionContext, + ): + policy = self.create_policy( + featurizer=MaxHistoryTrackerFeaturizer(max_history=max_history), + model_storage=model_storage, + resource=resource, + execution_context=execution_context, + ) + + GREET_INTENT_NAME = "greet" + UTTER_GREET_ACTION = "utter_greet" + UTTER_BYE_ACTION = "utter_goodbye" + domain = Domain.from_yaml( + f""" + intents: + - {GREET_INTENT_NAME} + actions: + - {UTTER_GREET_ACTION} + - {UTTER_BYE_ACTION} + slots: + slot_1: + type: bool + initial_value: true + slot_2: + type: bool + slot_3: + type: bool + """ + ) + training_story = TrackerWithCachedStates.from_events( + "training story", + [ + ActionExecuted(UTTER_GREET_ACTION), + UserUttered(intent={"name": GREET_INTENT_NAME}), + ActionExecuted(UTTER_GREET_ACTION), + SlotSet("slot_3", True), + ActionExecuted(UTTER_BYE_ACTION), + ], + domain=domain, + slots=domain.slots, + ) + test_story = TrackerWithCachedStates.from_events( + "test story", + [ + UserUttered(intent={"name": GREET_INTENT_NAME}), + ActionExecuted(UTTER_GREET_ACTION), + SlotSet("slot_1", False), + ActionExecuted(UTTER_GREET_ACTION), + ActionExecuted(UTTER_GREET_ACTION), + UserUttered(intent={"name": GREET_INTENT_NAME}), + ActionExecuted(UTTER_GREET_ACTION), + SlotSet("slot_2", True), + ActionExecuted(UTTER_GREET_ACTION), + UserUttered(intent={"name": GREET_INTENT_NAME}), + ActionExecuted(UTTER_GREET_ACTION), + SlotSet("slot_3", True), + # ActionExecuted(UTTER_BYE_ACTION), + ], + domain=domain, + slots=domain.slots, + ) + policy.train([training_story], domain) + prediction = policy.predict_action_probabilities( + test_story, domain, RegexInterpreter() + ) + assert ( + domain.action_names_or_texts[ + prediction.probabilities.index(max(prediction.probabilities)) + ] + == UTTER_BYE_ACTION + ) + @pytest.mark.parametrize( "policy,supported_data", From 1018ff8364cb4a6e71e058f052fef8f41381820f Mon Sep 17 00:00:00 2001 From: Thomas Werkmeister Date: Mon, 30 Aug 2021 15:34:57 +0200 Subject: [PATCH 32/41] Fixed delorean code --- rasa/core/policies/memoization.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rasa/core/policies/memoization.py b/rasa/core/policies/memoization.py index d2fec4cb861b..ff7420c4cab6 100644 --- a/rasa/core/policies/memoization.py +++ b/rasa/core/policies/memoization.py @@ -384,7 +384,7 @@ def _recall_using_delorean( mcfly_tracker = _trim_tracker_by_max_history( tracker, self.config[POLICY_MAX_HISTORY] ) - mcfly_tracker = self._back_to_the_future(tracker) + mcfly_tracker = self._back_to_the_future(mcfly_tracker) while mcfly_tracker is not None: states = self._prediction_states(mcfly_tracker, domain,) From 0c59a5b2355de720b316971894c4ec2791bd4d5b Mon Sep 17 00:00:00 2001 From: Thomas Werkmeister Date: Mon, 30 Aug 2021 16:23:49 +0200 Subject: [PATCH 33/41] Update rasa/core/policies/memoization.py Co-authored-by: Joe Juzl --- rasa/core/policies/memoization.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rasa/core/policies/memoization.py b/rasa/core/policies/memoization.py index ff7420c4cab6..dce6670079e4 100644 --- a/rasa/core/policies/memoization.py +++ b/rasa/core/policies/memoization.py @@ -286,7 +286,7 @@ def load( try: with model_storage.read_from(resource) as path: metadata_file = Path(path) / cls._metadata_filename() - metadata = json.loads(rasa.shared.utils.io.read_file(metadata_file)) + metadata = rasa.shared.utils.io.read_json_file(metadata_file) lookup = metadata["lookup"] if (Path(path) / FEATURIZER_FILE).is_file(): From 919827dd90b36badaff8ef3e992757518bf3d2d0 Mon Sep 17 00:00:00 2001 From: Thomas Werkmeister Date: Mon, 30 Aug 2021 16:35:37 +0200 Subject: [PATCH 34/41] policy featurizer creation refactoring --- rasa/core/policies/policy.py | 16 +++++++--------- 1 file changed, 7 insertions(+), 9 deletions(-) diff --git a/rasa/core/policies/policy.py b/rasa/core/policies/policy.py index 0c7a76a76c3e..18457afb8f6b 100644 --- a/rasa/core/policies/policy.py +++ b/rasa/core/policies/policy.py @@ -128,7 +128,7 @@ def __init__( """Constructs a new Policy object.""" self.config = config if featurizer is None: - featurizer = self._create_featurizer(config) + featurizer = self._create_featurizer() self.__featurizer = featurizer self.priority = config.get(POLICY_PRIORITY, DEFAULT_POLICY_PRIORITY) @@ -151,10 +151,8 @@ def create( """Creates a new untrained policy (see parent class for full docstring).""" return cls(config, model_storage, resource, execution_context) - def _create_featurizer(self, policy_config: Dict[Text, Any]) -> TrackerFeaturizer: - policy_config = copy.deepcopy(policy_config) - - featurizer_configs = policy_config.get("featurizer") + def _create_featurizer(self) -> TrackerFeaturizer: + featurizer_configs = self.config.get("featurizer") if not featurizer_configs: return self._standard_featurizer() @@ -166,7 +164,7 @@ def _create_featurizer(self, policy_config: Dict[Text, Any]) -> TrackerFeaturize ) featurizer_config = featurizer_configs[0] - state_featurizer_configs = featurizer_config.pop("state_featurizer", None) + state_featurizer_configs = featurizer_config.get("state_featurizer", None) if state_featurizer_configs: state_featurizer_func = _get_featurizer_from_config( state_featurizer_configs, @@ -182,10 +180,10 @@ def _create_featurizer(self, policy_config: Dict[Text, Any]) -> TrackerFeaturize featurizer = featurizer_func(**featurizer_config) if ( isinstance(featurizer, MaxHistoryTrackerFeaturizer) - and POLICY_MAX_HISTORY in policy_config + and POLICY_MAX_HISTORY in self.config and POLICY_MAX_HISTORY not in featurizer_config ): - featurizer.max_history = policy_config[POLICY_MAX_HISTORY] + featurizer.max_history = self.config[POLICY_MAX_HISTORY] return featurizer def _standard_featurizer(self) -> MaxHistoryTrackerFeaturizer: @@ -653,7 +651,7 @@ def _get_featurizer_from_config( ) featurizer_config = config[0] - featurizer_name = featurizer_config.pop("name") + featurizer_name = featurizer_config["name"] featurizer_func = rasa.shared.utils.common.class_from_module_path( featurizer_name, lookup_path=lookup_path ) From f17ff762be673c59b5aa59c9da2752a02f2bfdfa Mon Sep 17 00:00:00 2001 From: Thomas Werkmeister Date: Mon, 30 Aug 2021 16:41:50 +0200 Subject: [PATCH 35/41] Made _standard_featurizer more uniform and fixed it in UnexpectedTED policy --- rasa/core/policies/policy.py | 4 +++- rasa/core/policies/ted_policy.py | 5 ----- rasa/core/policies/unexpected_intent_policy.py | 8 ++++---- 3 files changed, 7 insertions(+), 10 deletions(-) diff --git a/rasa/core/policies/policy.py b/rasa/core/policies/policy.py index 18457afb8f6b..baf0c1546e2b 100644 --- a/rasa/core/policies/policy.py +++ b/rasa/core/policies/policy.py @@ -188,7 +188,9 @@ def _create_featurizer(self) -> TrackerFeaturizer: def _standard_featurizer(self) -> MaxHistoryTrackerFeaturizer: """Initializes the standard featurizer for this policy.""" - return MaxHistoryTrackerFeaturizer(SingleStateFeaturizer()) + return MaxHistoryTrackerFeaturizer( + SingleStateFeaturizer(), self.config.get(POLICY_MAX_HISTORY) + ) @property def featurizer(self) -> TrackerFeaturizer: diff --git a/rasa/core/policies/ted_policy.py b/rasa/core/policies/ted_policy.py index e356360a0aae..4f4703a73275 100644 --- a/rasa/core/policies/ted_policy.py +++ b/rasa/core/policies/ted_policy.py @@ -344,11 +344,6 @@ def get_default_config() -> Dict[Text, Any]: POLICY_PRIORITY: DEFAULT_POLICY_PRIORITY, } - def _standard_featurizer(self) -> TrackerFeaturizer: - return MaxHistoryTrackerFeaturizer( - SingleStateFeaturizer(), max_history=self.config[POLICY_MAX_HISTORY] - ) - def __init__( self, config: Dict[Text, Any], diff --git a/rasa/core/policies/unexpected_intent_policy.py b/rasa/core/policies/unexpected_intent_policy.py index 266858a59eea..beafa4288c92 100644 --- a/rasa/core/policies/unexpected_intent_policy.py +++ b/rasa/core/policies/unexpected_intent_policy.py @@ -30,7 +30,7 @@ IntentTokenizerSingleStateFeaturizer, ) from rasa.shared.core.generator import TrackerWithCachedStates -from rasa.core.constants import DIALOGUE +from rasa.core.constants import DIALOGUE, POLICY_MAX_HISTORY from rasa.core.policies.policy import PolicyPrediction from rasa.core.policies.ted_policy import ( LABEL_KEY, @@ -326,10 +326,10 @@ def __init__( common.mark_as_experimental_feature("UnexpecTED Intent Policy") - @staticmethod - def _standard_featurizer(max_history: Optional[int] = None) -> TrackerFeaturizer: + def _standard_featurizer(self) -> TrackerFeaturizer: return IntentMaxHistoryTrackerFeaturizer( - IntentTokenizerSingleStateFeaturizer(), max_history=max_history + IntentTokenizerSingleStateFeaturizer(), + max_history=self.config.get(POLICY_MAX_HISTORY), ) @staticmethod From fc451641225c8dacdf0a9915f53b44fbde3b5354 Mon Sep 17 00:00:00 2001 From: Thomas Werkmeister Date: Mon, 30 Aug 2021 16:57:44 +0200 Subject: [PATCH 36/41] Removed unused imports --- rasa/core/policies/policy.py | 1 - rasa/core/policies/ted_policy.py | 1 - 2 files changed, 2 deletions(-) diff --git a/rasa/core/policies/policy.py b/rasa/core/policies/policy.py index baf0c1546e2b..a029ffb8cea8 100644 --- a/rasa/core/policies/policy.py +++ b/rasa/core/policies/policy.py @@ -1,6 +1,5 @@ from __future__ import annotations import abc -import copy import logging from enum import Enum from pathlib import Path diff --git a/rasa/core/policies/ted_policy.py b/rasa/core/policies/ted_policy.py index 4f4703a73275..7d7cd8363107 100644 --- a/rasa/core/policies/ted_policy.py +++ b/rasa/core/policies/ted_policy.py @@ -24,7 +24,6 @@ TrackerFeaturizer, MaxHistoryTrackerFeaturizer, ) -from rasa.core.featurizers.single_state_featurizer import SingleStateFeaturizer from rasa.shared.exceptions import RasaException from rasa.shared.nlu.constants import ( ACTION_TEXT, From b63cfe70e21b9335d3beb10fc61684cdc5c05d78 Mon Sep 17 00:00:00 2001 From: Thomas Werkmeister Date: Mon, 30 Aug 2021 18:12:25 +0200 Subject: [PATCH 37/41] Removing name from config dict before instantiating feeaturizers --- rasa/core/policies/policy.py | 8 ++++++-- rasa/shared/utils/common.py | 7 ++++++- tests/shared/utils/test_common.py | 17 +++++++++++++++++ 3 files changed, 29 insertions(+), 3 deletions(-) diff --git a/rasa/core/policies/policy.py b/rasa/core/policies/policy.py index a029ffb8cea8..3ef145c855b6 100644 --- a/rasa/core/policies/policy.py +++ b/rasa/core/policies/policy.py @@ -173,10 +173,14 @@ def _create_featurizer(self) -> TrackerFeaturizer: state_featurizer_config = state_featurizer_configs[0] featurizer_config["state_featurizer"] = state_featurizer_func( - **state_featurizer_config + **rasa.shared.utils.common.without_keys( + state_featurizer_config, {"name"} + ) ) - featurizer = featurizer_func(**featurizer_config) + featurizer = featurizer_func( + **rasa.shared.utils.common.without_keys(featurizer_config, {"name"}) + ) if ( isinstance(featurizer, MaxHistoryTrackerFeaturizer) and POLICY_MAX_HISTORY in self.config diff --git a/rasa/shared/utils/common.py b/rasa/shared/utils/common.py index aafda03e92eb..d96e2085fe73 100644 --- a/rasa/shared/utils/common.py +++ b/rasa/shared/utils/common.py @@ -3,7 +3,7 @@ import importlib import inspect import logging -from typing import Text, Dict, Optional, Any, List, Callable, Collection, Type +from typing import Text, Dict, Optional, Any, List, Callable, Collection, Type, Set from rasa.shared.exceptions import RasaException @@ -70,6 +70,11 @@ def sort_list_of_dicts_by_first_key(dicts: List[Dict]) -> List[Dict]: return sorted(dicts, key=lambda d: list(d.keys())[0]) +def without_keys(original: Dict[Any, Any], excluded_keys: Set[Any]) -> Dict[Any, Any]: + """Returns thee original dictionary without excludedd keys.""" + return {k: original[k] for k in original if k not in excluded_keys} + + def lazy_property(function: Callable) -> Any: """Allows to avoid recomputing a property over and over. diff --git a/tests/shared/utils/test_common.py b/tests/shared/utils/test_common.py index 323a431bf17a..d59303bceace 100644 --- a/tests/shared/utils/test_common.py +++ b/tests/shared/utils/test_common.py @@ -179,3 +179,20 @@ def test_class_from_module_path_fails(): module_path = "rasa.shared.core.domain.logger" with pytest.raises(RasaException): rasa.shared.utils.common.class_from_module_path(module_path) + + +@pytest.mark.parametrize( + "dictionary,excluded_keys,expected_result", + [ + ({"a": 1, "b": 2}, {"b"}, {"a": 1}), + ({"a": 1, "b": 2}, {"a", "b"}, {}), + ({"a": 1, "b": 2}, {}, {"a": 1, "b": 2}), + ({"a": 1, "b": 2}, {"c"}, {"a": 1, "b": 2}), + ({"a": 1, "b": 2, "c": 3}, {"a", "c"}, {"b": 2}), + ], +) +def test_without_keys(dictionary, excluded_keys, expected_result) -> None: + assert ( + rasa.shared.utils.common.without_keys(dictionary, excluded_keys) + == expected_result + ) From f35e0ceecb5171be3be4f6bc6c4d945bcab9e510 Mon Sep 17 00:00:00 2001 From: Thomas Werkmeister Date: Mon, 30 Aug 2021 19:16:06 +0200 Subject: [PATCH 38/41] Revert "Removing name from config dict before instantiating feeaturizers" This reverts commit b63cfe70e21b9335d3beb10fc61684cdc5c05d78. --- rasa/core/policies/policy.py | 8 ++------ rasa/shared/utils/common.py | 7 +------ tests/shared/utils/test_common.py | 17 ----------------- 3 files changed, 3 insertions(+), 29 deletions(-) diff --git a/rasa/core/policies/policy.py b/rasa/core/policies/policy.py index 3ef145c855b6..a029ffb8cea8 100644 --- a/rasa/core/policies/policy.py +++ b/rasa/core/policies/policy.py @@ -173,14 +173,10 @@ def _create_featurizer(self) -> TrackerFeaturizer: state_featurizer_config = state_featurizer_configs[0] featurizer_config["state_featurizer"] = state_featurizer_func( - **rasa.shared.utils.common.without_keys( - state_featurizer_config, {"name"} - ) + **state_featurizer_config ) - featurizer = featurizer_func( - **rasa.shared.utils.common.without_keys(featurizer_config, {"name"}) - ) + featurizer = featurizer_func(**featurizer_config) if ( isinstance(featurizer, MaxHistoryTrackerFeaturizer) and POLICY_MAX_HISTORY in self.config diff --git a/rasa/shared/utils/common.py b/rasa/shared/utils/common.py index d96e2085fe73..aafda03e92eb 100644 --- a/rasa/shared/utils/common.py +++ b/rasa/shared/utils/common.py @@ -3,7 +3,7 @@ import importlib import inspect import logging -from typing import Text, Dict, Optional, Any, List, Callable, Collection, Type, Set +from typing import Text, Dict, Optional, Any, List, Callable, Collection, Type from rasa.shared.exceptions import RasaException @@ -70,11 +70,6 @@ def sort_list_of_dicts_by_first_key(dicts: List[Dict]) -> List[Dict]: return sorted(dicts, key=lambda d: list(d.keys())[0]) -def without_keys(original: Dict[Any, Any], excluded_keys: Set[Any]) -> Dict[Any, Any]: - """Returns thee original dictionary without excludedd keys.""" - return {k: original[k] for k in original if k not in excluded_keys} - - def lazy_property(function: Callable) -> Any: """Allows to avoid recomputing a property over and over. diff --git a/tests/shared/utils/test_common.py b/tests/shared/utils/test_common.py index d59303bceace..323a431bf17a 100644 --- a/tests/shared/utils/test_common.py +++ b/tests/shared/utils/test_common.py @@ -179,20 +179,3 @@ def test_class_from_module_path_fails(): module_path = "rasa.shared.core.domain.logger" with pytest.raises(RasaException): rasa.shared.utils.common.class_from_module_path(module_path) - - -@pytest.mark.parametrize( - "dictionary,excluded_keys,expected_result", - [ - ({"a": 1, "b": 2}, {"b"}, {"a": 1}), - ({"a": 1, "b": 2}, {"a", "b"}, {}), - ({"a": 1, "b": 2}, {}, {"a": 1, "b": 2}), - ({"a": 1, "b": 2}, {"c"}, {"a": 1, "b": 2}), - ({"a": 1, "b": 2, "c": 3}, {"a", "c"}, {"b": 2}), - ], -) -def test_without_keys(dictionary, excluded_keys, expected_result) -> None: - assert ( - rasa.shared.utils.common.without_keys(dictionary, excluded_keys) - == expected_result - ) From a2611080551e3ecf20b45866a6d2e99484eb4db3 Mon Sep 17 00:00:00 2001 From: Thomas Werkmeister Date: Mon, 30 Aug 2021 19:17:09 +0200 Subject: [PATCH 39/41] Revert "policy featurizer creation refactoring" This reverts commit 919827dd90b36badaff8ef3e992757518bf3d2d0. --- rasa/core/policies/policy.py | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/rasa/core/policies/policy.py b/rasa/core/policies/policy.py index a029ffb8cea8..46dbb6cf22dc 100644 --- a/rasa/core/policies/policy.py +++ b/rasa/core/policies/policy.py @@ -127,7 +127,7 @@ def __init__( """Constructs a new Policy object.""" self.config = config if featurizer is None: - featurizer = self._create_featurizer() + featurizer = self._create_featurizer(config) self.__featurizer = featurizer self.priority = config.get(POLICY_PRIORITY, DEFAULT_POLICY_PRIORITY) @@ -150,8 +150,10 @@ def create( """Creates a new untrained policy (see parent class for full docstring).""" return cls(config, model_storage, resource, execution_context) - def _create_featurizer(self) -> TrackerFeaturizer: - featurizer_configs = self.config.get("featurizer") + def _create_featurizer(self, policy_config: Dict[Text, Any]) -> TrackerFeaturizer: + policy_config = copy.deepcopy(policy_config) + + featurizer_configs = policy_config.get("featurizer") if not featurizer_configs: return self._standard_featurizer() @@ -163,7 +165,7 @@ def _create_featurizer(self) -> TrackerFeaturizer: ) featurizer_config = featurizer_configs[0] - state_featurizer_configs = featurizer_config.get("state_featurizer", None) + state_featurizer_configs = featurizer_config.pop("state_featurizer", None) if state_featurizer_configs: state_featurizer_func = _get_featurizer_from_config( state_featurizer_configs, @@ -179,10 +181,10 @@ def _create_featurizer(self) -> TrackerFeaturizer: featurizer = featurizer_func(**featurizer_config) if ( isinstance(featurizer, MaxHistoryTrackerFeaturizer) - and POLICY_MAX_HISTORY in self.config + and POLICY_MAX_HISTORY in policy_config and POLICY_MAX_HISTORY not in featurizer_config ): - featurizer.max_history = self.config[POLICY_MAX_HISTORY] + featurizer.max_history = policy_config[POLICY_MAX_HISTORY] return featurizer def _standard_featurizer(self) -> MaxHistoryTrackerFeaturizer: @@ -652,7 +654,7 @@ def _get_featurizer_from_config( ) featurizer_config = config[0] - featurizer_name = featurizer_config["name"] + featurizer_name = featurizer_config.pop("name") featurizer_func = rasa.shared.utils.common.class_from_module_path( featurizer_name, lookup_path=lookup_path ) From be57395a96d091113d41c5817525f8b9d934cd94 Mon Sep 17 00:00:00 2001 From: Thomas Werkmeister Date: Mon, 30 Aug 2021 19:19:01 +0200 Subject: [PATCH 40/41] made create featurizer use copy of policy again --- rasa/core/policies/policy.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/rasa/core/policies/policy.py b/rasa/core/policies/policy.py index 46dbb6cf22dc..5c442c381f3a 100644 --- a/rasa/core/policies/policy.py +++ b/rasa/core/policies/policy.py @@ -1,5 +1,6 @@ from __future__ import annotations import abc +import copy import logging from enum import Enum from pathlib import Path @@ -150,8 +151,8 @@ def create( """Creates a new untrained policy (see parent class for full docstring).""" return cls(config, model_storage, resource, execution_context) - def _create_featurizer(self, policy_config: Dict[Text, Any]) -> TrackerFeaturizer: - policy_config = copy.deepcopy(policy_config) + def _create_featurizer(self) -> TrackerFeaturizer: + policy_config = copy.deepcopy(self.config) featurizer_configs = policy_config.get("featurizer") From 9e2210d583985044207a54b068cafdddc6880e2d Mon Sep 17 00:00:00 2001 From: Thomas Werkmeister Date: Mon, 30 Aug 2021 20:22:11 +0200 Subject: [PATCH 41/41] removed surplus argument --- rasa/core/policies/policy.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rasa/core/policies/policy.py b/rasa/core/policies/policy.py index 5c442c381f3a..aa16fbb924fa 100644 --- a/rasa/core/policies/policy.py +++ b/rasa/core/policies/policy.py @@ -128,7 +128,7 @@ def __init__( """Constructs a new Policy object.""" self.config = config if featurizer is None: - featurizer = self._create_featurizer(config) + featurizer = self._create_featurizer() self.__featurizer = featurizer self.priority = config.get(POLICY_PRIORITY, DEFAULT_POLICY_PRIORITY)