Remove Policy deprecations and set max_history default to None. #9119

kedz · 2021-07-13T23:41:47Z

Proposed changes:

Remove the following deprecated policy classes:
- FormPolicy
- MappingPolicy
- FallbackPolicy
- TwoStageFallbackPolicy
- SklearnPolicy
Update the max history defaults in MemoizationPolicy.
Remove/edit tests that depended on deprecated modules.
Remove deprecated policies from docs.
Remove any soon-to-be-broken links from CHANGELOG.mdx to docs sections for these policies.

Status (please check what you already did):

Remove deprecated policy classes.
Update the max history defaults in policies
Remove/edit tests that depended on these modules
Remove deprecated policies from docs
Remove any soon-to-be-broken links from CHANGELOG.mdx to docs sections for these policies.
updated the changelog (please check changelog for instructions)
reformat files using black (please check Readme for instructions)

rasa/core/policies/ensemble.py

tests/core/test_agent.py

joejuzl · 2021-07-14T15:45:06Z

rasa/core/policies/ensemble.py

-    from rasa.core.policies.form_policy import FormPolicy
-
-    suited_policies_for_forms = (FormPolicy, RulePolicy)
+    suited_policies_for_forms = (RulePolicy)


These parenthesis are doing nothing. You would need a comma to make it a tuple, but probably a tuple is not needed now.

Removed the tuple and just now check that there is a rule policy instance in the config to handle forms.

…policy

joejuzl · 2021-07-15T12:19:33Z

reminder to tackle rasa.core.policies.memoization.py:L87:

"f""Please configure the max history in your configuration file, ""
f""currently 'max_history' is set to old default value of ""
f""'{max_history}'. If you want to have infinite max history ""
f""set it to 'None' explicitly. We will change the default value of ""
 f""'max_history' in the future to 'None'."","

…policy

…licy are removed.

kedz · 2021-07-16T04:21:22Z

The CI tests are now passing but our core regression tests are failing.

Looking into this some more, I see that the problem is unrelated to this PR. It seems that required_slots in form definitions was optional for domain files before 2.6. After 2.6 not including was still supported but deprecated. This week, support for not including it got dropped in main as part of the move to 3.0. Now including required_slots is necessary or YAML validation will fail.

Unfortunately, the two repos we run core regression tests on (finanicial-demo and helpdesk-assistant) have not had their domain files updated. I have created PRs for each of them to add this field (here and here). Once they have merged core regression tests should work again.

Although none of the core policy regression tests include any of the deprecated policies removed in this PR so the core regression tests should not be affected by any of the changes here (in theory, I wanted to run core regression tests now to confirm this which is how I discovered they were broken). We could merge if this PR is blocking other work and the core regression tests will get fixed when those other repos update their domain files?

samsucik

Overall looks good but I think we'll need to talk about how we deal with links to removed components, as well as with migration automation code that isn't for the current major version (in this case 3.x).

CHANGELOG.mdx

changelog/8880.removal.mdx

rasa/core/policies/ensemble.py

tests/core/test_config.py

tests/core/test_ensemble.py

tests/core/test_training.py

tests/core/test_policies.py

Co-authored-by: Sam Sucik <s.sucik@rasa.com>

github-actions · 2021-07-16T20:56:23Z

Commit: 60fb2f8, The full report is available as an artifact.

…policy

github-actions · 2021-07-16T21:58:04Z

Hey @kedz! 👋 To run model regression tests, comment with the /modeltest command and a configuration.

Tips 💡: The model regression test will be run on push events. You can re-run the tests by re-add status:model-regression-tests label or use a Re-run jobs button in Github Actions workflow.

Tips 💡: Every time when you want to change a configuration you should edit the comment with the previous configuration.

You can copy this in your comment and customize:

/modeltest

