Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Migration from old response selector format #6865

Closed
5 tasks
dakshvar22 opened this issue Oct 1, 2020 · 8 comments · Fixed by #7183
Closed
5 tasks

Migration from old response selector format #6865

dakshvar22 opened this issue Oct 1, 2020 · 8 comments · Fixed by #7183
Assignees
Labels
area:rasa-oss 🎡 Anything related to the open source Rasa framework type:enhancement ✨ Additions of new features or changes to existing ones, should be doable in a single PR

Comments

@dakshvar22
Copy link
Contributor

Description of Problem:
Since, we changed the training data format for response selector, it would be good to help users smoothly transition to the new format through a cli command.

Overview of the Solution:
It can be part of the rasa data convert command that now exists.
Migration changes that need to be covered -

  • Change all response templates that are defined for retrieval intents to have the utter_ prefix. For e.g.
## ask_name
* chitchat/ask_name
  - My name is retrieval bot

should get converted to

responses:
   utter_chitchat/ask_name:
     - text: My name is retrieval bot
  • Change all occurrences of actions with respond_ prefix to use utter_ prefix instead. These would be in domain and stories.

Definition of Done:

  • Tests are added
  • Feature described the docs
  • Feature mentioned in the changlog
@dakshvar22 dakshvar22 added type:enhancement ✨ Additions of new features or changes to existing ones, should be doable in a single PR area:rasa-oss 🎡 Anything related to the open source Rasa framework labels Oct 1, 2020
@dakshvar22 dakshvar22 added this to the 2.1 Rasa Open Source milestone Oct 1, 2020
@ricwo ricwo added the type:discussion 👨‍👧‍👦 Early stage of an idea or validation of thoughts. Should NOT be closed by PR. label Oct 2, 2020
@m-vdb m-vdb unassigned degiz Oct 22, 2020
@wochinge wochinge added priority:high and removed type:discussion 👨‍👧‍👦 Early stage of an idea or validation of thoughts. Should NOT be closed by PR. labels Oct 30, 2020
@m-vdb m-vdb self-assigned this Nov 2, 2020
@m-vdb
Copy link
Collaborator

m-vdb commented Nov 4, 2020

Apparently we already have a rasa data convert nlg command that does the conversion from Markdown to Yaml, so I'll modify the command to add the behaviour.

@m-vdb
Copy link
Collaborator

m-vdb commented Nov 4, 2020

@dakshvar22 I am not sure how to implement the updates to the stories and domain files (your second point). My initial thoughts where:

  1. use the rasa data convert nlg command
  2. rename the response names before the yaml file is dumped, then dump the file
  3. for every response renaming that was done, update the stories and domain files

1 and 2 are straightforward. For 3, I'm not sure how people would use it. Should they provide a --domain and --stories argument? Or should we detect where the domain and the stories files reside? For the former, it's straightforward. For the latter, I am not sure how to do it.

cc @degiz do you have any thoughts on this?

@dakshvar22
Copy link
Contributor Author

I think the default can be that the user provides path to the training data. In that path, we try to search for -

  1. NLU files(including responses.yml),
  2. Story files
    Additionally, we also search for domain in its default path. Based on what all we get, we should be able to modify.
    The command can also let the user explicitly specify the paths for domain and stories which would override the default paths.

What do you think?

@m-vdb
Copy link
Collaborator

m-vdb commented Nov 4, 2020

The thing is that with this approach, you have to consider the conversion of the whole project: are stories in Markdown or already converted to Yaml? I don't know if we can detect this reasonably well, I'd have to check.

@m-vdb m-vdb mentioned this issue Nov 4, 2020
3 tasks
@degiz
Copy link
Contributor

degiz commented Nov 5, 2020

The thing is that with this approach, you have to consider the conversion of the whole project: are stories in Markdown or already converted to Yaml

I believe the only two cases to support here is when users migrate from MD to MD (and we won't do that since MD is deprecated), or from YAML to YAML.

use the rasa data convert nlg command

I honestly think that rasa data convert nlg shouldn't look for domain and stories.
Instead, I'd rather add something like rasa data convert responses, and add descriptive help with an explaination of what it's doing.

So whole workflow for users with existing MD training data will be:

  • convert MD to YAML using rasa data convert {nlu|core|nlg}, that only reads MD files
  • convert responses to the utter_ prefix with rasa data convert responses, this will modify domain and stories if needed.

@m-vdb
Copy link
Collaborator

m-vdb commented Nov 5, 2020

Thanks @degiz

I honestly think that rasa data convert nlg shouldn't look for domain and stories.

can you briefly explain why?

@degiz
Copy link
Contributor

degiz commented Nov 5, 2020

can you briefly explain why?

rasa data convert {nlu|core|nlg} all work with training data only. Also, we don't require stories to be present when we convert nlu data, we also don't expect the domain to be present when we convert nlg.

Otherwise, we'll end up with a full English dictionary in the args 😄 IMO that's super confusing:
rasa data convert nlg --domain domain.yaml --stories stories.yaml

I realize though, that rasa data convert responses will also require at least --core param for the stories.

@m-vdb
Copy link
Collaborator

m-vdb commented Nov 5, 2020

OK makes sense. I'm for simplifying the implementation of this, because it will result in a simpler usage; having this in a separate command sounds better 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:rasa-oss 🎡 Anything related to the open source Rasa framework type:enhancement ✨ Additions of new features or changes to existing ones, should be doable in a single PR
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants