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

Rules #6088

Merged
merged 68 commits into from
Jul 10, 2020
Merged

Rules #6088

merged 68 commits into from
Jul 10, 2020

Conversation

wochinge
Copy link
Contributor

@wochinge wochinge commented Jun 29, 2020

Proposed changes:

  • add RulePolicy
  • add support for reading rules training data
  • move FormAction from SDK to Rasa Open Source
  • deprecate old "rule-like" policies (MappingPolicy, TwoStageFallbackPolicy, FallbackPolicy, FormPolicy)
  • implement TwoStageFallbackPolicy as action
  • add NLU component FallbackClassifier which predicts an intent nlu_fallback in case the confidence of the other intents is below a given threshold. This allows to write stories which handle low nlu confidence
  • new featurization for Forms. Happy paths are now represented as a single event.

Follow up issue: #6176

Status (please check what you already did):

  • 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)

Ghostvv and others added 30 commits April 29, 2020 17:44
* add sdk form

* adapt form from SDK to Rasa

* load form action & slot mappings dynamically from domain

* add simple test for RulePolicy

* remove old 'EmbeddingPolicy

* trigger form using default rules

* split stories into ML and rules

* remove warnings

* handle form unhappy paths with RulePolicy

* add `...` to default actions

* fix error when converting domain forms to dict

* update rule symbol to '>>' and fix data generation

* update tqdm desc

* fixes for forms in domain

* make function to remove predictions more general

* omit activation events for forms when deactivating immediately

* adapt rule examples to implementation

* add `is_rule_tracker=True` to tests

* match rule pattern in story file

* add test for immediate submit

* simplify form prediction

* remove unused imports

* FormAction now asks for slot

* unfeaturize `requested_slot`

* fix form submit rule

* implement review comments

* polish

- add todos
- more tests
- more comments

* remove unused variable

* remove print

* Update examples/rules/data/stories.md

* fix max_history error

* check if featurizer is not None as well

* uncomment slack

* add missing events to rules example

* update test stories to changed logic

* fix more tests

* comment extracting other slots from arbitrary entity

Co-authored-by: ricwo <ric.wkr@gmail.com>
Co-authored-by: ricwo <20581300+ricwo@users.noreply.github.com>
Co-authored-by: Vova Vv <mr.voov@gmail.com>
* add NLU component to set fallback intent

* add example rule for simple fallback

* fix form test case
* implement abstract loop interface

* adapt `FormAction` to new `loop` interface

* add `TwoStageFallbackAction`

* add todo for renaming `active_form`

* make NLU fallback intent a hardcoded intent name

It has to be hardcoded since the TwoStageFallbackPolicy relies on it's name, similar to the out of scope intent name

* rename to `loops` (plural)
* implement form unfeaturization

* rename `active_form` to `active_loop`

* adapt tests for rule policy

* use existing `REQUESTED_SLOT` constant

* use full EventVerbosity in TwoStageFallbackAction

* nuke FormPolicy test

The `FormPolicy` will be removed in favor of the `RulePolicy` so no need to fix / adapt the tests here.

* fix ` Duplicate actions in domain` in `rasa interactive`
* remove form unfeaturization, fix rules

* rename variable

* add validation false event

* remove unneeded method

* Update examples/formbot/data/stories.md

Co-authored-by: Tobias Wochinger <t.wochinger@rasa.com>

* add comments and clean rule policy

* Update rasa/core/policies/rule_policy.py

Co-authored-by: Tobias Wochinger <t.wochinger@rasa.com>

* Update rasa/core/policies/rule_policy.py

Co-authored-by: Tobias Wochinger <t.wochinger@rasa.com>

* Update rasa/core/policies/rule_policy.py

Co-authored-by: Tobias Wochinger <t.wochinger@rasa.com>

* Update rasa/core/policies/rule_policy.py

Co-authored-by: Tobias Wochinger <t.wochinger@rasa.com>

* black

* Update rasa/core/policies/rule_policy.py

Co-authored-by: Tobias Wochinger <t.wochinger@rasa.com>

* Update rasa/core/policies/rule_policy.py

Co-authored-by: Tobias Wochinger <t.wochinger@rasa.com>

* fix code quality

Co-authored-by: Tobias Wochinger <t.wochinger@rasa.com>
@wochinge wochinge marked this pull request as ready for review July 9, 2020 17:58
wochinge and others added 4 commits July 9, 2020 21:28
* deprecate old rule-like policies

* only run `FormAction` if users defined a slot mapping for the form in the domain

* remove Todo as TwoStageFallbackPolicy is deprecated anyway

* make it a module function as it has no self reference

* translate rule examples to yaml

* fix slot warning

* add explanation comment to `FallbackClassifier`

* add changelog

* Don't try to generate stories if no StorySteps given

* disable warnings about default intents when RulePolicy is available

* fix test for invalid yaml
Copy link
Contributor

@tabergma tabergma left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Took another brief look. Code wise it looks great. Good job 🚀

examples/rules/domain.yml Outdated Show resolved Hide resolved
rasa/core/training/story_reader/markdown_story_reader.py Outdated Show resolved Hide resolved
changelog/6088.feature.rst Outdated Show resolved Hide resolved
changelog/6088.feature.rst Outdated Show resolved Hide resolved
wochinge and others added 2 commits July 10, 2020 11:06
Co-authored-by: Tanja <tabergma@gmail.com>
@wochinge
Copy link
Contributor Author

@Ghostvv Tanja's new feedback is implemented.

Are we ready to merge? :drum_roll: 🤩

@Ghostvv
Copy link
Contributor

Ghostvv commented Jul 10, 2020

Tanja has to give us green flag 😇. I can approve, but it'll be cheating PR review😜

@wochinge
Copy link
Contributor Author

You care about cheating at this point? 😄 Let me quickly create another GitHub account ... 🤣

Copy link
Contributor

@tabergma tabergma left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🥁 💯

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants