Response generator responses with multimedia elements #6323

tmbo · 2020-08-03T11:59:50Z

Proposed changes:

added yaml support for responses. example of a responses.yml with the format as it is currently implemented:

responses:
  chitchat/ask_weather:
  - text: Where do you want to check the weather?
    buttons:
    - title: Current location
      payload: here
    - title: Other place
      payload: other_location

  chitchat/ask_name:
  - text: my name is Sara, Rasa's documentation bot!
    image: "https://i.imgur.com/nGF1K8f.jpg"

responses model only gets trained on the text part of the first response.
allows users to use multimedia elements (images, buttons, ...) in response templates
the format (and the parser) is the same as we use for utter templates in the domain
this includes the changes to port the moodbot demo to the new file format (needed an example...)

Status (please check what you already did):

handle case where the response.text is empty -> use the intent name instead
added some tests for the functionality
updated the documentation
updated the changelog (please check changelog for instructions)
reformat files using black (please check Readme for instructions)

extended response format adds the ability to add images, buttons, ... to responses generated from the response selector. the format is the same as we use for utter templates in the domain.

tmbo · 2020-08-03T12:07:01Z

@dakshvar22 are there any concerns regarding the data format?

tmbo · 2020-08-03T12:18:46Z

rasa/core/training/story_reader/markdown_story_reader.py

@@ -233,7 +233,7 @@ def is_markdown_story_file(file_path: Text) -> bool:
        """
        suffix = PurePath(file_path).suffix

-        if suffix and suffix != MARKDOWN_FILE_EXTENSION:
+        if suffix not in MARKDOWN_FILE_EXTENSIONS:


I think we should discuss if we keep the original behaviour: I changed it to keep it uniform across readers. The difference between the versions is that the original one would happily read files without an extension as story files, which seems arbitrary. Nevertheless, it is breaking backwards compatibility in an odd way, so kind of betting that no one relies on this odd behaviour 🤔

Should we add this to the changelog? Training data form in markdown format has to have the file suffix .md from now on`?

tmbo · 2020-08-03T12:21:44Z

rasa/nlu/classifiers/diet_classifier.py

@@ -1098,7 +1104,7 @@ def _load_model(
            data_signature=model_data_example.get_signature(),
            label_data=label_data,
            entity_tag_specs=entity_tag_specs,
-            config=meta,
+            config=copy.deepcopy(meta),


this fix was needed as this config gets passed to tensorflow which in place convert all values of the dictionary to tensorflow types (e.g. it will replace an ordinary python string with a tensorflow specific string). to avoid tf modifying this original dictionary we need to pass in a copy...

dakshvar22 · 2020-08-03T12:40:04Z

@tmbo Data format looks good. Just two comments(maybe you have thought of these already):

In contrast to usual response templates, there cannot be multiple text attributes for one retrieval intent. For example, this would be invalid:

responses:
  chitchat/ask_weather:
  - text: Where do you want to check the weather?
    buttons:
    - title: Current location
      payload: here
    - title: Other place
      payload: other_location
 - text: Do you want to know the weather for today?
    buttons:
    - title: Today
      payload: here
    - title: Whole week
      payload: other_location

This is primarily because the response selector can only be trained with one ground truth label. Supporting multiple responses should be left out of scope for now IMO.

I am not sure of this but the text attribute of any response template is optional, right? For example, is this valid? -

responses:
  chitchat/ask_weather:
  - buttons:
    - title: Current location
      payload: here
    - title: Other place
      payload: other_location

If it is valid, then we should use the full name of the retrieval intent(for e.g. - chitchat/ask_weather) as the proxy for the text attribute(just for model training)

tmbo · 2020-08-03T12:45:40Z

In contrast to usual response templates, there cannot be multiple text attributes for one retrieval intent. For example, this would be invalid:

just because the ML model can't be trained with multiple of these, it doesn't mean this needs to be invalid. e.g. we could do the same as we do for utterance templates and just select a random one.

tmbo · 2020-08-03T12:47:05Z

I am not sure of this but the text attribute of any response template is optional, right? For example, is this valid? -

Yes I think that is valid. I like the solution, let's use the intent name in that case

dakshvar22 · 2020-08-03T12:54:25Z

just because the ML model can't be trained with multiple of these, it doesn't mean this needs to be invalid. e.g. we could do the same as we do for utterance templates and just select a random one.

Yes, we can do that but which one would the model be trained on? It has to be just one and taking a random one for training, doesn't seem like a good option. If we always take the first response as the training response would it introduce any confusion/problems in the UX?(I am not very clear on this myself) For example, model would need to be retrained if the first response was swapped with the last.

tmbo · 2020-08-03T13:35:00Z

well, we can train the model on either the first, all of them or any of them: whatever you prefer.

I think from a UX perspective it is easier to explain that the format is exactly the same as the one for utterance templates. The model gets retrained on any change to the responses at the moment, I think from the training perspective it is pretty consistent.

…esponses-with-extras

rasa/nlu/training_data/training_data.py

wochinge

first part of the review. Looking good so far 💯

examples/moodbot/data/rules.yml

changelog/6323.improvement.md

tests/core/test_exporter.py

rasa/nlu/training_data/training_data.py

tests/nlu/training_data/test_training_data.py

tests/nlu/selectors/test_selectors.py

wochinge

nearly done now. Had to submit due to outdated diff

rasa/core/actions/action.py

rasa/nlu/schemas/nlu.yml

rasa/nlu/training_data/formats/markdown_nlg.py

rasa/nlu/training_data/formats/rasa_yaml.py

wochinge · 2020-08-14T13:06:37Z

tests/test_server.py

-        domain_file = stack.enter_context(open(default_domain_path))
-        config_file = stack.enter_context(open(default_stack_config))
-        nlu_file = stack.enter_context(open(default_nlu_data))
+    domain_data = rasa_utils.io.read_yaml_file(default_domain_path)


How about using parametrize or adding another test to make sure it still works with markdown?

I took a closer look and there are already a couple of other methods in there that test the same endpoint with markdown files. since this actually trains a model and takes time, I'd rather avoid adding more training runs. what do you think?

Yes, but then I'd maybe rename the tests to make it clear that this testing the MD support. Otherwise somebody might come, adapt the test, and suddenly all MD testing is gone.

Another quick way would be to extract the payload extraction to a separate function and test that using parametrize.

rasa/nlu/training_data/training_data.py

rasa/nlu/selectors/response_selector.py

rasa/nlu/classifiers/diet_classifier.py

wochinge

What a PR - great work! 👍

tests/nlu/training_data/formats/test_rasa_yaml.py

docs/docs/chitchat-faqs.mdx

wochinge · 2020-08-14T13:34:58Z

rasa/core/training/story_reader/markdown_story_reader.py

@@ -233,7 +233,7 @@ def is_markdown_story_file(file_path: Text) -> bool:
        """
        suffix = PurePath(file_path).suffix

