Parallel population processing

valer1435 commented 1 year ago

И какое поведение будет у новой реализации, если n_jobs=1? Будет вызываться Parallel с n_jobs=1? Вроде был отдельный Dispatcher под это

kasyanovse commented 1 year ago

И какое поведение будет у новой реализации, если n_jobs=1? Будет вызываться Parallel с n_jobs=1? Вроде был отдельный Dispatcher под это

Проблемой Parallel с n_jobs=1 не является, потому что в таком случае код выполняется в том же потоке, в котором он вызывается. Проблему с Dispatcher буду фиксить если такой вариант распараллеливания зайдет.

maypink commented 1 year ago

Насчет ReproductionController: в моем понимании, это класс, ответственный за один эволюционный цикл. да, как ты правильно подметил, сейчас он есть только в EvoGraphOptimizer.

Но GOLEM позиционируется, как универсальное решение, поэтому инфраструктура должна быть реализована по своего рода "строительным блокам". В таком случае, ReproductionController очень удобен, так как он делает один эволюционный цикл атомарным для оптимизатора и + вынесение этой логики в отдельный класс делает EvoGraphOptimizer чище -- в нем находятся только те методы, за которые должен нести ответственность непосредственно оптимизатор.

Also, здесь Гриша писал про мотивацию делать ReproductionController

kasyanovse commented 1 year ago

Я не против вернуть ReproductionController. С учетом имеющихся правок такое решение приведет к почти полному дублированию функционала, используемого для набора начальной популяции. Если процесс воспроизводства поколения сейчас реализован нормально и не требует существенных корректировок, то как воссоздать ReproductionController? Да и в целом, необходимость полного дублирования говорит о том, что отдельный модуль, скорее всего, не нужен.

Also, здесь Гриша писал про мотивацию делать ReproductionController

Новый код удовлетворяет задаче, решаемой здесь, и в то же время смягчает недостатки подхода, предложенного там же. Единственное, что отладка не такая удобная, но у меня есть пара предложений как это можно решить, не теряя в производительности. Ну и оценить корректность реализации я могу лишь с натяжкой.

Небольшое резюме (субъективное): Если вернуть ReproductionController:

Плюсы
1. Является универсальным строительным блоком
Минусы
1. Большая часть функционала (логического) дублирует аналогичный функционал в EvoGraphOptimizer (набор начального поколения)
2. Не востребован как универсальный блок на текущий момент
3. Логически он отнесен к operators, однако:
  1. В EvoGraphOptimizer он не сохраняется в EvoGraphOptimizer.operators (для него не обновляются requirements)
  2. Он сам использует другие operators внутри себя
  3. Его интерфейс отличается от других operators

Если не возвращать ReproductionController:

Плюсы
1. Меньше кода, меньше проблем, проще структура
2. Меньше дублирования кода или функциональности
Минусы
1. Отсутствие универсального строительного блока

Свлю точку зрения я пояснил (надеюсь))), однако я не погружен в GOLEM, не знаю как будет оптимальней с точки зрения развития библиотеки. Сделаю как ты посчитаешь правильным)

pep8speaks commented 1 year ago

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

In the file golem/core/optimisers/genetic/gp_optimizer.py:

Line 5:1: F401 'golem.core.optimisers.genetic.operators.crossover.Crossover' imported but unused Line 8:1: F401 'golem.core.optimisers.genetic.operators.mutation.Mutation' imported but unused

In the file golem/core/optimisers/genetic/operators/crossover.py:

Line 99:121: E501 line too long (121 > 120 characters)

In the file golem/core/optimisers/genetic/operators/mutation.py:

Line 176:121: E501 line too long (132 > 120 characters)

In the file golem/core/optimisers/genetic/operators/node.py:

Line 3:1: F401 'itertools.chain' imported but unused Line 8:1: F401 'golem.core.optimisers.graph.OptGraph' imported but unused Line 162:32: E127 continuation line over-indented for visual indent Line 163:32: E127 continuation line over-indented for visual indent Line 223:1: E302 expected 2 blank lines, found 1 Line 228:5: F841 local variable 'checked_type' is assigned to but never used Line 240:30: F541 f-string is missing placeholders

In the file golem/core/optimisers/genetic/operators/reproduction.py:

