Merge 1.10.x into master (RasaHQ#5823)

chkoss · tabergma · indam23 · web-flow · commit 6e23dd3b5dca · 2020-05-18T15:37:39.000+02:00
* move set_up_tensorboard_writer to fit

* don't log during loading

* add changelog entry

* check if spacy features are empty

* add changelog

* add test

* update sed command

* add log message

* fix tests

* check if vectors are present

* add separate laoding parameter

* fix test

* fix :ref:s and update docs on form deactivation in validation function

* Fix sed command

* log request URL on exception

* use concat_url for duckling url

* changelog entry

* Add missing quote

* use clean up custom entities

* add changelog

* Don't add empty roles/groups

* add changelog

* import module instead of method

* make sure 'collect' functions don't alter their inputs

* add changelog entry

* add test

* include review feedback

Co-authored-by: Tanja Bergmann &lt;tabergma@gmail.com&gt;
Co-authored-by: melindaloubser1 &lt;melinda.loubser@gmail.com&gt;
Co-authored-by: Melinda Loubser &lt;32034278+melindaloubser1@users.noreply.github.com&gt;
Co-authored-by: Federico Tedin &lt;federicotedin@gmail.com&gt;
Co-authored-by: Ella &lt;erohmensing@gmail.com&gt;
Co-authored-by: Federico Tedin &lt;federicotdn@users.noreply.github.com&gt;
Co-authored-by: Tom Bocklisch &lt;tom@rasa.com&gt;
Co-authored-by: Roberto &lt;43567378+rasabot@users.noreply.github.com&gt;
diff --git a/CHANGELOG.rst b/CHANGELOG.rst
@@ -39,7 +39,7 @@ Features
 
   .. note::
 
-      Composite entities are currently just supported by the :ref:``diet-classifier`` and :ref:``CRFEntityExtractor``.
+      Composite entities are currently just supported by the :ref:`diet-classifier` and :ref:`CRFEntityExtractor`.
 - `#5465 <https://github.com/rasahq/rasa/issues/5465>`_: Update training data format for NLU to support entities with a role or group label.
 
   You can now specify synonyms, roles, and groups of entities using the following data format:
@@ -66,7 +66,7 @@ Features
 
   The markdown format ``[LA](location:Los Angeles)`` is deprecated. To update your training data file just
   execute the following command on the terminal of your choice:
-  ``sed -i .deprecated -E 's/\[(.*)\]\((.*):(.*)\)/\[\1\]\{"entity": "\2", "value": "\3"\}/g' nlu.md``
+  ``sed -i -E 's/\[([^)]+)\]\(([^)]+):([^)]+)\)/[\1]{"entity": "\2", "value": "\3"}/g' nlu.md``
 
   For more information about the new data format see :ref:`training-data-format`.
 
diff --git a/changelog/5617.bugfix.rst b/changelog/5617.bugfix.rst
@@ -0,0 +1 @@
+Don't create TensorBoard log files during prediction.
diff --git a/changelog/5638.bugfix.rst b/changelog/5638.bugfix.rst
@@ -0,0 +1 @@
+Fix: DIET breaks with empty spaCy model
diff --git a/changelog/5755.bugfix.rst b/changelog/5755.bugfix.rst
@@ -0,0 +1,2 @@
+Remove ``clean_up_entities`` from extractors that extract pre-defined entities.
+Just keep the clean up method for entity extractors that extract custom entities.
diff --git a/changelog/5788.misc.rst b/changelog/5788.misc.rst
@@ -0,0 +1 @@
+Don't set roles and groups to ``O`` (nothing found) when constructing entities from model predictions.
diff --git a/changelog/5792.bugfix.rst b/changelog/5792.bugfix.rst
@@ -0,0 +1,2 @@
+Fixed issue where the ``DucklingHTTPExtractor`` component would
+not work if its `url` contained a trailing slash.
diff --git a/changelog/5794.improvement.rst b/changelog/5794.improvement.rst
@@ -0,0 +1,3 @@
+Creating a ``Domain`` using ``Domain.fromDict`` can no longer alter the input dictionary.
+Previously, there could be problems when the input dictionary was re-used for other
+things after creating the ``Domain`` from it.
diff --git a/docs/core/forms.rst b/docs/core/forms.rst
@@ -191,9 +191,38 @@ to set, you can set more slots than just the one you are validating from inside
 a helper validation method. However, you are responsible for making sure that
 those extra slot values are valid.
 
-You can also deactivate the form directly during this validation step (in case the
-slot is filled with something that you are certain can't be handled) by returning
-``self.deactivate()``
+In case the slot is filled with something that you are certain can't be handled
+and you want to deactivate the form directly,
+you can overwrite the ``request_next_slot()`` method to do so. The example below
+checks the value of the ``cuisine`` slot directly, but you could use any logic
+you'd like to trigger deactivation:
+
+.. code-block:: python
+
+    def request_next_slot(
+        self,
+        dispatcher: "CollectingDispatcher",
+        tracker: "Tracker",
+        domain: Dict[Text, Any],
+    ) -> Optional[List[EventType]]:
+        """Request the next slot and utter template if needed,
+            else return None"""
+        for slot in self.required_slots(tracker):
+            if self._should_request_slot(tracker, slot):
+
+                ## Condition of validated slot that triggers deactivation
+                if slot == "cuisine" and tracker.get_slot("cuisine") == "caribbean":
+                    dispatcher.utter_message(text="Sorry, I can't help you with that")
+                    return self.deactivate()
+                
+                ## For all other slots, continue as usual
+                logger.debug(f"Request next slot '{slot}'")
+                dispatcher.utter_message(
+                    template=f"utter_ask_{slot}", **tracker.slots
+                )
+                return [SlotSet(REQUESTED_SLOT, slot)]
+        return None
+
 
 If nothing is extracted from the user's utterance for any of the required slots, an
 ``ActionExecutionRejection`` error will be raised, meaning the action execution
diff --git a/docs/migration-guide.rst b/docs/migration-guide.rst
@@ -137,9 +137,9 @@ General
       # ... any other parameters
 
   ``CRFEntityExtractor`` featurizes user messages on its own, it does not depend on any featurizer.
-  We extracted the featurization from the component into the new featurizer :ref:``LexicalSyntacticFeaturizer``. Thus,
+  We extracted the featurization from the component into the new featurizer :ref:`LexicalSyntacticFeaturizer`. Thus,
   in order to obtain the same results as before, you need to add this featurizer to your pipeline before the
-  :ref:``diet-classifier``.
+  :ref:`diet-classifier`.
   Specifying ``CRFEntityExtractor`` in the configuration maps to the above component definition, the behaviour
   is unchanged from previous versions.
 
diff --git a/docs/nlu/entity-extraction.rst b/docs/nlu/entity-extraction.rst
@@ -173,7 +173,7 @@ The entity object returned by the extractor will include the detected role/group
 
 .. note::
 
-    Composite entities are currently only supported by the :ref:``diet-classifier`` and :ref:``CRFEntityExtractor``.
+    Composite entities are currently only supported by the :ref:`diet-classifier` and :ref:`CRFEntityExtractor`.
 
 In order to properly train your model with entities that have roles/groups, make sure to include enough training data
 examples for every combination of entity and role/group label.
diff --git a/docs/nlu/training-data-format.rst b/docs/nlu/training-data-format.rst
@@ -82,12 +82,12 @@ case-insensitive regex patterns that are added to the regex features.
 
 .. warning::
     The synonym format to specify synonyms ``[savings account](source_account:savings)`` is deprecated. Please use the
-    new format ``[savings account]{"entity": "source_account", "value": "savings"}``. To update your training data
-    file execute the following command on the terminal of your choice:
-    ``sed -i .deprecated -E 's/\[(.+)\]\((.+):(.+)\)/\[\1\]\{"entity": "\2", "value": "\3"\}/g' <nlu training data file>``
-    Assume you training data file is called ``nlu.md``. After you executed the above command, this file will
-    contain the new training data format. The command also creates a backup file with the old training data format,
-    called ``nlu.md.deprecated``.
+    new format ``[savings account]{"entity": "source_account", "value": "savings"}``.
+
+    To update your training data file execute the following command on the terminal of your choice:
+    ``sed -i -E 's/\[([^)]+)\]\(([^)]+):([^)]+)\)/[\1]{"entity": "\2", "value": "\3"}/g' <nlu training data file>``
+    Your NLU training data file will contain the new training data format after you executed the above command.
+    Depending on your OS you might need to update the syntax of the sed command.
 
 JSON Format
 -----------
diff --git a/docs/user-guide/setting-up-ci-cd.rst b/docs/user-guide/setting-up-ci-cd.rst
@@ -212,4 +212,4 @@ the Rasa assistant that you can talk to in the Rasa Docs, and
 Both use `Github Actions <https://github.com/features/actions>`_ as a CI/CD tool.
 
 These examples are just two of many possibilities. If you have a CI/CD setup you like, please
-share it with the Rasa community on the `Rasa Forum <https://forum.rasa.com/>`_.
+share it with the Rasa community on the `forum <https://forum.rasa.com>`_.
diff --git a/rasa/core/domain.py b/rasa/core/domain.py
@@ -259,6 +259,8 @@ def collect_slots(slot_dict: Dict[Text, Any]) -> List[Slot]:
         # it is super important to sort the slots here!!!
         # otherwise state ordering is not consistent
         slots = []
+        # make a copy to not alter the input dictionary
+        slot_dict = copy.deepcopy(slot_dict)
         for slot_name in sorted(slot_dict):
             slot_class = Slot.resolve_by_type(slot_dict[slot_name].get("type"))
             if "type" in slot_dict[slot_name]:
@@ -332,6 +334,8 @@ def collect_intent_properties(
         Returns:
             The intent properties to be stored in the domain.
         """
+        # make a copy to not alter the input argument
+        intents = copy.deepcopy(intents)
         intent_properties = {}
         duplicates = set()
         for intent in intents:
diff --git a/rasa/nlu/extractors/duckling_http_extractor.py b/rasa/nlu/extractors/duckling_http_extractor.py
@@ -5,6 +5,7 @@
 import requests
 from typing import Any, List, Optional, Text, Dict
 
+import rasa.utils.endpoints as endpoints_utils
 from rasa.constants import DOCS_URL_COMPONENTS
 from rasa.nlu.constants import ENTITIES
 from rasa.nlu.config import RasaNLUModelConfig
@@ -113,15 +114,23 @@ def _payload(self, text: Text, reference_time: int) -> Dict[Text, Any]:
         }
 
     def _duckling_parse(self, text: Text, reference_time: int) -> List[Dict[Text, Any]]:
-        """Sends the request to the duckling server and parses the result."""
+        """Sends the request to the duckling server and parses the result.
 
+        Args:
+            text: Text for duckling server to parse.
+            reference_time: Reference time in milliseconds.
+
+        Returns:
+            JSON response from duckling server with parse data.
+        """
+        parse_url = endpoints_utils.concat_url(self._url(), "/parse")
         try:
             payload = self._payload(text, reference_time)
             headers = {
                 "Content-Type": "application/x-www-form-urlencoded; charset=UTF-8"
             }
             response = requests.post(
-                self._url() + "/parse",
+                parse_url,
                 data=payload,
                 headers=headers,
                 timeout=self.component_config.get("timeout"),
@@ -130,9 +139,8 @@ def _duckling_parse(self, text: Text, reference_time: int) -> List[Dict[Text, An
                 return response.json()
             else:
                 logger.error(
-                    "Failed to get a proper response from remote "
-                    "duckling. Status Code: {}. Response: {}"
-                    "".format(response.status_code, response.text)
+                    f"Failed to get a proper response from remote "
+                    f"duckling at '{parse_url}. Status Code: {response.status_code}. Response: {response.text}"
                 )
                 return []
         except (
diff --git a/rasa/nlu/extractors/extractor.py b/rasa/nlu/extractors/extractor.py
@@ -248,13 +248,13 @@ def _create_new_entity(
                 ENTITY_ATTRIBUTE_TYPE
             ][idx]
 
-        if ENTITY_ATTRIBUTE_ROLE in tag_names:
+        if ENTITY_ATTRIBUTE_ROLE in tag_names and role_tag != NO_ENTITY_TAG:
             entity[ENTITY_ATTRIBUTE_ROLE] = role_tag
             if confidences is not None:
                 entity[ENTITY_ATTRIBUTE_CONFIDENCE_ROLE] = confidences[
                     ENTITY_ATTRIBUTE_ROLE
                 ][idx]
-        if ENTITY_ATTRIBUTE_GROUP in tag_names:
+        if ENTITY_ATTRIBUTE_GROUP in tag_names and group_tag != NO_ENTITY_TAG:
             entity[ENTITY_ATTRIBUTE_GROUP] = group_tag
             if confidences is not None:
                 entity[ENTITY_ATTRIBUTE_CONFIDENCE_GROUP] = confidences[
diff --git a/rasa/nlu/featurizers/dense_featurizer/spacy_featurizer.py b/rasa/nlu/featurizers/dense_featurizer/spacy_featurizer.py
@@ -1,5 +1,6 @@
 import numpy as np
 import typing
+import logging
 from typing import Any, Optional, Text, Dict, List, Type
 
 from rasa.nlu.config import RasaNLUModelConfig
@@ -20,6 +21,9 @@
     from spacy.tokens import Doc
 
 
+logger = logging.getLogger(__name__)
+
+
 class SpacyFeaturizer(DenseFeaturizer):
     @classmethod
     def required_components(cls) -> List[Type[Component]]:
@@ -52,25 +56,29 @@ def train(
                 self._set_spacy_features(example, attribute)
 
     def get_doc(self, message: Message, attribute: Text) -> Any:
-
         return message.get(SPACY_DOCS[attribute])
 
     def process(self, message: Message, **kwargs: Any) -> None:
-
         self._set_spacy_features(message)
 
-    def _set_spacy_features(self, message: Message, attribute: Text = TEXT):
+    def _set_spacy_features(self, message: Message, attribute: Text = TEXT) -> None:
         """Adds the spacy word vectors to the messages features."""
+        doc = self.get_doc(message, attribute)
+
+        if doc is None:
+            return
 
-        message_attribute_doc = self.get_doc(message, attribute)
+        # in case an empty spaCy model was used, no vectors are present
+        if doc.vocab.vectors_length == 0:
+            logger.debug("No features present. You are using an empty spaCy model.")
+            return
 
-        if message_attribute_doc is not None:
-            features = self._features_for_doc(message_attribute_doc)
+        features = self._features_for_doc(doc)
 
-            cls_token_vec = self._calculate_cls_vector(features, self.pooling_operation)
-            features = np.concatenate([features, cls_token_vec])
+        cls_token_vec = self._calculate_cls_vector(features, self.pooling_operation)
+        features = np.concatenate([features, cls_token_vec])
 
-            features = self._combine_with_existing_dense_features(
-                message, features, DENSE_FEATURE_NAMES[attribute]
-            )
-            message.set(DENSE_FEATURE_NAMES[attribute], features)
+        features = self._combine_with_existing_dense_features(
+            message, features, DENSE_FEATURE_NAMES[attribute]
+        )
+        message.set(DENSE_FEATURE_NAMES[attribute], features)
diff --git a/rasa/nlu/training_data/formats/markdown.py b/rasa/nlu/training_data/formats/markdown.py
@@ -109,8 +109,8 @@ def reads(self, s: Text, **kwargs: Any) -> "TrainingData":
                 '[<entity-text>]{"entity": "<entity-type>", "value": '
                 '"<entity-synonym>"}.'
                 "\nYou can use the following command to update your training data file:"
-                "\nsed -i .deprecated -E 's/\\[(.+)\\]\\((.+):(.+)\\)/\\[\\1\\]\\{"
-                '"entity": "\\2", "value": "\\3"\\}/g\' nlu.md',
+                "\nsed -i -E 's/\\[([^)]+)\\]\\(([^)]+):([^)]+)\\)/[\\1]{"
+                '"entity": "\\2", "value": "\\3"}/g\' nlu.md',
                 category=FutureWarning,
                 docs=DOCS_URL_TRAINING_DATA_NLU,
             )
diff --git a/rasa/utils/endpoints.py b/rasa/utils/endpoints.py
@@ -1,11 +1,9 @@
+import aiohttp
 import logging
 import os
-
-import aiohttp
 from aiohttp.client_exceptions import ContentTypeError
-from typing import Any, Optional, Text, Dict
-
 from sanic.request import Request
+from typing import Any, Optional, Text, Dict
 
 import rasa.utils.io
 from rasa.constants import DEFAULT_REQUEST_TIMEOUT
@@ -44,14 +42,20 @@ def concat_url(base: Text, subpath: Optional[Text]) -> Text:
     Strips leading slashes from the subpath if necessary. This behaves
     differently than `urlparse.urljoin` and will not treat the subpath
     as a base url if it starts with `/` but will always append it to the
-    `base`."""
+    `base`.
+
+    Args:
+        base: Base URL.
+        subpath: Optional path to append to the base URL.
 
+    Returns:
+        Concatenated URL with base and subpath.
+    """
     if not subpath:
         if base.endswith("/"):
             logger.debug(
-                "The URL '{}' has a trailing slash. Please make sure the "
-                "target server supports trailing slashes for this "
-                "endpoint.".format(base)
+                f"The URL '{base}' has a trailing slash. Please make sure the "
+                f"target server supports trailing slashes for this endpoint."
             )
         return base
 
diff --git a/rasa/utils/tensorflow/models.py b/rasa/utils/tensorflow/models.py
@@ -48,37 +48,37 @@ def __init__(
 
         self.random_seed = random_seed
 
+        self.tensorboard_log_dir = tensorboard_log_dir
+        self.tensorboard_log_level = tensorboard_log_level
+
         self.train_summary_writer = None
         self.test_summary_writer = None
         self.model_summary_file = None
         self.tensorboard_log_on_epochs = True
 
-        self._set_up_tensorboard_writer(tensorboard_log_level, tensorboard_log_dir)
-
-    def _set_up_tensorboard_writer(
-        self, tensorboard_log_level: Text, tensorboard_log_dir: Optional[Text] = None
-    ) -> None:
-        if tensorboard_log_dir is not None:
-            if tensorboard_log_level not in TENSORBOARD_LOG_LEVELS:
+    def _set_up_tensorboard_writer(self) -> None:
+        if self.tensorboard_log_dir is not None:
+            if self.tensorboard_log_level not in TENSORBOARD_LOG_LEVELS:
                 raise ValueError(
-                    f"Provided '{TENSORBOARD_LOG_LEVEL}' ('{tensorboard_log_level}') "
+                    f"Provided '{TENSORBOARD_LOG_LEVEL}' ('{self.tensorboard_log_level}') "
                     f"is invalid! Valid values are: {TENSORBOARD_LOG_LEVELS}"
                 )
-
-            self.tensorboard_log_on_epochs = tensorboard_log_level == "epoch"
+            self.tensorboard_log_on_epochs = self.tensorboard_log_level == "epoch"
 
             current_time = datetime.datetime.now().strftime("%Y%m%d-%H%M%S")
             class_name = self.__class__.__name__
 
-            train_log_dir = f"{tensorboard_log_dir}/{class_name}/{current_time}/train"
-            test_log_dir = f"{tensorboard_log_dir}/{class_name}/{current_time}/test"
+            train_log_dir = (
+                f"{self.tensorboard_log_dir}/{class_name}/{current_time}/train"
+            )
+            test_log_dir = (
+                f"{self.tensorboard_log_dir}/{class_name}/{current_time}/test"
+            )
 
             self.train_summary_writer = tf.summary.create_file_writer(train_log_dir)
             self.test_summary_writer = tf.summary.create_file_writer(test_log_dir)
 
-            self.model_summary_file = (
-                f"{tensorboard_log_dir}/{class_name}/{current_time}/model_summary.txt"
-            )
+            self.model_summary_file = f"{self.tensorboard_log_dir}/{class_name}/{current_time}/model_summary.txt"
 
     def batch_loss(
         self, batch_in: Union[Tuple[tf.Tensor], Tuple[np.ndarray]]
@@ -99,10 +99,15 @@ def fit(
         evaluate_every_num_epochs: int,
         batch_strategy: Text,
         silent: bool = False,
+        loading: bool = False,
         eager: bool = False,
     ) -> None:
         """Fit model data"""
 
+        # don't setup tensorboard writers when training during loading
+        if not loading:
+            self._set_up_tensorboard_writer()
+
         tf.random.set_seed(self.random_seed)
         np.random.seed(self.random_seed)
 
@@ -257,6 +262,7 @@ def load(
             evaluate_on_num_examples=0,
             batch_strategy=SEQUENCE,
             silent=True,  # don't confuse users with training output
+            loading=True,  # don't use tensorboard while loading
             eager=True,  # no need to build tf graph, eager is faster here
         )
         # load trained weights
diff --git a/tests/core/test_domain.py b/tests/core/test_domain.py
diff --git a/tests/nlu/extractors/test_extractor.py b/tests/nlu/extractors/test_extractor.py
diff --git a/tests/nlu/featurizers/test_spacy_featurizer.py b/tests/nlu/featurizers/test_spacy_featurizer.py
diff --git a/tests/utils/test_endpoints.py b/tests/utils/test_endpoints.py

Original file line number	Diff line number	Diff line change
`@@ -0,0 +1 @@`
	`1`	`+Don't create TensorBoard log files during prediction.`
Original file line number	Diff line number	Diff line change
`@@ -0,0 +1 @@`
	`1`	`+Fix: DIET breaks with empty spaCy model`
Original file line number	Diff line number	Diff line change
`@@ -0,0 +1,2 @@`
	`1`	+Remove ``clean_up_entities`` from extractors that extract pre-defined entities.
	`2`	`+Just keep the clean up method for entity extractors that extract custom entities.`
Original file line number	Diff line number	Diff line change
`@@ -0,0 +1 @@`
	`1`	+Don't set roles and groups to ``O`` (nothing found) when constructing entities from model predictions.
Original file line number	Diff line number	Diff line change
`@@ -0,0 +1,2 @@`
	`1`	+Fixed issue where the ``DucklingHTTPExtractor`` component would
	`2`	+not work if its `url` contained a trailing slash.
Original file line number	Diff line number	Diff line change
`@@ -0,0 +1,3 @@`
	`1`	+Creating a ``Domain`` using ``Domain.fromDict`` can no longer alter the input dictionary.
	`2`	`+Previously, there could be problems when the input dictionary was re-used for other`
	`3`	+things after creating the ``Domain`` from it.