-        if suffix and suffix != MARKDOWN_FILE_EXTENSION:
+        if suffix not in MARKDOWN_FILE_EXTENSIONS:


Should we add this to the changelog? Training data form in markdown format has to have the file suffix .md from now on`?

tmbo · 2020-08-14T13:54:03Z

thanks a lot @wochinge fur the super quick and thorough review 🚀

wochinge · 2020-08-14T15:30:08Z

@tmbo During the review I noticed the close coupling of training data objects (Domain, TrainingData) with the different Writer classes.

What do think about having a pendant to TrainingDataImporter to

support other file format implementations (e.g. implementations of the community, adapters to other frameworks)
clear separation of concerns / classes don't do multiple things at the same time
easier testability
DRYer (each class is doing some sort of if endswith "json" elif "endswith md" currently)

…esponses-with-extras

tmbo · 2020-08-17T07:39:53Z

yes @wochinge I think that is a good idea 👍

* Use rules for greet, goodbye & challenge * Convert nlu & stories to yml * Add '-' in front of examples * Add 'y' , 'n' to affirm & deny intents * Remove 'greet -> utter_greet' rule, and use in stories instead * implement yaml response format as well as extended response format extended response format adds the ability to add images, buttons, ... to responses generated from the response selector. the format is the same as we use for utter templates in the domain. * code style improvement * fixed linter error * fixed some more test and renamed nlg_stories to responses * fixed typing issue * fixed more types * fixed tests * Update training_data.py * fixed import errror * fixed remaining tests * fixed name error * Update rules.yml * added tests for responses * added changelog entry * updated documentation * applied review suggestions * integrated review comments * fixed typing issue Co-authored-by: Arjaan Buijk <[email protected]> Co-authored-by: Arjaan Buijk <[email protected]> Co-authored-by: Roberto <[email protected]>

ArjaanBuijk and others added 9 commits July 16, 2020 16:47

Use rules for greet, goodbye & challenge

991e962

Convert nlu & stories to yml

ed8b40b

Add '-' in front of examples

393a9b1

Add 'y' , 'n' to affirm & deny intents

349c831

Remove 'greet -> utter_greet' rule, and use in stories instead

5f7f12b

Merge branch 'master' into moodbot-w-rules

63e0957

Merge branch 'master' into moodbot-w-rules

85d7026

Merge branch 'master' into moodbot-w-rules

189dcd5

implement yaml response format as well as extended response format

204518c

extended response format adds the ability to add images, buttons, ... to responses generated from the response selector. the format is the same as we use for utter templates in the domain.

tmbo mentioned this pull request Aug 3, 2020

moodbot with rules & yml files #6229

Closed

tmbo commented Aug 3, 2020

View reviewed changes

tmbo added 7 commits August 3, 2020 15:41

code style improvement

b994089

Merge branch 'master' into responses-with-extras

a2a93aa

fixed linter error

59f9032

Merge branch 'responses-with-extras' of github.com:RasaHQ/rasa into r…

db5b79b

…esponses-with-extras

fixed some more test and renamed nlg_stories to responses

7d8eded

fixed typing issue

067a800

fixed more types

a9de073

tmbo added this to the 2.0a2 Rasa Open Source milestone Aug 10, 2020

merged master

6f3037f

tmbo mentioned this pull request Aug 12, 2020

Use labels for response selector training #6392

Closed

4 tasks

tmbo added 2 commits August 12, 2020 18:08

merged master

66cce74

merged master

a3e10ce

dakshvar22 reviewed Aug 14, 2020

View reviewed changes

rasa/nlu/training_data/training_data.py Outdated Show resolved Hide resolved

updated documentation

baab400

wochinge reviewed Aug 14, 2020

View reviewed changes

tmbo added 2 commits August 14, 2020 14:45

Merge branch 'master' into responses-with-extras

4ab9fdc

applied review suggestions

3867bcc

wochinge reviewed Aug 14, 2020

View reviewed changes

wochinge approved these changes Aug 14, 2020

View reviewed changes

tmbo added 2 commits August 14, 2020 15:56

integrated review comments

4e2af9f

Merge branch 'master' into responses-with-extras

8758d16

tmbo added the status:ready-to-merge label Aug 14, 2020

rasabot added 2 commits August 14, 2020 16:46

Merge branch 'master' into responses-with-extras

8c8de57

Merge branch 'master' into responses-with-extras

979d9da

rasabot and others added 10 commits August 14, 2020 17:32

Merge branch 'master' into responses-with-extras

9ced840

Merge branch 'master' into responses-with-extras

0749ee7

Merge branch 'master' into responses-with-extras

80b2c53

Merge branch 'master' into responses-with-extras

2b731c3

Merge branch 'master' into responses-with-extras

8bc3ac7

Merge branch 'master' into responses-with-extras

c7efb8a

Merge branch 'master' into responses-with-extras

2d74746

Merge branch 'master' into responses-with-extras

5e9cbf4

fixed typing issue

2e3b126

Merge branch 'responses-with-extras' of github.com:RasaHQ/rasa into r…

62c699e

…esponses-with-extras

tmbo merged commit da11cf9 into master Aug 17, 2020

tmbo deleted the responses-with-extras branch August 17, 2020 08:04

wochinge mentioned this pull request Aug 17, 2020

TrainingDataExporter #4584

Closed

dakshvar22 mentioned this pull request Aug 25, 2020

Proposal to change training data format for ResponseSelector #6480

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Response generator responses with multimedia elements #6323

Response generator responses with multimedia elements #6323

tmbo commented Aug 3, 2020 •

edited

Loading

tmbo commented Aug 3, 2020

tmbo Aug 3, 2020

wochinge Aug 14, 2020

tmbo Aug 3, 2020

dakshvar22 commented Aug 3, 2020 •

edited

Loading

tmbo commented Aug 3, 2020

tmbo commented Aug 3, 2020

dakshvar22 commented Aug 3, 2020 •

edited

Loading

tmbo commented Aug 3, 2020

wochinge left a comment

wochinge left a comment

wochinge Aug 14, 2020

tmbo Aug 14, 2020

wochinge Aug 14, 2020

wochinge left a comment

wochinge Aug 14, 2020

tmbo commented Aug 14, 2020

wochinge commented Aug 14, 2020

tmbo commented Aug 17, 2020

Response generator responses with multimedia elements #6323

Response generator responses with multimedia elements #6323

Conversation

tmbo commented Aug 3, 2020 • edited Loading

tmbo commented Aug 3, 2020

tmbo Aug 3, 2020

Choose a reason for hiding this comment

wochinge Aug 14, 2020

Choose a reason for hiding this comment

tmbo Aug 3, 2020

Choose a reason for hiding this comment

dakshvar22 commented Aug 3, 2020 • edited Loading

tmbo commented Aug 3, 2020

tmbo commented Aug 3, 2020

dakshvar22 commented Aug 3, 2020 • edited Loading

tmbo commented Aug 3, 2020

wochinge left a comment

Choose a reason for hiding this comment

wochinge left a comment

Choose a reason for hiding this comment

wochinge Aug 14, 2020

Choose a reason for hiding this comment

tmbo Aug 14, 2020

Choose a reason for hiding this comment

wochinge Aug 14, 2020

Choose a reason for hiding this comment

wochinge left a comment

Choose a reason for hiding this comment

wochinge Aug 14, 2020

Choose a reason for hiding this comment

tmbo commented Aug 14, 2020

wochinge commented Aug 14, 2020

tmbo commented Aug 17, 2020

tmbo commented Aug 3, 2020 •

edited

Loading

dakshvar22 commented Aug 3, 2020 •

edited

Loading

dakshvar22 commented Aug 3, 2020 •

edited

Loading