aimclub / GOLEM

Graph Optimiser for Learning and Evolution of Models
https://thegolem.readthedocs.io
BSD 3-Clause "New" or "Revised" License
60 stars 7 forks source link

Tuner refactoring #27

Closed valer1435 closed 1 year ago

valer1435 commented 2 years ago

Now tuner uses metrics that not compatible with composer metrics. Also it has a monolith structure.

What should be done:

UPD (from @gkirgizov) Additional subtasks:

Regarding 1st and 3rd points by Valera, possibly Objective class is useful here.

nicl-nno commented 2 years ago

А что если на каком-то уровне вообще обобщить тюнер на OptGraph-ы? Не привязываясь к пайплайнам как таковым.

gkirgizov commented 2 years ago

По поводу первого пункта: в каком-нибудь случае метрики для оптимизатора и для тюнера могут различаться? Или их можно объединить раз и навсегда?

nicl-nno commented 2 years ago

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

gkirgizov commented 2 years ago

@valer1435 Можешь немного пояснить 2 и 4 пункты? Возможно, с примерами

valer1435 commented 2 years ago

По поводу первого пункта: в каком-нибудь случае метрики для оптимизатора и для тюнера могут различаться? Или их можно объединить раз и навсегда?

Тут я имел ввиду, что сейчас в тюнер напрямую прокидываются callable метрики (sklearn'овские roc-auc или f1). Хотелось бы использовать тот класс метрик, который мы используем в композере. (Также хочется Objective, про который ты уже писал выше)

@valer1435 Можешь немного пояснить 2 и 4 пункты? Возможно, с примерами

По второму пункту - сейчас в tune_pipeline передается куча параметров, которые можно было заменить на Objective. Еще у нас есть sequential тюнер, который нигде, кроме теста не используется.

По четвертому - это относится больше к представлению параметров моделей. Сейчас параметры моделей - это просто словарь. Иногда возникают ситуации, когда нужно обновить некорректные параметры модели (например, длину лага, если она больше длинны временного ряда). Сейчас для этого нужно ставить специальные флаги и get_params() должен возвращать помимо списка параметров еще и список названий измененных параметров. И получается такое. Предлагаю сделать обертку над параметрами, которая будет сама подхватывать статус параметра (изменился ли он без участия тюнера/композера)

gkirgizov commented 2 years ago

Спасибо за подробные пояснения.

По четвертому - это относится больше к представлению параметров моделей.

Действительно это стоит переделать, хорошо, что отметил это. Я посмотрел -- тянет на отдельный issue, так как затрагиает много реализаций и много классов (Tuner, Node, ModelImplementation и другие). Предлагаю это реализовать следующим шагом. Я опишу и создам issue позже.

Dreamlone commented 2 years ago

у нас есть sequential тюнер, который нигде, кроме теста не используется

Это не совсем так. Последовательный тюнер имеет важную особенность: он может настраивать отдельные узлы, а не пайплайны целиком. Такая функциональность например нужна при локальном анализе чувствительности, который реализован в фреймворке версии ещё Иры. Если решите от последовательного избавляться, то стоит вынести эту функциональность в отдельную сущность

gkirgizov commented 1 year ago

closed by #21