Loading and predicting with graph based model. #9747

joejuzl · 2021-09-29T15:15:48Z

Proposed changes:
This implements most of the functionality for loading and predicting with the graph runner.

TODO:

Fixing caching in tests using a fixture (fix cache usage in tests #9787)
Fix interactive.
yaml_store_reader in shared is using regex_message_handler.
Do we need to know what type of model we load (nlu/core)?
Some other TODO's in the code.

wochinge · 2021-09-29T15:41:01Z

Wohooo! 🎉 Amazing work! Will have a look tomorrow 🔎

joejuzl · 2021-09-30T13:50:53Z

These are the public methods of the processor that end up with a graph run:

.handle_message
# Handle a new message - full prediction
    calls ._run_prediction_loop
        loops over:
            .predict_next_with_tracker_if_should
                calls ._predict_next_action_with_tracker
                    runs graph
            ._run_action
    _save_tracker

.predict_next_with_tracker_if_should
# predicts next action but checking for should_predict_another_action etc. doesn't save tracker
    checks should_predict_another_action
    calls ._predict_next_action_with_tracker
        runs graph


.predict_next_with_tracker
# predicts next action but doesn't save tracker.
    calls ._predict_next_action_with_tracker
        runs graph

.predict_next_for_sender_id
# predict_next action but don't run it. Saves tracker (but only affects followup action)
    calls .predict_next_with_tracker
        calls ._predict_next_action_with_tracker
            runs graph
    _save_tracker (what actually gets saved?)

.log_message
# processes a message and optionally saves it to the tracker:
    calls ._handle_message_with_tracker
        calls .parse_message
            runs graph
        updates tracker with result
    optionally _save_tracker

.trigger_external_user_uttered
# triggers external message with intent and entities specified
    adds message to tracker
    calls ._run_prediction_loop
        loops over:
            .predict_next_with_tracker_if_should
                calls ._predict_next_action_with_tracker
                    runs graph
            ._run_action
    _save_tracker

.parse_message
# process message without tracker
    runs graph
    
.handle_reminder
    calls .trigger_external_user_uttered

rasa/core/agent.py

rasa/core/featurizers/tracker_featurizers.py

joejuzl · 2021-09-30T13:46:42Z

rasa/core/processor.py

        self.domain = domain
        self.tracker_store = tracker_store
        self.lock_store = lock_store
        self.max_number_of_predictions = max_number_of_predictions
-        self.message_preprocessor = message_preprocessor


We probably need to include this in the migration guide.

rasa/core/processor.py

rasa/core/run.py

rasa/server.py

rasa/shared/core/events.py

rasa/shared/core/training_data/story_reader/yaml_story_reader.py

rasa/telemetry.py

wochinge

I'll continue tomorrow morning. So far it's been a very pleasant read. Considering that we are avoiding refactorings for now just this state is already multiple factors better than what we had 💯

data/graph_schemas/config_pretrained_embeddings_mitie_predict_schema.yml

rasa/nlu/classifiers/keyword_intent_classifier.py

rasa/cli/shell.py

rasa/core/agent.py

wochinge · 2021-09-30T15:33:30Z

rasa/core/agent.py

@@ -557,47 +431,42 @@ async def parse_message_using_nlu_interpreter(
                }

        """
-
-        processor = self.create_processor()
+        if not self.is_ready():


Does it make sense to make this a decorator? Somewhat duplicate with ensure_loaded_agent in server 🤔

rasa/core/agent.py

wochinge

Next batch of comments - I think I'm mostly missing the tests now

rasa/core/processor.py

rasa/core/test.py

wochinge · 2021-10-01T09:18:08Z

rasa/core/test.py

-            parsed_message = processor.interpreter.featurize_message(
-                Message(data={TEXT: text})
-            )
+            parsed_message = processor.parse_message(UserMessage(text=text))


don't we only do this when having e2e which means that we already have the tokens from the time when we verified the NLU test result?

rasa/core/training/story_conflict.py

rasa/graph_components/providers/prediction_output_provider.py

wochinge

Amazing stuff!

rasa/core/test.py

rasa/engine/runner/dask.py

rasa/model_testing.py

tests/core/test_run.py

tests/engine/test_validation.py

tests/graph_components/providers/test_prediction_output_provider.py

tests/test_server.py

tests/nlu/test_config.py

…ure-revamp/9604/prediction

…604/prediction/cache-fix fix cache usage in tests

…604/prediction/clazz-impl-in-fingerprint include class implementation in fingerprint

…RasaHQ/rasa into 3.0-architecture-revamp/9604/prediction

…768/project_fingerprint add project fingerprint to model metadata

…RasaHQ/rasa into 3.0-architecture-revamp/9604/prediction

wochinge

Great work! Requesting changes due to the raise at the server start

rasa/core/agent.py

rasa/core/run.py

wochinge · 2021-10-07T09:42:37Z

rasa/core/test.py

-            parsed_message = processor.interpreter.featurize_message(
-                Message(data={TEXT: text})
-            )
+            parsed_message = processor.parse_message(UserMessage(text=text))


rasa/engine/storage/local_model_storage.py

rasa/engine/storage/storage.py

wochinge · 2021-10-07T09:47:41Z

rasa/server.py

        )
+        new_agent.lock_store = app.agent.lock_store


I don't really get why we have to do the lockstore separately but as it was done like that before 🤷🏻

wochinge

💯 🎸 🚀

joejuzl requested a review from wochinge September 29, 2021 15:15

Loading and predicting with graph based model.

5885e5b

joejuzl force-pushed the 3.0-architecture-revamp/9604/prediction branch from 520d4aa to 5885e5b Compare September 29, 2021 15:16

remove unneeded nlu_merge_needs

309c1f2

joejuzl commented Sep 30, 2021

View reviewed changes

wochinge reviewed Sep 30, 2021

View reviewed changes

joejuzl linked an issue Oct 1, 2021 that may be closed by this pull request

Integrate rasa train and rasa run with new model architecture #9604

Closed

4 tasks

joejuzl marked this pull request as ready for review October 1, 2021 08:14

joejuzl requested a review from a team as a code owner October 1, 2021 08:14

joejuzl requested review from a team and alopez and removed request for a team and alopez October 1, 2021 08:14

wochinge reviewed Oct 1, 2021

View reviewed changes

wochinge mentioned this pull request Oct 4, 2021

Replace old os.path calls by pathlib correspondents - part 1 #9708

Closed

4 tasks

joejuzl and others added 11 commits October 6, 2021 11:48

Include training type in metadata (#9799)

70faeab

include class implementation in fingerprint

cbe45dd

copy existing module

ffe6657

Merge branch '3.0-architecture-revamp/9277/recipe' into 3.0-architect…

76edcc5

…ure-revamp/9604/prediction

fix cache usage in tests

f328571

fix some code quality issues

de063b2

fix some model training errors

1724bb1

omit reverting back sesion wide fixture

9409b02

fix MitieIntentClassifier return type

2ab1bb4

retrieve latest model first

d05f2fe

Merge pull request #9787 from RasaHQ/origin/3.0-architecture-revamp/9…

4039d5f

…604/prediction/cache-fix fix cache usage in tests

joejuzl and others added 13 commits October 6, 2021 16:29

refactor agent and processor loading

fc406df

PR comments

de51774

more PR comments

f93de25

fix config validation with trainable tokenizer

8d5df31

Merge pull request #9788 from RasaHQ/origin/3.0-architecture-revamp/9…

d08856c

…604/prediction/clazz-impl-in-fingerprint include class implementation in fingerprint

add simple Agent.load()

0ed988c

Merge branch '3.0-architecture-revamp/9604/prediction' of github.com:…

895a8f9

…RasaHQ/rasa into 3.0-architecture-revamp/9604/prediction

add project fingerprint to model metadata

214c398

drop no longer necessary initialize_processor

8c4a070

Merge pull request #9808 from RasaHQ/origin/3.0-architecture-revamp/9…

e945ff0

…768/project_fingerprint add project fingerprint to model metadata

return from constructor

689979f

Merge branch '3.0-architecture-revamp/9604/prediction' of github.com:…

5efe8d4

…RasaHQ/rasa into 3.0-architecture-revamp/9604/prediction

fix tests

4483a09

wochinge suggested changes Oct 7, 2021

View reviewed changes

wochinge and others added 4 commits October 7, 2021 11:50

fix test_fingerprint_changes_if_module_changes

804c224

fix tests

7140520

add clarifying comment

f78b260

small changes

6f95c60

wochinge mentioned this pull request Oct 7, 2021

Architecture Revamp: Polishing / Wrap up #9816

Closed

12 tasks

joejuzl added 2 commits October 7, 2021 12:17

return empty agent from any exception in load_agent

44674fd

remove unnecessary try catch

ae9fa99

wochinge approved these changes Oct 7, 2021

View reviewed changes

joejuzl merged commit 2e7b6ee into 3.0-architecture-revamp/9277/recipe Oct 7, 2021

joejuzl deleted the 3.0-architecture-revamp/9604/prediction branch October 7, 2021 10:32

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Loading and predicting with graph based model. #9747

Loading and predicting with graph based model. #9747

joejuzl commented Sep 29, 2021 •

edited

Loading

wochinge commented Sep 29, 2021

joejuzl commented Sep 30, 2021

joejuzl Sep 30, 2021

wochinge left a comment

wochinge Sep 30, 2021

wochinge left a comment

wochinge Oct 1, 2021

wochinge left a comment

wochinge left a comment

wochinge Oct 7, 2021

wochinge Oct 7, 2021

wochinge left a comment

Loading and predicting with graph based model. #9747

Loading and predicting with graph based model. #9747

Conversation

joejuzl commented Sep 29, 2021 • edited Loading

TODO:

wochinge commented Sep 29, 2021

joejuzl commented Sep 30, 2021

joejuzl Sep 30, 2021

Choose a reason for hiding this comment

wochinge left a comment

Choose a reason for hiding this comment

wochinge Sep 30, 2021

Choose a reason for hiding this comment

wochinge left a comment

Choose a reason for hiding this comment

wochinge Oct 1, 2021

Choose a reason for hiding this comment

wochinge left a comment

Choose a reason for hiding this comment

wochinge left a comment

Choose a reason for hiding this comment

wochinge Oct 7, 2021

Choose a reason for hiding this comment

wochinge Oct 7, 2021

Choose a reason for hiding this comment

wochinge left a comment

Choose a reason for hiding this comment

joejuzl commented Sep 29, 2021 •

edited

Loading