Models repository refactored
Вариант рефакторинга репозитория моделей:
- Структура вынесена в json
- Вместо иерархической структуры - каждой модели или их группе присваиваются теги. По ним можно делать запросы, например: получить все линейные модели как repo.models_with_tag(tags=['linear'])
- Соответствие между моделью и стратегий вычисления задается в репозитории (можно будет убрать громоздкие словари из models).
- Введен набор меток, позволяющих задавать ограничения на расположение моделей в структуре цепочки.
- Убрал чрезмерно разросшийся ModelsTypesEnum и заменил его на строковое название модели из репозитоия.
? Возможно есть смысл сделать глобальный репозиторий-синглтон? Чтобы можно было его динамически расширять и модифицировать (например, добавлять новые атомизированные модели).
Created by: pep8speaks
Hello @nicl-nno! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:
- In the file
test/test_transform.py:
Line 69:72: W291 trailing whitespace Line 113:49: W291 trailing whitespace Line 156:53: W291 trailing whitespace Line 181:53: W291 trailing whitespace
Comment last updated at 2020-07-09 13:25:05 UTC
- In the file
7 7 from core.composer.node import NodeGenerator 8 8 from core.models.data import InputData, OutputData 9 9 from core.repository.dataset_types import DataTypesEnum 10 from core.repository.model_types_repository import ModelTypesIdsEnum 11 from core.repository.tasks import Task, Task, TaskTypesEnum, TsForecastingParams 10 from core.repository.tasks import Task, TaskTypesEnum, TsForecastingParams 11 from core.models.data import OutputData 12 from core.repository.model_types_repository import ModelTypesRepository 13 14 from core.repository.quality_metrics_repository import MetricsRepository, RegressionMetricsEnum 12 15 from core.utils import project_root 13 16 14 17 6 6 7 7 from core.composer.chain import Chain, as_nx_graph 8 8 from core.composer.node import PrimaryNode, SecondaryNode 9 from core.repository.model_types_repository import ModelGroupsIdsEnum, ModelTypesIdsEnum, ModelTypesRepository 9 from core.repository.model_types_repository import ModelTypesRepository 10 10 from core.repository.tasks import Task - Last updated by Elizaveta Lutsenko
- Last updated by Elizaveta Lutsenko
- Last updated by Elizaveta Lutsenko
- Last updated by Elizaveta Lutsenko
- Last updated by Elizaveta Lutsenko
114 116 115 117 return trained_model, tuned_params 116 118 117 def _convert_to_sklearn(self, model_type: ModelTypesIdsEnum): 119 def _convert_to_sklearn(self, model_type: str): 118 120 if model_type in self.__model_by_types.keys(): 119 121 return self.__model_by_types[model_type] 120 122 else: 121 123 raise ValueError(f'Impossible to obtain SKlearn strategy for {model_type}') 122 124 125 def _find_model_by_impl(self, impl): 126 for model, model_impl in self.__model_by_types.items(): 1 1 import numpy as np Created by: J3FALL
Отмечу этот скрипт на будущее как "потенциально требует рефакторинга". Все-таки, привычнее видеть такие около hardcode-штуки во внешних конфигах. Возможно, какой-нибудь продвинутный json-парсер для питона позволит там вынести эти параметры из скрипта, а также решит полностью или частично проблемы с его валидацией.
- Last updated by Elizaveta Lutsenko
- Last updated by Elizaveta Lutsenko
11 if isinstance(field_value, str): 12 return eval_field_str(field_value) 13 else: 14 return field_value 15 else: 16 return default 17 18 19 def eval_field_str(field_value) -> Union[List[DataTypesEnum], 20 List[TaskTypesEnum]]: 21 return eval(field_value) 22 23 24 def eval_strategy_str(field_value): 25 exec(f'from core.models.evaluation.evaluation import {field_value}') 26 return eval(field_value) 39 39 return _compatible_task_types[main_task_type] Created by: J3FALL
- В этом скрипте много неиспользуемых импортов.
- Функцию _initialise_repo я бы перенес ниже конструктора.
И, отвечая на вопрос про синглтон: я определенно не фанат синглтонов, но можно здесь добавить lazy-инициализацию - добавить приватное поле класса repo и записывать в него результат функции self._initialise_repo(repo_path) при первом вызове конструктора. А при повторном вызове конструктора - считывать это значения. Получается что-то около кэширования. Но если чтение и парсинг json-а не особо критично, то можно этого и не делать.
Created by: J3FALL
Общее предложение: мы отказались от enum-ов для типов моделей и alias-ы для моделей запрятаны в приватных полях, как картинке. Я предлагаю добавить простую утилитарную функцию, которая позволяет собрать по группам все основные модели из других фреймворков, которые мы поддерживаем, и вывести их в виде словаря/списка или просто в качестве print-а. Например, для Sklearn-а довольно понятно - тут мы явно используем их классы. Для TPOT-а - это уже эстиматоры - TPOTClassifier или TPOTRegressor.
В будущем, мне кажется, это будет (а) полезным для стороннего пользователя - ему будет понятнее, как в федоте sklearn(insert_another_ml_framework) модели "мапятся" на оригинальное api фреймворков - функции или классы, а также (б) позволит нам автоматически поддерживать в документации это соответствие (рассчитываю, что скоро мы интегрируем автоматическую генерацию документации на все наши функции).
7 7 from core.composer.node import NodeGenerator 8 8 from core.models.data import InputData, OutputData 9 9 from core.repository.dataset_types import DataTypesEnum 10 from core.repository.model_types_repository import ModelTypesIdsEnum 11 from core.repository.tasks import Task, Task, TaskTypesEnum, TsForecastingParams 10 from core.repository.tasks import Task, TaskTypesEnum, TsForecastingParams 11 from core.models.data import OutputData 12 from core.repository.model_types_repository import ModelTypesRepository 13 14 from core.repository.quality_metrics_repository import MetricsRepository, RegressionMetricsEnum 12 15 from core.utils import project_root 13 16 14 17 6 6 7 7 from core.composer.chain import Chain, as_nx_graph 8 8 from core.composer.node import PrimaryNode, SecondaryNode 9 from core.repository.model_types_repository import ModelGroupsIdsEnum, ModelTypesIdsEnum, ModelTypesRepository 9 from core.repository.model_types_repository import ModelTypesRepository 10 10 from core.repository.tasks import Task 114 116 115 117 return trained_model, tuned_params 116 118 117 def _convert_to_sklearn(self, model_type: ModelTypesIdsEnum): 119 def _convert_to_sklearn(self, model_type: str): 118 120 if model_type in self.__model_by_types.keys(): 119 121 return self.__model_by_types[model_type] 120 122 else: 121 123 raise ValueError(f'Impossible to obtain SKlearn strategy for {model_type}') 122 124 125 def _find_model_by_impl(self, impl): 126 for model, model_impl in self.__model_by_types.items(): Разбил. Но теперь пришлось добавить в json ещё и namespace помимо самой стратегии.
1 { 2 "metadata": { Внесу в документацию в https://github.com/nccr-itmo/FEDOT/issues/126
39 39 return _compatible_task_types[main_task_type] 1, 2: Ты видимо тут про файл model_types_repository. Поправил.
Да, можно так, сделал. В целом не оч. критично, но для большей гибкости использования не помешает.
Добавил свойство implementation_info в каждую стратегию. Оно выводит строку информации о реализации модели.
Но это удобно только для sklearn моделей, с остальными нужно отдельный словарь создавать. Показал на примере statmodels. Выглядит как-то громоздко.
В utils внёс метод print_models_info, которые печатает список моделей и их описаний реализаций.
11 if isinstance(field_value, str): 12 return eval_field_str(field_value) 13 else: 14 return field_value 15 else: 16 return default 17 18 19 def eval_field_str(field_value) -> Union[List[DataTypesEnum], 20 List[TaskTypesEnum]]: 21 return eval(field_value) 22 23 24 def eval_strategy_str(field_value): 25 exec(f'from core.models.evaluation.evaluation import {field_value}') 26 return eval(field_value) Добавил тесты в test_repository.py
114 116 115 117 return trained_model, tuned_params 116 118 117 def _convert_to_sklearn(self, model_type: ModelTypesIdsEnum): 119 def _convert_to_sklearn(self, model_type: str): 118 120 if model_type in self.__model_by_types.keys(): 119 121 return self.__model_by_types[model_type] 120 122 else: 121 123 raise ValueError(f'Impossible to obtain SKlearn strategy for {model_type}') 122 124 125 def _find_model_by_impl(self, impl): 126 for model, model_impl in self.__model_by_types.items():