```yml
##########
## Available datasets
##########
# - "Carbon Bot" (NLU)
# - "Hermit" (NLU)
# - "Private 1" (NLU)
# - "Private 2" (NLU)
# - "Private 3" (NLU)
# - "Sara" (NLU)
# - "financial-demo" (NLU, Core)
# - "helpdesk-assistant" (NLU, Core)

##########
## Available NLU configurations
##########
# - "BERT + DIET(bow) + ResponseSelector(bow)"
# - "BERT + DIET(seq) + ResponseSelector(t2t)"
# - "Spacy + DIET(bow) + ResponseSelector(bow)"
# - "Spacy + DIET(seq) + ResponseSelector(t2t)"
# - "Sparse + BERT + DIET(bow) + ResponseSelector(bow)"
# - "Sparse + BERT + DIET(seq) + ResponseSelector(t2t)"
# - "Sparse + DIET(bow) + ResponseSelector(bow)"
# - "Sparse + DIET(seq) + ResponseSelector(t2t)"
# - "Sparse + Spacy + DIET(bow) + ResponseSelector(bow)"
# - "Sparse + Spacy + DIET(seq) + ResponseSelector(t2t)"

##########
## Available Core configurations
##########
# - "Rules"
# - "Rules + AugMemo"
# - "Rules + AugMemo + TED"
# - "Rules + Memo"
# - "Rules + Memo + TED"
# - "Rules + TED"

## Example configuration
#################### syntax #################
## include:
##   - dataset: ["<dataset_name>"]
##     config: ["<configuration_name>"]
#
## Example:
## include:
##  - dataset: ["Carbon Bot"]
##    config: ["Sparse + DIET(bow) + ResponseSelector(bow)"]
#
## Shortcut:
## You can use the "all" shortcut to include all available configurations or datasets
#
## Example: Use the "Sparse + EmbeddingIntent + ResponseSelector(bow)" configuration
## for all available datasets
## include:
##  - dataset: ["all"]
##    config: ["Sparse + DIET(bow) + ResponseSelector(bow)"]
#
## Example: Use all available configurations for the "Carbon Bot" and "Sara" datasets
## and for the "Hermit" dataset use the "Sparse + DIET + ResponseSelector(T2T)" and
## "BERT + DIET + ResponseSelector(T2T)" configurations:
## include:
##  - dataset: ["Carbon Bot", "Sara"]
##    config: ["all"]
##  - dataset: ["Hermit"]
##    config: ["Sparse + DIET(seq) + ResponseSelector(t2t)", "BERT + DIET(seq) + ResponseSelector(t2t)"]
#
## Example: Define a branch name to check-out for a dataset repository. Default branch is 'main'
## dataset_branch: "test-branch"
## include:
##  - dataset: ["Carbon Bot", "Sara"]
##    config: ["all"]
##
## Shortcuts:
## You can use the "all" shortcut to include all available configurations or datasets.
## You can use the "all-nlu" shortcut to include all available NLU configurations or datasets.
## You can use the "all-core" shortcut to include all available core configurations or datasets.

include:
 - dataset: ["Carbon Bot"]
   config: ["Sparse + DIET(bow) + ResponseSelector(bow)"]

```

github-actions · 2021-07-16T21:58:07Z

/modeltest

include:
 - dataset: ["all-core"]
   config: ["all-core"]

github-actions · 2021-07-16T21:58:09Z

The model regression tests have started. It might take a while, please be patient.
As soon as results are ready you'll see a new comment with the results.

Used configuration can be found in the comment.

github-actions · 2021-07-16T22:17:29Z

Commit: 3480ec1, The full report is available as an artifact.

samsucik

Looks good, let's deal with the links and then we're good to go I think 🙂

joejuzl

I love the smell of deleted code in the morning 🥀

kedz · 2021-07-19T14:21:53Z

@samsucik I created an issue here to track dead docs links. With that in place can I get permission to merge?

samsucik · 2021-07-19T14:24:42Z

@kedz I think so 😉

samsucik

🚀

…o deprecate-policy

kedz added 2 commits July 13, 2021 19:39

Remove deprecated policies and miscellaneous code depending on them.

9f141b7

Remove tests using deprecated polcies.

bb88670

kedz requested a review from a team as a code owner July 13, 2021 23:41

kedz requested review from a team and aeshky and removed request for a team and aeshky July 13, 2021 23:41

Merge branch 'main' into deprecate-policy

aa55e24

joejuzl suggested changes Jul 14, 2021

View reviewed changes

kedz added 5 commits July 15, 2021 00:46

Fixed syntax error in tests.

86b4084

Removed tuple no longer neccessary.

c4c60cd

Fix some code quality issues.

734f217

Fix some code quality issues.

0ca4e08

Merge branch 'main' of https://github.com/rasahq/rasa into deprecate-…

1aebc98

…policy

kedz added 8 commits July 15, 2021 09:11

Merge branch 'main' of https://github.com/rasahq/rasa into deprecate-…

4bd0bf2

