Add support for different recipes #10641

tayfun · 2022-01-06T18:16:57Z

Fixes #10473

Proposed changes:

Add support for different recipe files
See the docs: https://10641--rasahq-docs-rasa-v2.netlify.app/docs/rasa/graph-recipe/

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)

Fixes #10473

tayfun · 2022-01-11T17:10:07Z

After removing autoconfigure module, following rename was done:

get_configuration => auto_configure
_auto_configure => complete_config

tayfun · 2022-01-12T11:08:21Z

rasa/shared/importers/importer.py

+        model_config = recipe.graph_config_for_recipe(
+            config, cli_par, training_type=training_type, is_finetuning=is_finetuning,
+        )
+        return model_config


This is causing tests to fail because shared is not supposed to import anything from outside (ie. recipes).

tayfun · 2022-01-14T13:35:28Z

tests/shared/importers/test_importer.py

-    rasa.shared.utils.io.write_yaml(invalid, config_file)
-
-    with pytest.raises(YamlValidationException):
-        TrainingDataImporter.load_from_config(str(config_file))


This test moved to tests/graph_components/validators/test_default_recipe_validator.py as auto-config is dependent on recipe and not importer.

tayfun · 2022-01-14T13:44:38Z

tests/shared/importers/test_autoconfig.py

-
-    config = autoconfig.get_configuration(str(config_file))
-
-    assert config == {}


This test has been removed because importers will return empty dict without going through auto-config now on.

All other tests in this file have been moved to tests/engine/recipes/test_default_recipe.py because auto-config is recipe business.

Since now training type is used for auto-config

* fix over-indent in Tokenizers section of docs * add changelog * rename changelog from bug to docs

joejuzl · 2022-02-01T10:56:12Z

changelog/10473.feature.md

+Support other recipe types.
+
+This pull request also adds support for graph recipes, see details at
+https://rasa.com/docs/rasa/model-configuration check Graph Recipes section.


I guess you can use an anchor link to go directly to that section?

It's a new page not an anchor and I can't link to it because it's not live yet, docs check fails with dead link :D

joejuzl · 2022-02-02T09:22:59Z

docs/docs/graph-recipe.mdx

-We now support graph recipes in addition to the default recipe. What graph recipes provide is more powerful control over how execution graph schemas are built.
+:::tip Default Recipe or Graph Recipe?
+
+You will probably only need graph recipes if you're running ML experiments or ablation studies on an existing model. We recommend starting with the default recipe and for many applications that will be all that's needed.


"ablation studies" feels a bit specific? (I had to google it 😁 )

I've used the exact wording from @rctatman actually and I had to google it too :D I think it is good because it has the effect of warning against using graph recipes if the user is not too advanced.

docs/docs/graph-recipe.mdx

joejuzl · 2022-02-02T09:27:27Z

rasa/engine/recipes/graph_recipe.py

+        if training_type == TrainingType.NLU:
+            core_target = None
+        else:
+            core_target = config.get("core_target", "select_prediction")


Maybe it's better to fail fast rather than have a default?

I'm more inclined towards "fail-safe" rather than "fail-fast" but @ka-bu had a similar opinion on another line where if config file changes between Rasa reading it and auto-configuring that we should fail instead of warning the user. She has a comment on this pull request. I feel defensive programming is better here as this is also the default target for default recipe? We could also add a warning log as a sort of a trade off? Otherwise I can also fail it :) Not too strong on this.

I'm just thinking from a user perspective:

User decides to create a new graph recipe config.

They forget to add the targets

They get an error saying that component select_prediction or RegexMessageHandler does not exist in the schema.

They assume they are missing a required component, when actually the just need to configure the targets to fit their components.

Added code to raise with a message if targets are missing.

Co-authored-by: Joe Juzl <[email protected]>

This reverts commit 998da42.

github-actions · 2022-02-02T14:22:26Z

🚀 A preview of the docs have been deployed at the following URL: https://10641--rasahq-docs-rasa-v2.netlify.app/docs/rasa

joejuzl

🚀 🚀

Tayfun Sen added 6 commits January 6, 2022 18:15

Add support for different recipes

78fc387

Fixes #10473

Remove extra import

661342c

Fix changes so tests pass and backwards compatible

4554cc2

Merge branch 'main' into tayfun/10473-support-other-recipes

561b282

Graph recipe beginnings

890812b

Fix code climate

e883752

