Refactor pipeline generation as builder #509 #597

gkirgizov · 2022-03-15T16:01:03Z

Refactor Assumptions Pipeline building process

Primary changes:

Extract TaskAssumptions as a kind of data-class containing actual readable assumptions.
Extract PreprocessingBuilder which is almost independent of the task. Handles gaps and categorical data.
PrimaryAssumptionsBuilder handles case of InputData, logger and available operations.
MultiModalData assumptions case is reduced to InputData case. Can now be easily customized if needed. Handled by MultiModalAssumptionsBuilder. In principle, can be also extended to handle available operations mechanics.
Actual pipeline building is handled by PipelineBuilder underneath. It's independent, all other logic is built on top. So it can be used elsewhere for somewhat shorter construction of pipelines.

pep8speaks · 2022-03-15T16:01:08Z

Hello @gkirgizov! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:

There are currently no PEP 8 issues detected in this Pull Request. Cheers! 🍻

Comment last updated at 2022-03-22 11:03:46 UTC

fedot/api/api_utils/assumptions_builder.py

fedot/api/api_utils/pipeline_builder.py

fedot/api/api_utils/assumptions_builder.py

fedot/api/api_utils/pipeline_builder.py

test/unit/api/test_pipeline_builder.py

Dreamlone

Я бы предложил все .py файлы отвечающие за построение начальных приближений вынести в отдельный модуль в api, - что-то вроде такого

fedot/api/api_utils/legacy_assumptions.py

fedot/api/api_utils/initial_assumptions.py

fedot/api/api_utils/assumptions_builder.py

fedot/api/api_utils/pipeline_builder.py

fedot/core/data/data_preprocessing.py

Dreamlone · 2022-03-16T06:53:22Z

test/unit/api/test_assumption_builder.py

+def pipeline_contains_one(pipeline: Pipeline, op: OpT) -> bool:
+    def is_the_op(node: Node):


Лучше в нотации писать, что op operation_name: str

Это же относится и к другим сокращениям) Расширь их тоже плиз

test/unit/api/test_assumption_builder.py

Dreamlone · 2022-03-16T06:55:45Z

test/unit/api/test_assumption_builder.py

+def get_suitable_operations_for_task(task_type: TaskTypesEnum, data_type: DataTypesEnum, repo='model'):
+    return OperationTypesRepository(repo).suitable_operation(task_type=task_type, data_type=data_type)[0]


Мб тут можно переиспользовать operations_for_task из fedot/api/help.py, или get_operations_for_task из fedot/core/repository/operation_types_repository.py

В этих методах не указывается data_type параметр, что, вероятно, повлияет на результат. Да и все-таки это прямое использование публичного api репозитория, не вижу чего-то криминального в этом. Но, конечно, могу чего-то не знать.

Ок, убедил
Ещё тут опишусь: пожалуйста, не помечай треды как решенные, мне так тяжело сообразить что в итоге было изменено и когда (могу запутаться) - я сам их помечу как resolved, когда буду просматривать код второй раз или когда буду лазить по коммитам, оставленным в комментариях

А, ок, без проблем. Отмечал, чтобы самому не запутаться, что поправил, а что нет.

А, ок, без проблем. Отмечал, чтобы самому не запутаться, что поправил, а что нет.

Ты простой оставляй коммент по каждому, тогда будет легко понять что done а что нет.

gkirgizov · 2022-03-16T12:56:45Z

Я бы предложил все .py файлы отвечающие за построение начальных приближений вынести в отдельный модуль в api, - что-то вроде такого

Насчет этого -- сейчас остался только assumptions_builder.py & pipeline_builder.py. (Я дропнул легаси и initial_assumptions.) Второй по логике не относится к assumptions, а как раз скорее api_utils. Выносить же один файл, наверное, нет смысла. Согласны?

nicl-nno · 2022-03-16T13:18:09Z

Второй по логике не относится к assumptions, а как раз скорее api_utils

Может PIpelineBuilder его отправить вообще в Pipelines? Он же не только в API может быть применен

gkirgizov · 2022-03-16T13:18:58Z

Второй по логике не относится к assumptions, а как раз скорее api_utils

Может PIpelineBuilder его отправить вообще в Pipelines? Он же не только в API может быть применен

Вот да, мне тоже кажется, что ему где-то еще место. Перенесу в Pipelines.

codecov · 2022-03-16T13:21:40Z

Codecov Report

Merging #597 (95a8175) into master (30945d6) will increase coverage by 0.41%.
The diff coverage is 93.75%.

❗ Current head 95a8175 differs from pull request most recent head b97359a. Consider uploading reports for the commit b97359a to get more accurate results

@@            Coverage Diff             @@
##           master     #597      +/-   ##
==========================================
+ Coverage   86.23%   86.65%   +0.41%     
==========================================
  Files         145      149       +4     
  Lines       10959    11080     +121     
==========================================
+ Hits         9451     9601     +150     
+ Misses       1508     1479      -29

