Fix adding mutation as parent operator #262
Conversation
YamLyubov
commented
Jan 29, 2024
YamLyubov
commented
- Mutation should be saved as a string to avoid problems with custom mutations serialisation
- Bandits should store arm action mapping accordingly: key should be a string
|
Hello @YamLyubov! 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 2024-06-05 10:17:55 UTC |
| @@ -25,7 +25,8 @@ def __init__(self, | |||
| super().__init__(actions=actions, enable_logging=enable_logging) | |||
| self.actions = list(actions) | |||
| self._indices = list(range(len(actions))) | |||
| self._arm_by_action = dict(zip(actions, self._indices)) | |||
| # str because parent operator for mutation is stored as string for custom mutations serialisation | |||
| self._arm_by_action = dict(map(lambda x, y: (str(x), y), actions, self._indices)) | |||
There was a problem hiding this comment.
Преобразование str(fun), если не ошибаюсь, в разных потоках для одной функции даст разные результаты. Если бандит будет работать в разных потоках (что возможно в будущем?), то это место для потенциальной ошибки.
Можно перебдеть и везде использовать __name__ и name. Проще всего сделат маппинг __name__ на name для Enum с типами мутаций.
There was a problem hiding this comment.
Хотя, в целом, дело даже не в бандитах. Если индивиды будут готовиться в разных потоках (что будет, надеюсь, в ближайшем будущем), то при использовании str одна и та же функция мутации даст разные строки в разных потоках, что некорректно.
There was a problem hiding this comment.
Это тоже поправила
There was a problem hiding this comment.
Я тут вспомнил один грустный момент. У partial нет __name__, поэтому все упадет если функция мутации будет обёрнута в него.
There was a problem hiding this comment.
Можно поступить не очень изящно: создать класс CustomMutation и оборачивать в него все кастомные мутации. Или динамически создавать Enum с мутациями, тогда у каждой мутации будет name.
YamLyubov
force-pushed
the
mutation-parent-operator
branch
from
25ab80a
to
54e5cd1
Compare