Tayfun Sen added 4 commits January 11, 2022 17:11

Fix auto_configure test failures

08da835

Fix mypy errors

f78b775

Fix tests

4b8d17a

Fix flake8 warnings

4d1288a

tayfun commented Jan 12, 2022

View reviewed changes

Tayfun Sen added 7 commits January 13, 2022 18:38

What happens in shared stays in shared

3f8f471

Merge branch 'main' into tayfun/10473-support-other-recipes

d967bc2

Fix tests

9332d63

Fix mypy complaints

495fc49

Fix tests

3306db5

Fix tests

a057cdc

Fix extra import

fc19f83

tayfun commented Jan 14, 2022

View reviewed changes

Tayfun Sen added 9 commits January 14, 2022 14:51

No need to change auto-config template

df2098e

Since now training type is used for auto-config

Merge branch 'main' into tayfun/10473-support-other-recipes

c44ea8c

Mark as experimental and track recipe via telemetry

248d0ea

Warn if CLI parameters for graph recipe

1b25fa3

Fix graph recipe

1ddc9be

Merge branch 'main' into tayfun/10473-support-other-recipes

bcb659c

Fix tests

ef42783

Black reformat

3716a7d

Update with new component

a1fa40a

Tayfun Sen and others added 8 commits January 28, 2022 15:55

Make tests clear; rewrite without monkeypatch

1b9954a

Fix typo

cb454dd

Add tip at the top for graph recipe docs

fa43bac

fix over-indent in Tokenizers section of docs (#10555)

249a046

* fix over-indent in Tokenizers section of docs * add changelog * rename changelog from bug to docs

Merge branch 'main' into tayfun/10473-support-other-recipes

1d3dc70

Fix mypy error

8e631e3

Fix tests for auto-config

495fb46

Update docs with target nodes

e5fa3ca

tayfun requested review from joejuzl and wochinge January 31, 2022 13:04

Merge branch 'main' into tayfun/10473-support-other-recipes

2b5af0f

joejuzl reviewed Feb 2, 2022

View reviewed changes

Tayfun Sen and others added 11 commits February 2, 2022 10:27

Update docs/docs/graph-recipe.mdx

fb9479b

Co-authored-by: Joe Juzl <[email protected]>

Move graph recipe to data folder for tests

2bc6fe5

Warn if multiple importers are used

9c771c8

Add graph config schemas to telemetry

b43566f

Fix changelog URL

998da42

Merge branch 'main' into tayfun/10473-support-other-recipes

3a491d4

Revert "Fix changelog URL"

16a4c24

This reverts commit 998da42.

Fix link to yet inexistent graph recipe docs

fa1c2eb

Add test for graph recipe telemetry event

5efdb50

Make targets required, raise if not provided

b978569

Fix mypy

506f130

tayfun requested a review from joejuzl February 2, 2022 14:58

tayfun enabled auto-merge (squash) February 2, 2022 15:00

tayfun requested a review from ka-bu February 2, 2022 15:00

joejuzl approved these changes Feb 2, 2022

View reviewed changes

tayfun merged commit 6339856 into main Feb 2, 2022

tayfun deleted the tayfun/10473-support-other-recipes branch February 2, 2022 15:19

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add support for different recipes #10641

Add support for different recipes #10641

tayfun commented Jan 6, 2022 •

edited

Loading

tayfun commented Jan 11, 2022

tayfun Jan 12, 2022 •

edited

Loading

tayfun Jan 14, 2022

tayfun Jan 14, 2022

tayfun Jan 14, 2022

joejuzl Feb 1, 2022

tayfun Feb 2, 2022

joejuzl Feb 2, 2022

tayfun Feb 2, 2022

joejuzl Feb 2, 2022

tayfun Feb 2, 2022

joejuzl Feb 2, 2022

tayfun Feb 2, 2022

github-actions bot commented Feb 2, 2022

joejuzl left a comment


		config = autoconfig.get_configuration(str(config_file))

		assert config == {}

Add support for different recipes #10641

Add support for different recipes #10641

Conversation

tayfun commented Jan 6, 2022 • edited Loading

tayfun commented Jan 11, 2022

tayfun Jan 12, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

github-actions bot commented Feb 2, 2022

joejuzl left a comment

Choose a reason for hiding this comment

tayfun commented Jan 6, 2022 •

edited

Loading

tayfun Jan 12, 2022 •

edited

Loading