Skip to content

Commit

Permalink
Light cleanup of GenerationStrategyInterface (#2256)
Browse files Browse the repository at this point in the history
Summary:
Pull Request resolved: #2256

Key changes:
1. bring back to GS aspects of GSI that are not needed there and are polluting the interface,
2. make `_name` a required attribute of both GSI and GS.

Re: 2), having it be set during the first call to `name` property was causing weird bugs in equality checks, there two GSs looked like they were equal but they weren't at the time of the initial equality check (one had the `_name` set because its `name` prop was called, and another did not yet). They would become equal during the call to `__repr__` that occurred in reporting their inequality as an error (!), because `__repr__` would call `GS.name`, which would result in `GS._name` getting set. Weird stuff!

Reviewed By: danielcohenlive, mgarrard

Differential Revision: D51441575

fbshipit-source-id: 44908f30be2f93be4713c06bd81777d3e6235d43
  • Loading branch information
Lena Kashtelyan authored and facebook-github-bot committed Mar 8, 2024
1 parent b7e0312 commit c7dadf4
Show file tree
Hide file tree
Showing 6 changed files with 151 additions and 48 deletions.
87 changes: 44 additions & 43 deletions ax/core/generation_strategy_interface.py
61 changes: 61 additions & 0 deletions ax/core/tests/test_generation_strategy_interface.py
27 changes: 24 additions & 3 deletions ax/modelbridge/generation_strategy.py
12 changes: 11 additions & 1 deletion ax/modelbridge/tests/test_generation_strategy.py
5 changes: 4 additions & 1 deletion ax/service/tests/scheduler_test_utils.py
7 changes: 7 additions & 0 deletions ax/utils/testing/core_stubs.py

0 comments on commit c7dadf4

Please sign in to comment.