Impacted Files	Coverage Δ
...edot/api/api_utils/assumptions/task_assumptions.py	`80.76% <80.76%> (ø)`
...dot/api/api_utils/assumptions/operations_filter.py	`94.73% <94.73%> (ø)`
...t/api/api_utils/assumptions/assumptions_builder.py	`95.40% <95.40%> (ø)`
fedot/core/pipelines/pipeline_builder.py	`97.26% <97.26%> (ø)`
fedot/api/api_utils/api_composer.py	`78.28% <100.00%> (ø)`
...api/api_utils/assumptions/preprocessing_builder.py	`100.00% <100.00%> (ø)`
fedot/api/main.py	`78.73% <100.00%> (+0.12%)`	⬆️
...re/serializers/coders/opt_history_serialization.py	`36.36% <0.00%> (-3.64%)`	⬇️
fedot/core/optimisers/graph.py	`90.98% <0.00%> (-0.62%)`	⬇️
fedot/core/pipelines/template.py	`85.95% <0.00%> (-0.42%)`	⬇️
... and 23 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 30945d6...b97359a. Read the comment docs.

fedot/api/api_utils/assumptions_builder.py

Dreamlone · 2022-03-16T14:49:23Z

Насчет этого -- сейчас остался только assumptions_builder.py & pipeline_builder.py. (Я дропнул легаси и initial_assumptions.) Второй по логике не относится к assumptions, а как раз скорее api_utils. Выносить же один файл, наверное, нет смысла. Согласны?

В данном случае с вашими с Колей комментариями согласен. Просто на будущее - я бы хотел взять курс на растаскивание api_utils на отдельные субмодули в рамках большого и структурированного API в ближайшее время (понятно, что не к этому PR). Потому что API заметно усложнился с того момента, как появился "api_utils". Можно будет его потом как-то более прозрачно структурировать, чем надежный подход "всё в кучу"

fedot/api/api_utils/api_composer.py

Dreamlone · 2022-03-16T14:54:31Z

fedot/api/api_utils/assumptions_builder.py

+
+    @staticmethod
+    def for_task(task: Task):
+        task2assumptions = {


Давай без цифр лучше обойдемся без необходимости)

это ж сокращение в смысле task_to_assumptions
но ок, переименую

Давай я ещё поиграю в душного деда - вместо task_to_assumptions лучше подошло бы assumption_by_task, потому что ниже ты получаешь подходящего строителя именно по имеющейся задаче

fedot/api/api_utils/assumptions_builder.py

fedot/api/api_utils/api_composer.py

fedot/api/api_utils/assumptions_builder.py

test/unit/api/test_assumption_builder.py

test/unit/pipelines/test_pipeline_builder.py

fedot/api/api_utils/api_composer.py

fedot/api/api_utils/assumptions/preprocessing_builder.py

test/unit/pipelines/test_pipeline_builder.py

test/unit/models/test_custom_model_introduction.py

test/unit/pipelines/test_pipeline_builder.py

fedot/core/pipelines/pipeline_builder.py

fedot/api/api_utils/assumptions/task_assumptions.py

fedot/api/api_utils/api_composer.py

Dreamlone

Всё кул, осталось сделать rebase на мастер ветку и можно мержить

Dreamlone · 2022-03-22T09:49:00Z

А претензии PEP8 можешь поправить ещё плиз?

…ine from Nodes

…me methods.

Extract TaskAssumptions as a kind of data-class containing actual readable assumptions. Extract PreprocessingBuilder which is almost independent of the task. Handles gaps and categorical data. PrimaryAssumptionsBuilder handles case of InputData, logger and available operations. MultiModalData assumptions case is reduced to InputData case. Can now be easily customized if needed. Also now handles available operations. Actual Pipeline building is handled by PipelineBuilder underneath.

For case when SupplementaryData has no computed column_types

…branch

Also fix review comments in builders, rename interfaces

This reverts commit 1265b35

And drop initial_assumptions.py.

Also fix tests according to review comments. Restructure them a bit.

This also changes semantics of PipelineBuilder from 'one-shot' to reusable, as they now return copies. This change is required for simplifying TaskAssumptions.

- In case when no PipelineBuilder satisfies OperationsFilter -- return only a single fallback alternative instead of many. - Add dependency on OperationTypesRepository to TaskAssumptions to build valid fallback pipeline in TSForecastingAssumptions.

gkirgizov requested a review from Dreamlone March 15, 2022 16:01

gkirgizov self-assigned this Mar 15, 2022

nicl-nno requested changes Mar 15, 2022

View reviewed changes

Dreamlone requested changes Mar 16, 2022

View reviewed changes

nicl-nno reviewed Mar 16, 2022

View reviewed changes

fedot/api/api_utils/assumptions_builder.py Outdated Show resolved Hide resolved

Dreamlone reviewed Mar 16, 2022

View reviewed changes

gkirgizov requested review from Dreamlone and nicl-nno March 17, 2022 13:32

nicl-nno approved these changes Mar 17, 2022

View reviewed changes

Dreamlone requested changes Mar 17, 2022

View reviewed changes