…policy

Removed tests incorrectly added during merge conflict resolution.

ac00ac8

Code quality fixes.

011a2c8

Code quality fixes.

4a4fb96

Removed function incorrectly added during merge conflict resolution.

9fcd55e

Removed blank line I accidentally added.

7325137

Added a docstring to a public method.

e1ad8c1

Removed warning about max_history default and set default to None.

84ef2d3

kedz changed the title ~~3.0 Deprecation Warnings: Remove Policy deprecations~~ Remove Policy deprecations and set max_history default to None. Jul 15, 2021

kedz added 4 commits July 15, 2021 12:52

Updated test to use DEFAULT_MAX_HISTORY constant.

23f3cb1

Handled merge conflicts.

107caac

Removed logic than can never happen now that FormPolicy and MappingPo…

147d42d

…licy are removed.

Remove test parametrization that is no longer necessary.

26bf1f6

kedz added the runner:gpu label Jul 15, 2021

kedz requested a review from samsucik July 16, 2021 04:23

samsucik suggested changes Jul 16, 2021

View reviewed changes

Update rasa/core/policies/ensemble.py

43a3524

Co-authored-by: Sam Sucik <s.sucik@rasa.com>

kedz added 4 commits July 16, 2021 17:30

Added names of policies removed to changelog.

2aa6722

Removed code erroneously added from merge conflict resolution.

b93ccc1

Merge branch 'main' of https://github.com/rasahq/rasa into deprecate-…

6ec2346

…policy

Add test for default max_history.

7665359

kedz requested review from joejuzl and samsucik July 16, 2021 22:52

kedz removed runner:gpu status:model-regression-tests labels Jul 16, 2021

samsucik reviewed Jul 19, 2021

View reviewed changes

joejuzl approved these changes Jul 19, 2021

View reviewed changes

kedz requested a review from samsucik July 19, 2021 14:22

samsucik approved these changes Jul 19, 2021

View reviewed changes

samsucik and others added 5 commits July 19, 2021 16:25

Merge branch 'main' into deprecate-policy

9af231b

Code quality.

b58059b

Merge branch 'main' into deprecate-policy

6b8427a

trigger GitHub actions

b0ae78a

Merge branch 'deprecate-policy' of https://github.com/rasahq/rasa int…

bcfe368

…o deprecate-policy

kedz merged commit 85626b9 into main Jul 19, 2021

kedz deleted the deprecate-policy branch July 19, 2021 16:39

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Remove Policy deprecations and set max_history default to None. #9119

Remove Policy deprecations and set max_history default to None. #9119

kedz commented Jul 13, 2021 •

edited

Loading

joejuzl Jul 14, 2021

kedz Jul 15, 2021

joejuzl commented Jul 15, 2021

kedz commented Jul 16, 2021 •

edited

Loading

samsucik left a comment

github-actions bot commented Jul 16, 2021

github-actions bot commented Jul 16, 2021

github-actions bot commented Jul 16, 2021

github-actions bot commented Jul 16, 2021

github-actions bot commented Jul 16, 2021

samsucik left a comment

joejuzl left a comment

kedz commented Jul 19, 2021 •

edited

Loading

samsucik commented Jul 19, 2021

samsucik left a comment

Remove Policy deprecations and set max_history default to None. #9119

Remove Policy deprecations and set max_history default to None. #9119

Conversation

kedz commented Jul 13, 2021 • edited Loading

joejuzl Jul 14, 2021

Choose a reason for hiding this comment

kedz Jul 15, 2021

Choose a reason for hiding this comment

joejuzl commented Jul 15, 2021

kedz commented Jul 16, 2021 • edited Loading

samsucik left a comment

Choose a reason for hiding this comment

github-actions bot commented Jul 16, 2021

github-actions bot commented Jul 16, 2021

github-actions bot commented Jul 16, 2021

github-actions bot commented Jul 16, 2021

github-actions bot commented Jul 16, 2021

samsucik left a comment

Choose a reason for hiding this comment

joejuzl left a comment

Choose a reason for hiding this comment

kedz commented Jul 19, 2021 • edited Loading

samsucik commented Jul 19, 2021

samsucik left a comment

Choose a reason for hiding this comment

kedz commented Jul 13, 2021 •

edited

Loading

kedz commented Jul 16, 2021 •

edited

Loading

kedz commented Jul 19, 2021 •

edited

Loading