Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactor API: ComposerRequirements and ComposerBuilder #852

Merged
merged 50 commits into from
Sep 13, 2022
Merged

Conversation

gkirgizov
Copy link
Collaborator

@gkirgizov gkirgizov commented Aug 30, 2022

Reorganize ComposerRequirements & GraphOptimiserParameters to be more intuitive and logical.

Main changes

  • Composer requirements now contain infrastructural (algorithm-independent, like stop condition, performance etc) and graph-composing options
  • GPGraphOptimiserParameters now contain only evolutionary-operator-related options
  • Updated tests & examples that used ComposerBuilder
  • Set with_history in ComposerBuilder by default
  • Move some implicit evolutionary parameters from operators to GPGraphOptimzierParameters

So, all in all, I hope, it's now a bit more straightforward to define parameters in fedot. Mainly users should look for PipelineComposerRequirements & ComposerBuilder methods.

@gkirgizov gkirgizov added refactoring api Anything related to user-facing interfaces & parameter passing labels Aug 30, 2022
@codecov
Copy link

codecov bot commented Aug 30, 2022

Codecov Report

Merging #852 (44bc34e) into master (3db5464) will increase coverage by 0.24%.
The diff coverage is 95.51%.

@@            Coverage Diff             @@
##           master     #852      +/-   ##
==========================================
+ Coverage   87.72%   87.97%   +0.24%     
==========================================
  Files         195      196       +1     
  Lines       13356    13345      -11     
==========================================
+ Hits        11717    11740      +23     
+ Misses       1639     1605      -34     
Impacted Files Coverage Δ
fedot/api/api_utils/params.py 92.00% <ø> (ø)
fedot/core/composer/gp_composer/gp_composer.py 95.12% <ø> (ø)
.../core/optimisers/gp_comp/parameters/graph_depth.py 77.27% <50.00%> (-13.64%) ⬇️
...ore/optimisers/gp_comp/operators/regularization.py 55.81% <70.00%> (-2.33%) ⬇️
fedot/api/api_utils/metrics.py 83.33% <85.71%> (+3.33%) ⬆️
...dot/core/optimisers/gp_comp/operators/crossover.py 90.58% <85.71%> (-1.55%) ⬇️
fedot/core/optimisers/gp_comp/operators/elitism.py 96.66% <87.50%> (-0.56%) ⬇️
...t/core/optimisers/gp_comp/operators/inheritance.py 96.29% <88.88%> (-3.71%) ⬇️
...timisers/gp_comp/pipeline_composer_requirements.py 94.44% <92.85%> (-5.56%) ⬇️
...edot/core/optimisers/gp_comp/operators/operator.py 90.90% <93.75%> (+0.90%) ⬆️
... and 33 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@aPovidlo aPovidlo self-requested a review August 31, 2022 08:05
@gkirgizov gkirgizov mentioned this pull request Sep 1, 2022
@pep8speaks
Copy link

pep8speaks commented Sep 5, 2022

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

Line 2:1: F401 'typing.TYPE_CHECKING' imported but unused

Line 1:1: F401 'typing.TYPE_CHECKING' imported but unused

Comment last updated at 2022-09-13 08:15:25 UTC

@@ -15,35 +18,43 @@ class MutationStrengthEnum(Enum):

@dataclass
class PipelineComposerRequirements(ComposerRequirements):
"""
Dataclass is for defining the requirements for composition process of genetic programming composer
"""Defines algorithm-specific parameters of evolutionary optimizer.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

algorithm-specific parameters

Не очень понятно, про какой алгоритм речь.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

подредактировал f0f8467

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"Parameters of evolutionary optimizer that define features of the evolutionary algorithm
and restrictions on the graph composition process"

Я думаю тут стоит сформулировать отличия PipelineComposerRequirements от OptimiserRequirements, чтобы не было путаницы. Даже если сейчас не будет полностью разделять - стоит пояснить, почему это вообще два разных класса.

Comment on lines 24 to 31
:param crossover_prob: crossover probability (the chance that two chromosomes exchange some of their parts)
:param mutation_prob: mutation probability
:param mutation_strength: strength of mutation in tree (using in certain mutation types)
:param max_pipeline_fit_time: time constraint for operation fitting (minutes)
:param start_depth: start value of tree depth
:param validation_blocks: number of validation blocks for time series validation
:param n_jobs: num of n_jobs
:param show_progress: bool indicating whether to show progress using tqdm or not
:param collect_intermediate_metric: save metrics for intermediate (non-root) nodes in pipeline
:param keep_n_best: Number of the best individuals of previous generation to keep in next generation.

Population and graph parameters, possibly adaptive:
:param offspring_rate: offspring rate used on next population
:param pop_size: initial population size; if unspecified, default value is used
:param max_pop_size: maximum population size; optional, if unspecified, then population size is unbound
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

А должны ли ту быть специфичные именно для эволюции штуки? Предполагал что в req к композеру будет как раз-то, что от оптимизатора не зависит (max_depth, например)

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ты про crossover_prob, mutation_prob, mutation_strength ? Согласен, тоже хотел их перенести в GraphOptimizerParameters, но наткнулся на то, что придется побольше кода менять -- тк в процессе эволюции именно requirements адаптивно модифицируются. Это чисто техническая проблемка -- подумаю еще, как это получше организовать. В этом PR не хотелось много времени на это тратить. Тут более простая реорганизация, которая особых изменений в коде оптимизатора не потребовала. Предлагаю это в другом PR устроить, тут уже хватает изменений.

И в целом различие между композером и оптимизтором особо не определено. Все параметры так или иначе параметры оптимизатора. Тут скорее различие в том, что какие-то относятся именно к эволюционному алогритму, а какие-то -- к графовым характеристикам. Если концептуальное различие между оптимизатором и композером именно в этом -- тогда понимаю.

n_jobs, start_depth, show_progress:
 PipelineComposerRequirements -> ComposerRequirements
duplicated max_pipeline_fit_time: drop from pipeline_composer_requirements.py
…rams

Move most of GraphOptimizerParams into ComposerRequirements.
Make the former a dataclass
with_auto_depth_configuration -> adaptive_depth
depth_increase_step -> adaptive_depth_max_stagnation
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api Anything related to user-facing interfaces & parameter passing refactoring
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants