From a52e19daafaea212743dc3190cecd12633d892f7 Mon Sep 17 00:00:00 2001 From: Joseph Juzl Date: Fri, 4 Dec 2020 11:17:57 +0100 Subject: [PATCH 1/7] Load NLU model in fine-tune mode with updated config --- rasa/nlu/model.py | 54 +++++++++++++++++++++++++++++++++++++++++------ rasa/nlu/train.py | 6 +++--- rasa/train.py | 26 +++++++++++------------ 3 files changed, 64 insertions(+), 22 deletions(-) diff --git a/rasa/nlu/model.py b/rasa/nlu/model.py index 241863aeb7da..af75c4fca247 100644 --- a/rasa/nlu/model.py +++ b/rasa/nlu/model.py @@ -138,6 +138,7 @@ def __init__( cfg: RasaNLUModelConfig, component_builder: Optional[ComponentBuilder] = None, skip_validation: bool = False, + model_to_finetune: Optional["Interpreter"] = None, ): self.config = cfg @@ -154,8 +155,10 @@ def __init__( if not self.skip_validation: components.validate_requirements(cfg.component_names) - # build pipeline - self.pipeline = self._build_pipeline(cfg, component_builder) + if model_to_finetune: + self.pipeline = model_to_finetune.pipeline + else: + self.pipeline = self._build_pipeline(cfg, component_builder) def _build_pipeline( self, cfg: RasaNLUModelConfig, component_builder: ComponentBuilder @@ -297,6 +300,8 @@ def load( model_dir: Text, component_builder: Optional[ComponentBuilder] = None, skip_validation: bool = False, + new_config: Optional[Dict] = None, + finetuning_epoch_fraction: float = 1.0, ) -> "Interpreter": """Create an interpreter based on a persisted model. @@ -307,25 +312,62 @@ def load( model_dir: The path of the model to load component_builder: The :class:`rasa.nlu.components.ComponentBuilder` to use. + new_config: Optional new config to use for the new epochs. + finetuning_epoch_fraction: Value to multiply all epochs by. Returns: An interpreter that uses the loaded model. """ - model_metadata = Metadata.load(model_dir) + if new_config: + Interpreter._update_epochs_from_new_config( + model_metadata, new_config, finetuning_epoch_fraction + ) + Interpreter.ensure_model_compatibility(model_metadata) - return Interpreter.create(model_metadata, component_builder, skip_validation) + return Interpreter.create( + model_metadata, + component_builder, + skip_validation, + should_finetune=new_config is not None, + ) + + @staticmethod + def _update_epochs_from_new_config( + model_metadata: Metadata, + new_config: Optional[Dict] = None, + finetuning_epoch_fraction: float = 1.0, + ): + for p1, p2 in zip(model_metadata.metadata["pipeline"], new_config["pipeline"]): + assert p1.get("name") == p2.get("name") + if "epochs" in p1: + p1["epochs"] = ( + p2.get("epochs", p1["epochs"]) * finetuning_epoch_fraction + ) @staticmethod def create( model_metadata: Metadata, component_builder: Optional[ComponentBuilder] = None, skip_validation: bool = False, + should_finetune: bool = False, ) -> "Interpreter": - """Load stored model and components defined by the provided metadata.""" + """Create model and components defined by the provided metadata. - context = {} + Args: + model_metadata: The metadata describing each component. + component_builder: The + :class:`rasa.nlu.components.ComponentBuilder` to use. + skip_validation: If set to `True`, does not check that all + required packages for the components are installed + before loading them. + should_finetune: Indicates if the model components will be fine-tuned. + + Returns: + An interpreter that uses the created model. + """ + context = {"finetune_mode": should_finetune} if component_builder is None: # If no builder is passed, every interpreter creation will result diff --git a/rasa/nlu/train.py b/rasa/nlu/train.py index c2fc060b2268..4a7ae63151d4 100644 --- a/rasa/nlu/train.py +++ b/rasa/nlu/train.py @@ -3,7 +3,6 @@ from typing import Any, Optional, Text, Tuple, Union, Dict import rasa.shared.utils.common -import rasa.utils.common as common_utils from rasa.nlu import config, utils from rasa.nlu.components import ComponentBuilder from rasa.nlu.config import RasaNLUModelConfig @@ -84,7 +83,6 @@ async def train( training_data_endpoint: Optional[EndpointConfig] = None, persist_nlu_training_data: bool = False, model_to_finetune: Optional[Interpreter] = None, - finetuning_epoch_fraction: float = 1.0, **kwargs: Any, ) -> Tuple[Trainer, Interpreter, Optional[Text]]: """Loads the trainer and the data and runs the training of the model.""" @@ -96,7 +94,9 @@ async def train( # Ensure we are training a model that we can save in the end # WARN: there is still a race condition if a model with the same name is # trained in another subprocess - trainer = Trainer(nlu_config, component_builder) + trainer = Trainer( + nlu_config, component_builder, model_to_finetune=model_to_finetune + ) persistor = create_persistor(storage) if training_data_endpoint is not None: training_data = await load_data_from_endpoint( diff --git a/rasa/train.py b/rasa/train.py index dcc3f428b8ea..a0fc7011642d 100644 --- a/rasa/train.py +++ b/rasa/train.py @@ -631,7 +631,9 @@ async def _train_nlu_with_validated_data( ) if model_to_finetune: - model_to_finetune = _nlu_model_for_finetuning(model_to_finetune) + model_to_finetune = _nlu_model_for_finetuning( + model_to_finetune, config, finetuning_epoch_fraction + ) if not model_to_finetune: rasa.shared.utils.cli.print_warning( @@ -652,7 +654,6 @@ async def _train_nlu_with_validated_data( fixed_model_name="nlu", persist_nlu_training_data=persist_nlu_training_data, model_to_finetune=model_to_finetune, - finetuning_epoch_fraction=finetuning_epoch_fraction, **additional_arguments, ) rasa.shared.utils.cli.print_color( @@ -674,19 +675,18 @@ async def _train_nlu_with_validated_data( return _train_path -def _nlu_model_for_finetuning(model_to_finetune: Text) -> Optional[Interpreter]: - from rasa.core.interpreter import RasaNLUInterpreter - +def _nlu_model_for_finetuning( + model_to_finetune: Text, new_config: Dict, finetuning_epoch_fraction: float = 1.0, +) -> Optional[Interpreter]: path_to_archive = model.get_model_for_finetuning(model_to_finetune) if not path_to_archive: return None - try: - interpreter = _interpreter_from_previous_model(path_to_archive) - if interpreter and isinstance(interpreter, RasaNLUInterpreter): - return interpreter.interpreter - except Exception: - # Anything might go wrong. In that case we skip model finetuning. - pass + with model.unpack_model(path_to_archive) as unpacked: + _, old_nlu = model.get_model_subdirectories(unpacked) - return None + return Interpreter.load( + old_nlu, + new_config=new_config, + finetuning_epoch_fraction=finetuning_epoch_fraction, + ) From 25c23ca1cabd6abb65e6deb60ac06c140e237047 Mon Sep 17 00:00:00 2001 From: Joseph Juzl Date: Fri, 4 Dec 2020 13:42:10 +0100 Subject: [PATCH 2/7] Test --- rasa/nlu/model.py | 6 ++++-- tests/test_train.py | 31 +++++++++++++++++++++++-------- 2 files changed, 27 insertions(+), 10 deletions(-) diff --git a/rasa/nlu/model.py b/rasa/nlu/model.py index af75c4fca247..75cfd4d9e8fd 100644 --- a/rasa/nlu/model.py +++ b/rasa/nlu/model.py @@ -1,6 +1,7 @@ import copy import datetime import logging +from math import ceil import os from typing import Any, Dict, List, Optional, Text @@ -340,9 +341,10 @@ def _update_epochs_from_new_config( finetuning_epoch_fraction: float = 1.0, ): for p1, p2 in zip(model_metadata.metadata["pipeline"], new_config["pipeline"]): - assert p1.get("name") == p2.get("name") + if not p1.get("name") == p2.get("name"): + raise InvalidModelError("Inconsistent config for model to fine-tune.") if "epochs" in p1: - p1["epochs"] = ( + p1["epochs"] = ceil( p2.get("epochs", p1["epochs"]) * finetuning_epoch_fraction ) diff --git a/tests/test_train.py b/tests/test_train.py index d5aa2eff8bf7..c108ba9a2a7e 100644 --- a/tests/test_train.py +++ b/tests/test_train.py @@ -13,6 +13,7 @@ import rasa.core import rasa.nlu import rasa.shared.importers.autoconfig as autoconfig +import rasa.shared.utils.io from rasa.core.agent import Agent from rasa.core.interpreter import RasaNLUInterpreter from rasa.nlu.model import Interpreter @@ -430,30 +431,44 @@ def test_model_finetuning_nlu( monkeypatch: MonkeyPatch, default_domain_path: Text, default_nlu_data: Text, - default_stack_config: Text, - trained_rasa_model: Text, + trained_moodbot_path: Text, use_latest_model: bool, ): mocked_nlu_training = AsyncMock(return_value="") monkeypatch.setattr(rasa.nlu, rasa.nlu.train.__name__, mocked_nlu_training) + mock_interpreter_create = Mock(wraps=Interpreter.create) + monkeypatch.setattr(Interpreter, "create", mock_interpreter_create) + (tmp_path / "models").mkdir() output = str(tmp_path / "models") if use_latest_model: - trained_rasa_model = str(Path(trained_rasa_model).parent) + trained_moodbot_path = str(Path(trained_moodbot_path).parent) + + old_config = rasa.shared.utils.io.read_yaml_file("examples/moodbot/config.yml") + old_config["pipeline"][-1]["epochs"] = 10 + new_config_path = tmp_path / "new_config.yml" + rasa.shared.utils.io.write_yaml(old_config, new_config_path) train_nlu( - default_stack_config, - default_nlu_data, + str(new_config_path), + "examples/moodbot/data/nlu.yml", output=output, - model_to_finetune=trained_rasa_model, - finetuning_epoch_fraction=1, + model_to_finetune=trained_moodbot_path, + finetuning_epoch_fraction=0.5, ) + assert mock_interpreter_create.call_args[1]["should_finetune"] + mocked_nlu_training.assert_called_once() _, kwargs = mocked_nlu_training.call_args - assert isinstance(kwargs["model_to_finetune"], Interpreter) + model_to_finetune = kwargs["model_to_finetune"] + assert isinstance(model_to_finetune, Interpreter) + + new_diet_metadata = model_to_finetune.model_metadata.metadata["pipeline"][-1] + assert new_diet_metadata["name"] == "DIETClassifier" + assert new_diet_metadata["epochs"] == 5 @pytest.mark.parametrize("model_to_fine_tune", ["invalid-path-to-model", "."]) From 6260d4c51b1bc2fad5567175fadda63538b5bee4 Mon Sep 17 00:00:00 2001 From: Joseph Juzl Date: Mon, 7 Dec 2020 09:34:54 +0100 Subject: [PATCH 3/7] PR comments --- rasa/nlu/model.py | 2 +- rasa/train.py | 6 ++++-- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/rasa/nlu/model.py b/rasa/nlu/model.py index 75cfd4d9e8fd..a7e71ad74e9b 100644 --- a/rasa/nlu/model.py +++ b/rasa/nlu/model.py @@ -369,7 +369,7 @@ def create( Returns: An interpreter that uses the created model. """ - context = {"finetune_mode": should_finetune} + context = {"should_finetune": should_finetune} if component_builder is None: # If no builder is passed, every interpreter creation will result diff --git a/rasa/train.py b/rasa/train.py index a0fc7011642d..48626e85ac26 100644 --- a/rasa/train.py +++ b/rasa/train.py @@ -2,7 +2,7 @@ import os import tempfile from contextlib import ExitStack -from typing import Text, Optional, List, Union, Dict, TYPE_CHECKING +from typing import Any, Text, Optional, List, Union, Dict, TYPE_CHECKING import rasa.core.interpreter from rasa.shared.nlu.interpreter import NaturalLanguageInterpreter @@ -676,7 +676,9 @@ async def _train_nlu_with_validated_data( def _nlu_model_for_finetuning( - model_to_finetune: Text, new_config: Dict, finetuning_epoch_fraction: float = 1.0, + model_to_finetune: Text, + new_config: Dict[Text, Any], + finetuning_epoch_fraction: float = 1.0, ) -> Optional[Interpreter]: path_to_archive = model.get_model_for_finetuning(model_to_finetune) if not path_to_archive: From 638008fb025a8c9a06177e2810f9bb940b8522c3 Mon Sep 17 00:00:00 2001 From: Joseph Juzl Date: Mon, 7 Dec 2020 14:18:51 +0100 Subject: [PATCH 4/7] PR comments --- rasa/nlu/model.py | 16 +++++++++------- tests/test_train.py | 19 +++++++++++++++---- 2 files changed, 24 insertions(+), 11 deletions(-) diff --git a/rasa/nlu/model.py b/rasa/nlu/model.py index a7e71ad74e9b..3c3b4798b0ac 100644 --- a/rasa/nlu/model.py +++ b/rasa/nlu/model.py @@ -27,6 +27,7 @@ from rasa.shared.nlu.training_data.training_data import TrainingData from rasa.shared.nlu.training_data.message import Message from rasa.nlu.utils import write_json_to_file +from rasa.utils.tensorflow.constants import EPOCHS logger = logging.getLogger(__name__) @@ -140,7 +141,7 @@ def __init__( component_builder: Optional[ComponentBuilder] = None, skip_validation: bool = False, model_to_finetune: Optional["Interpreter"] = None, - ): + ) -> None: self.config = cfg self.skip_validation = skip_validation @@ -340,12 +341,13 @@ def _update_epochs_from_new_config( new_config: Optional[Dict] = None, finetuning_epoch_fraction: float = 1.0, ): - for p1, p2 in zip(model_metadata.metadata["pipeline"], new_config["pipeline"]): - if not p1.get("name") == p2.get("name"): - raise InvalidModelError("Inconsistent config for model to fine-tune.") - if "epochs" in p1: - p1["epochs"] = ceil( - p2.get("epochs", p1["epochs"]) * finetuning_epoch_fraction + for old_component_config, new_component_config in zip( + model_metadata.metadata["pipeline"], new_config["pipeline"] + ): + if EPOCHS in old_component_config: + old_component_config[EPOCHS] = ceil( + new_component_config.get(EPOCHS, old_component_config[EPOCHS]) + * finetuning_epoch_fraction ) @staticmethod diff --git a/tests/test_train.py b/tests/test_train.py index c108ba9a2a7e..6188905311fa 100644 --- a/tests/test_train.py +++ b/tests/test_train.py @@ -12,6 +12,7 @@ import rasa.model import rasa.core import rasa.nlu +from rasa.nlu.classifiers.diet_classifier import DIETClassifier import rasa.shared.importers.autoconfig as autoconfig import rasa.shared.utils.io from rasa.core.agent import Agent @@ -19,6 +20,7 @@ from rasa.nlu.model import Interpreter from rasa.train import train_core, train_nlu, train +from rasa.utils.tensorflow.constants import EPOCHS from tests.conftest import DEFAULT_CONFIG_PATH, DEFAULT_NLU_DATA, AsyncMock from tests.core.conftest import DEFAULT_DOMAIN_PATH_WITH_SLOTS, DEFAULT_STORIES_FILE from tests.test_model import _fingerprint @@ -440,14 +442,20 @@ def test_model_finetuning_nlu( mock_interpreter_create = Mock(wraps=Interpreter.create) monkeypatch.setattr(Interpreter, "create", mock_interpreter_create) + mock_DIET_load = Mock(wraps=DIETClassifier.load) + monkeypatch.setattr(DIETClassifier, "load", mock_DIET_load) + (tmp_path / "models").mkdir() output = str(tmp_path / "models") if use_latest_model: trained_moodbot_path = str(Path(trained_moodbot_path).parent) + # Typically models will be fine-tuned with a smaller number of epochs than training + # from scratch. + # Fine-tuning will use the number of epochs in the new config. old_config = rasa.shared.utils.io.read_yaml_file("examples/moodbot/config.yml") - old_config["pipeline"][-1]["epochs"] = 10 + old_config["pipeline"][-1][EPOCHS] = 10 new_config_path = tmp_path / "new_config.yml" rasa.shared.utils.io.write_yaml(old_config, new_config_path) @@ -462,13 +470,16 @@ def test_model_finetuning_nlu( assert mock_interpreter_create.call_args[1]["should_finetune"] mocked_nlu_training.assert_called_once() - _, kwargs = mocked_nlu_training.call_args - model_to_finetune = kwargs["model_to_finetune"] + _, nlu_train_kwargs = mocked_nlu_training.call_args + model_to_finetune = nlu_train_kwargs["model_to_finetune"] assert isinstance(model_to_finetune, Interpreter) + _, diet_kwargs = mock_DIET_load.call_args + assert diet_kwargs["should_finetune"] is True + new_diet_metadata = model_to_finetune.model_metadata.metadata["pipeline"][-1] assert new_diet_metadata["name"] == "DIETClassifier" - assert new_diet_metadata["epochs"] == 5 + assert new_diet_metadata[EPOCHS] == 5 @pytest.mark.parametrize("model_to_fine_tune", ["invalid-path-to-model", "."]) From 496b47bd3c19833d1c011ba2c47f8b0de2cae2d8 Mon Sep 17 00:00:00 2001 From: Joseph Juzl Date: Mon, 7 Dec 2020 14:44:43 +0100 Subject: [PATCH 5/7] wip --- examples/moodbot/config.yml | 4 ++-- rasa/nlu/model.py | 15 +++++++++++++-- 2 files changed, 15 insertions(+), 4 deletions(-) diff --git a/examples/moodbot/config.yml b/examples/moodbot/config.yml index 5b8a3ffbaa66..c4de25e2c9a1 100644 --- a/examples/moodbot/config.yml +++ b/examples/moodbot/config.yml @@ -6,11 +6,11 @@ pipeline: - name: "SpacyFeaturizer" - name: "DIETClassifier" entity_recognition: False - epochs: 50 + epochs: 1 policies: - name: TEDPolicy max_history: 5 - epochs: 100 + epochs: 1 - name: MemoizationPolicy - name: RulePolicy diff --git a/rasa/nlu/model.py b/rasa/nlu/model.py index 3c3b4798b0ac..5182024a7b98 100644 --- a/rasa/nlu/model.py +++ b/rasa/nlu/model.py @@ -6,6 +6,7 @@ from typing import Any, Dict, List, Optional, Text import rasa.nlu +# from rasa.nlu.registry import get_component_class from rasa.shared.exceptions import RasaException import rasa.shared.utils.io import rasa.utils.io @@ -335,6 +336,11 @@ def load( should_finetune=new_config is not None, ) + @staticmethod + def _get_default_value_for_component(name: Text, key: Text) -> Any: + # return get_component_class(name).defaults[key] + return 300 + @staticmethod def _update_epochs_from_new_config( model_metadata: Metadata, @@ -345,9 +351,14 @@ def _update_epochs_from_new_config( model_metadata.metadata["pipeline"], new_config["pipeline"] ): if EPOCHS in old_component_config: + new_epochs = new_component_config.get( + EPOCHS, + Interpreter._get_default_value_for_component( + old_component_config["class"], EPOCHS + ), + ) old_component_config[EPOCHS] = ceil( - new_component_config.get(EPOCHS, old_component_config[EPOCHS]) - * finetuning_epoch_fraction + new_epochs * finetuning_epoch_fraction ) @staticmethod From d3c2b679c301a3690c621f5fd603ca613ab5eac0 Mon Sep 17 00:00:00 2001 From: Joseph Juzl Date: Mon, 7 Dec 2020 16:44:47 +0100 Subject: [PATCH 6/7] Use default epochs if not provided --- examples/moodbot/config.yml | 4 ++-- rasa/nlu/model.py | 6 +++--- tests/test_train.py | 36 ++++++++++++++++++++++++++++++++++++ 3 files changed, 41 insertions(+), 5 deletions(-) diff --git a/examples/moodbot/config.yml b/examples/moodbot/config.yml index c4de25e2c9a1..5b8a3ffbaa66 100644 --- a/examples/moodbot/config.yml +++ b/examples/moodbot/config.yml @@ -6,11 +6,11 @@ pipeline: - name: "SpacyFeaturizer" - name: "DIETClassifier" entity_recognition: False - epochs: 1 + epochs: 50 policies: - name: TEDPolicy max_history: 5 - epochs: 1 + epochs: 100 - name: MemoizationPolicy - name: RulePolicy diff --git a/rasa/nlu/model.py b/rasa/nlu/model.py index 5182024a7b98..8eca9f6c963a 100644 --- a/rasa/nlu/model.py +++ b/rasa/nlu/model.py @@ -6,7 +6,6 @@ from typing import Any, Dict, List, Optional, Text import rasa.nlu -# from rasa.nlu.registry import get_component_class from rasa.shared.exceptions import RasaException import rasa.shared.utils.io import rasa.utils.io @@ -338,8 +337,9 @@ def load( @staticmethod def _get_default_value_for_component(name: Text, key: Text) -> Any: - # return get_component_class(name).defaults[key] - return 300 + from rasa.nlu.registry import get_component_class + + return get_component_class(name).defaults[key] @staticmethod def _update_epochs_from_new_config( diff --git a/tests/test_train.py b/tests/test_train.py index 6188905311fa..53e091b2c4e4 100644 --- a/tests/test_train.py +++ b/tests/test_train.py @@ -482,6 +482,42 @@ def test_model_finetuning_nlu( assert new_diet_metadata[EPOCHS] == 5 +def test_model_finetuning_nlu_with_default_epochs( + tmp_path: Path, + monkeypatch: MonkeyPatch, + default_domain_path: Text, + default_nlu_data: Text, + trained_moodbot_path: Text, +): + mocked_nlu_training = AsyncMock(return_value="") + monkeypatch.setattr(rasa.nlu, rasa.nlu.train.__name__, mocked_nlu_training) + + (tmp_path / "models").mkdir() + output = str(tmp_path / "models") + + # Providing a new config with no epochs will mean the default amount are used + # and then scaled by `finetuning_epoch_fraction`. + old_config = rasa.shared.utils.io.read_yaml_file("examples/moodbot/config.yml") + del old_config["pipeline"][-1][EPOCHS] + new_config_path = tmp_path / "new_config.yml" + rasa.shared.utils.io.write_yaml(old_config, new_config_path) + + train_nlu( + str(new_config_path), + "examples/moodbot/data/nlu.yml", + output=output, + model_to_finetune=trained_moodbot_path, + finetuning_epoch_fraction=0.5, + ) + + mocked_nlu_training.assert_called_once() + _, nlu_train_kwargs = mocked_nlu_training.call_args + model_to_finetune = nlu_train_kwargs["model_to_finetune"] + new_diet_metadata = model_to_finetune.model_metadata.metadata["pipeline"][-1] + assert new_diet_metadata["name"] == "DIETClassifier" + assert new_diet_metadata[EPOCHS] == DIETClassifier.defaults[EPOCHS] * 0.5 + + @pytest.mark.parametrize("model_to_fine_tune", ["invalid-path-to-model", "."]) def test_model_finetuning_with_invalid_model( tmp_path: Path, From d9f563af317030848bbdc20a2a82b3657eba48ec Mon Sep 17 00:00:00 2001 From: Joseph Juzl Date: Mon, 7 Dec 2020 19:18:53 +0100 Subject: [PATCH 7/7] Name change --- rasa/nlu/model.py | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/rasa/nlu/model.py b/rasa/nlu/model.py index 8eca9f6c963a..2765b4c01a83 100644 --- a/rasa/nlu/model.py +++ b/rasa/nlu/model.py @@ -323,7 +323,7 @@ def load( model_metadata = Metadata.load(model_dir) if new_config: - Interpreter._update_epochs_from_new_config( + Interpreter._update_metadata_epochs( model_metadata, new_config, finetuning_epoch_fraction ) @@ -342,11 +342,11 @@ def _get_default_value_for_component(name: Text, key: Text) -> Any: return get_component_class(name).defaults[key] @staticmethod - def _update_epochs_from_new_config( + def _update_metadata_epochs( model_metadata: Metadata, new_config: Optional[Dict] = None, finetuning_epoch_fraction: float = 1.0, - ): + ) -> Metadata: for old_component_config, new_component_config in zip( model_metadata.metadata["pipeline"], new_config["pipeline"] ): @@ -360,6 +360,7 @@ def _update_epochs_from_new_config( old_component_config[EPOCHS] = ceil( new_epochs * finetuning_epoch_fraction ) + return model_metadata @staticmethod def create(