Pass copy of config to emloop training #37
Conversation
emloop/tests/cli/common_test.py
Outdated
| def __init__(self, config_str: str): | ||
| super().__init__(config_str) | ||
| config = yaml.load(config_str) | ||
| self.train = {'a': 'b'} |
There was a problem hiding this comment.
We assume that the data are always batched, so 'a': ['b'] is preferred here.
| @@ -239,3 +278,23 @@ def test_create_model(tmpdir): | |||
| restored_model = create_model(config=new_config, output_dir=tmpdir + '_restored', dataset=dataset, | |||
| restore_from=tmpdir) | |||
| assert isinstance(restored_model, DummyModelWithKwargs2) | |||
|
|
|||
|
|
|||
| def test_config_file_is_unchanged(tmpdir): | |||
There was a problem hiding this comment.
Please add link to the issue for which this is regression test.
emloop/tests/cli/common_test.py
Outdated
|
|
||
| assert config['dataset']['dataset_config'] == ['a', 'b', 'c'] | ||
| assert config['hooks'][0]['emloop.tests.cli.common_test.DummyConfigHook']['variables'] == ['a', 'b'] | ||
| assert config['model']['architecture']['model_config'] == ['a', 'b', 'c', 'd'] |
There was a problem hiding this comment.
You can just pass deepcopy() to the run() function and test that the entire dict has not changed.
emloop/tests/cli/common_test.py
Outdated
| dataset_config[1], dataset_config[0], dataset_config[2] | ||
|
|
||
| def train_stream(self): | ||
| yield self.train |
There was a problem hiding this comment.
You can yield the dict directly, no private variable needed.
emloop/tests/cli/common_test.py
Outdated
|
|
||
| def _configure_dataset(self, dataset_config: List[str], **kwargs): | ||
| dataset_config[0], dataset_config[1], dataset_config[2] = \ | ||
| dataset_config[1], dataset_config[0], dataset_config[2] |
There was a problem hiding this comment.
You can also change those directly in the constructor.
| self.change_config(**kwargs) | ||
|
|
||
| def change_config(self, variables: List[str], **kwargs): | ||
| variables[0], variables[1] = variables[1], variables[0] |
There was a problem hiding this comment.
I suggest moving this to the constructor. Also the self.kwargs are never used, no need to save them.
emloop/tests/cli/common_test.py
Outdated
| class DummyConfigModel(DummyModel): | ||
| """Dummy model which changes config.""" | ||
| def __init__(self, **kwargs): | ||
| self.kwargs = kwargs |
There was a problem hiding this comment.
never used, just remove this variable
|
|
||
| def _create_model(self, architecture: Mapping, **kwargs): | ||
| config = architecture['model_config'] | ||
| config[0], config[1], config[2], config[3] = config[1], config[0], config[3], config[2] |
There was a problem hiding this comment.
Change those directly in the constructor.
closes #31