Line 2:1: F401 'copy.copy' imported but unused Line 3:1: F401 'dataclasses.dataclass' imported but unused Line 4:1: F401 'enum.Enum' imported but unused Line 5:1: F401 'itertools.chain' imported but unused Line 6:1: F401 'multiprocessing.managers.DictProxy' imported but unused Line 8:1: F401 'queue.Queue' imported but unused Line 9:1: F401 'random.sample' imported but unused Line 10:1: F401 'typing.Union' imported but unused Line 17:1: F401 'golem.core.optimisers.fitness.Fitness' imported but unused Line 19:1: F401 'golem.core.optimisers.genetic.operators.crossover.CrossoverTypesEnum' imported but unused Line 20:1: F401 'golem.core.optimisers.genetic.operators.mutation.MutationType' imported but unused Line 25:1: F401 'golem.core.optimisers.graph.OptGraph' imported but unused Line 33:1: E303 too many blank lines (3) Line 79:121: E501 line too long (122 > 120 characters) Line 170:45: F821 undefined name 'ReproducerWorkerTask' Line 171:43: F821 undefined name 'ReproducerWorkerTask' Line 214:27: F541 f-string is missing placeholders

In the file golem/core/optimisers/populational_optimizer.py:

Line 7:1: F401 'golem.core.optimisers.genetic.evaluation.MultiprocessingDispatcher' imported but unused

In the file test/unit/optimizers/gp_operators/test_genetic_pipelines.py:

Line 38:51: E251 unexpected spaces around keyword / parameter equals Line 38:53: E251 unexpected spaces around keyword / parameter equals Line 40:14: E127 continuation line over-indented for visual indent Line 56:1: E302 expected 2 blank lines, found 1 Line 157:5: F841 local variable '_individuals_output_count' is assigned to but never used Line 170:5: E303 too many blank lines (2) Line 207:1: F811 redefinition of unused 'test_genetic_node_with_nonfailed_task' from line 141 Line 219:5: F841 local variable 'final_tasks' is assigned to but never used

In the file test/unit/optimizers/gp_operators/test_reproduction_controller.py:

Line 11:1: F401 'golem.core.optimisers.genetic.operators.crossover.Crossover' imported but unused Line 13:1: F401 'golem.core.optimisers.genetic.operators.mutation.Mutation' imported but unused

Comment last updated at 2023-11-16 14:27:58 UTC

aim-pep8-bot commented 1 year ago

Hello @kasyanovse! 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! :beers:

Comment last updated at 2023-11-24 14:59:31 UTC

maypink commented 11 months ago

Я не против вернуть ReproductionController. С учетом имеющихся правок такое решение приведет к почти полному дублированию функционала, используемого для набора начальной популяции. Если процесс воспроизводства поколения сейчас реализован нормально и не требует существенных корректировок, то как воссоздать ReproductionController? Да и в целом, необходимость полного дублирования говорит о том, что отдельный модуль, скорее всего, не нужен.

Also, здесь Гриша писал про мотивацию делать ReproductionController

Новый код удовлетворяет задаче, решаемой здесь, и в то же время смягчает недостатки подхода, предложенного там же. Единственное, что отладка не такая удобная, но у меня есть пара предложений как это можно решить, не теряя в производительности. Ну и оценить корректность реализации я могу лишь с натяжкой.

Небольшое резюме (субъективное): Если вернуть ReproductionController:

Плюсы

Является универсальным строительным блоком

Минусы

Большая часть функционала (логического) дублирует аналогичный функционал в EvoGraphOptimizer (набор начального поколения)

Не востребован как универсальный блок на текущий момент

Логически он отнесен к operators, однако:

В EvoGraphOptimizer он не сохраняется в EvoGraphOptimizer.operators (для него не обновляются requirements)

Он сам использует другие operators внутри себя

Его интерфейс отличается от других operators

Если не возвращать ReproductionController:

Плюсы

Меньше кода, меньше проблем, проще структура

Меньше дублирования кода или функциональности

Минусы

Отсутствие универсального строительного блока

Свлю точку зрения я пояснил (надеюсь))), однако я не погружен в GOLEM, не знаю как будет оптимальней с точки зрения развития библиотеки. Сделаю как ты посчитаешь правильным)

