-
Notifications
You must be signed in to change notification settings - Fork 7
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
Add tuning history to OptHistory #228
base: main
Are you sure you want to change the base?
Conversation
Hello @MorrisNein! 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 2023-11-04 12:07:43 UTC |
Hello @MorrisNein! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:
Comment last updated at 2023-11-04 12:07:51 UTC |
Codecov Report
@@ Coverage Diff @@
## main #228 +/- ##
==========================================
+ Coverage 72.16% 72.29% +0.13%
==========================================
Files 135 136 +1
Lines 8061 8184 +123
==========================================
+ Hits 5817 5917 +100
- Misses 2244 2267 +23
|
golem/core/tuning/tuner_interface.py
Outdated
for tuned_graph in tuned_graphs: | ||
obtained_metric = self.get_metric_value(graph=tuned_graph) | ||
obtained_metric = self.evaluate_graph(graph=tuned_graph, label='tuning_result') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Немного не догоняю что происходит)) В этом цикле будут создаваться новые поколения в history
для каждого посчитанного графа?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
В этом цикле будут создаваться новые поколения в history для каждого посчитанного графа?
Да, но так быть не должно. Исправил, разделив оценку графов и сохранение истории.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Я правильно понимаю, что есть тест, в котором проверяется соответствие количества новых поколений количеству шагов тюнера?
0af205f
to
37de94d
Compare
Добавил в тюнеры работу с классами истории оптимизации, раз уж теперь они могут сохранять в неё результаты. Сравнение графов между собой, на мой взгляд, стало удобнее, особенно для MultiObjFitness: не нужно перегонять метрики в класс MultiObjFitness и обратно. |
37de94d
to
1c42b3f
Compare
|
||
import numpy as np | ||
from iOpt.method.listener import ConsoleFullOutputListener | ||
from iOpt.problem import Problem | ||
from iOpt.solver import Solver | ||
from iOpt.solver_parametrs import SolverParameters | ||
from iOpt.trial import Point, FunctionValue | ||
from iOpt.trial import FunctionValue, Point |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Почему в этом тюнере не добавлена возможность сохранить в историю?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Пока что сохраняются изначальный и финальный графы через initial_check и final_check.
Но тут можно прокинуть в GolemProblem функцию для сохранения промежуточных результатов. Пожалуй, так и сделаю.
@@ -36,8 +43,7 @@ def __init__(self, | |||
default_save_dir: Optional[os.PathLike] = None): | |||
self._objective = objective or ObjectiveInfo() | |||
self._generations: List[Generation] = [] | |||
self.archive_history: List[List[Individual]] = [] | |||
self._tuning_result: Optional[Graph] = None | |||
self.evolution_best_archive: List[List[Individual]] = [] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Я не совсем понимаю зачем нужно такое переименование?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Я пока не реализовал ведение archive_history для тюнинга, потому что для оптимизации его ведёт фронт Парето в составе оптимизатора. Поэтому название поля явно отражает, что оно относится к истории эволюции, но не тюнинга.
Честно, не знаю, как тут лучше поступить.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ну точно не нужна отсылка к эволюции, для большей абстрактности.
Мне кажется идея что archive_history это основная история, а для файн-тюнинга - отдельная, будет доступной пользователю.
class OptHistoryLabels(str, Enum): | ||
initial_assumptions = 'initial_assumptions' | ||
extended_initial_assumptions = 'extended_initial_assumptions' | ||
evolution_results = 'evolution_results' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Аналогично, можно про эволюцию явно не писать.
@@ -61,8 +67,8 @@ def add_to_history(self, individuals: Sequence[Individual], generation_label: Op | |||
generation = Generation(individuals, self.generations_count, generation_label, generation_metadata) | |||
self.generations.append(generation) | |||
|
|||
def add_to_archive_history(self, individuals: Sequence[Individual]): | |||
self.archive_history.append(list(individuals)) | |||
def add_to_evolution_best_archive(self, individuals: Sequence[Individual]): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Может сделать единый метод, просто передавать в параметрах нужную метку?
self.init_metric = None | ||
self.obtained_metric = None | ||
self.init_individual = None | ||
self.obtained_individual = None |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Лучше final
Хорошо бы в одном из примеров показать как использовать новую фунциональность. У @YamLyubov был пример с раскраской графа, например. |
Adds an option to save graph to OptHistory after each objective evaluation in the process of tuning (Solves #226).
Allows tuners to work with Individual and Fitness for graphs comparison & history saving.
Changes OptHistory fields:
tuning_result
final_choices
->evolution_results
archive_history
->evolution_best_archive
Allows histories backward compatibility.
Fixes previously unknown bug with multi objective tuning.