gkirgizov requested a review from Dreamlone March 18, 2022 07:53

Dreamlone requested changes Mar 18, 2022

View reviewed changes

gkirgizov force-pushed the 509-pipeline-builder branch from aa9f3a5 to ebe1c26 Compare March 18, 2022 14:39

Dreamlone reviewed Mar 21, 2022

View reviewed changes

gkirgizov force-pushed the 509-pipeline-builder branch from ebe1c26 to 95a8175 Compare March 21, 2022 14:13

gkirgizov requested a review from Dreamlone March 21, 2022 15:05

Dreamlone requested changes Mar 21, 2022

View reviewed changes

gkirgizov force-pushed the 509-pipeline-builder branch from 95a8175 to 17b529b Compare March 21, 2022 17:04

Dreamlone approved these changes Mar 22, 2022

View reviewed changes

gkirgizov added 4 commits March 22, 2022 14:03

Add initial version of PipelineBuilder for incremental build of Pipel…

9c6404e

…ine from Nodes

Use PipelineBuilder instead of manual in initial_assumptions.py in so…

0a0a839

…me methods.

Substitute ApiInitialAssumptions for new impl, preserve legacy one

0eeda40

gkirgizov added 13 commits March 22, 2022 14:03

Add tests for PipelineBuilder

339795b

Add tests for AssumptionsBuilder

21492e5

Small fix in data_has_categorical_features

a9b8eea

For case when SupplementaryData has no computed column_types

Add more assumptions pipelines according to latest changes in master …

8d38442

…branch

Fix PEP8 issues. Delete legacy_assumptions.py.

56a7c1e

Also fix review comments in builders, rename interfaces

Revert "Small fix in data_has_categorical_features"

5b862ea

This reverts commit 1265b35

Substitute usages of ApiInitialAssumptions for AssumptionsBuilder.

e6f0f55

And drop initial_assumptions.py.

Drop type alias OpT in favor of just 'str'

21cc918

Move PipelineBuilder to core.pipelines, move tests

41228be

Also fix tests according to review comments. Restructure them a bit.

Allow merging PipelineBuilders

b051e96

This also changes semantics of PipelineBuilder from 'one-shot' to reusable, as they now return copies. This change is required for simplifying TaskAssumptions.

Simplify TaskAssumptions based on dict of preconstructed builders

cc01dc3

Restructure files & classes

0178fb9

gkirgizov force-pushed the 509-pipeline-builder branch from 17b529b to b97359a Compare March 22, 2022 11:03

gkirgizov merged commit e0ba2f0 into master Mar 22, 2022

nicl-nno deleted the 509-pipeline-builder branch November 23, 2022 13:41

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Refactor pipeline generation as builder #509 #597

Refactor pipeline generation as builder #509 #597

gkirgizov commented Mar 15, 2022

pep8speaks commented Mar 15, 2022 •

edited

Loading

Dreamlone left a comment

Dreamlone Mar 16, 2022

Dreamlone Mar 17, 2022

Dreamlone Mar 16, 2022

gkirgizov Mar 16, 2022

Dreamlone Mar 16, 2022

gkirgizov Mar 16, 2022

nicl-nno Mar 16, 2022

gkirgizov commented Mar 16, 2022 •

edited

Loading

nicl-nno commented Mar 16, 2022

gkirgizov commented Mar 16, 2022

codecov bot commented Mar 16, 2022 •

edited

Loading

Dreamlone commented Mar 16, 2022

Dreamlone Mar 16, 2022

gkirgizov Mar 16, 2022

Dreamlone Mar 17, 2022 •

edited

Loading

Dreamlone left a comment

Dreamlone commented Mar 22, 2022

		def pipeline_contains_one(pipeline: Pipeline, op: OpT) -> bool:
		def is_the_op(node: Node):

		def get_suitable_operations_for_task(task_type: TaskTypesEnum, data_type: DataTypesEnum, repo='model'):
		return OperationTypesRepository(repo).suitable_operation(task_type=task_type, data_type=data_type)[0]

Refactor pipeline generation as builder #509 #597

Refactor pipeline generation as builder #509 #597

Conversation

gkirgizov commented Mar 15, 2022

pep8speaks commented Mar 15, 2022 • edited Loading

Comment last updated at 2022-03-22 11:03:46 UTC

Dreamlone left a 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

gkirgizov commented Mar 16, 2022 • edited Loading

nicl-nno commented Mar 16, 2022

gkirgizov commented Mar 16, 2022

codecov bot commented Mar 16, 2022 • edited Loading

Codecov Report

Dreamlone commented Mar 16, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Dreamlone Mar 17, 2022 • edited Loading

Choose a reason for hiding this comment

Dreamlone left a comment

Choose a reason for hiding this comment

Dreamlone commented Mar 22, 2022

pep8speaks commented Mar 15, 2022 •

edited

Loading

gkirgizov commented Mar 16, 2022 •

edited

Loading

codecov bot commented Mar 16, 2022 •

edited

Loading

Dreamlone Mar 17, 2022 •

edited

Loading