я только за мультипроцессинг здесь, но надо понимать, насколько большой выигрыш мы заимеем, если откажемся от ООП в виде ReproductionController. Как мимнимум, предлагаю померить ускорение с изменениями в этом ПР и без, и, возможно, с последовательным распараллеливанием дважды для мутации и оценки, чтобы не терять ни в чем архитектурно. Если получится, что при однократном открывании потока мы действительно станем намного быстрее, тогда откажемся от репродукции в отдельном классе.

kasyanovse commented 11 months ago

я только за мультипроцессинг здесь, но надо понимать, насколько большой выигрыш мы заимеем, если откажемся от ООП в виде ReproductionController. Как мимнимум, предлагаю померить ускорение с изменениями в этом ПР и без, и, возможно, с последовательным распараллеливанием дважды для мутации и оценки, чтобы не терять ни в чем архитектурно. Если получится, что при однократном открывании потока мы действительно станем намного быстрее, тогда откажемся от репродукции в отдельном классе.

Само по себе двойное распараллеливание не даст сильной просадки по времени в обычном случае (недавно прочитал). joblib по умолчанию не глушит потоки после окончания расчетов, а ждет новых заданий.

Насчет возврата ReproductionController и старой структуры есть варианты:

Оставить ReproductionController, сделать в нем мутации параллельно. Внутри ReproductionController мутации вызываются в цикле и проблема одного тупящего потока (когда последний поток что-то долго считает после того, как завершились остальные) может значительно снизить эффект от распараллеливания. Плюс к тому некоторый функционал останется нераспараллеленым.
Переписать ReproductionController по типу того, что сейчас сделано в этом PR. В таком случае логика ReproductionController сократится до пары десятков строк. Насколько целесообразно оставлять класс, который будет частично дублировать функционал, реализованный в EvoGraphOptimizer._extend_population и состоять из пары десятков строк? Проблема одного тупящего потока будет в два раза актуальнее, чем при текущей реализации, так как многопоток будет разворачиваться два раза.
Отказаться от ReproductionController, а его функционал частично объединить с EvoGraphOptimizer._extend_population и выделить в отдельную функцию. Что и было сделано. При этом:
1. Упрощение
  1. На один модуль меньше, на один класс меньше.
  2. EvoGraphOptimizer._extend_population упростилась.
2. Ускорение за счет:
  1. Объединения двух из трех вложенных циклов при мутациях в поколениях (три цикла - один на уровне ReproductionController, внутри него мутации в цикле по поколению, внутри каждой мутации еще один цикл)
  2. Распараллеливания
  3. При решении #205 и выносе цикла из мутаций на уровень поколения, будет ускорение за счет уменьшения длительности распараллеленного функционала.
В последнем случае речь идет не только о скорости распараллеливания, но и о том, что сама структура EvoGraphOptimizer в этом PR проще (ИМХО) и годится для решения #205 (а это дополнительное ускорение).

codecov-commenter commented 11 months ago

Codecov Report

Attention: 169 lines in your changes are missing coverage. Please review.

Comparison is base (a46412c) 71.96% compared to head (8987b87) 70.13%.

Files	Patch %	Lines
.../core/optimisers/genetic/operators/reproduction.py	54.38%	104 Missing :warning:
golem/core/optimisers/genetic/operators/node.py	0.00%	51 Missing :warning:
...lem/core/optimisers/genetic/operators/crossover.py	56.25%	7 Missing :warning:
...olem/core/optimisers/genetic/operators/mutation.py	65.00%	7 Missing :warning:

Additional details and impacted files

```diff @@ Coverage Diff @@ ## main #199 +/- ## ========================================== - Coverage 71.96% 70.13% -1.84% ========================================== Files 136 137 +1 Lines 8130 8365 +235 ========================================== + Hits 5851 5867 +16 - Misses 2279 2498 +219 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

nicl-nno commented 10 months ago

FYI:

https://github.com/aimclub/GEFEST/pull/70 Вот тут этот PR обсуждался, мб что-то полезное можно сразу учесть. Не обязательно все сразу.

kasyanovse commented 10 months ago

Move to https://github.com/aimclub/GOLEM/pull/247

aimclub / GOLEM