Skip to content

Commit

Permalink
fix various spelling errors in gs stack (facebook#2448)
Browse files Browse the repository at this point in the history
Summary:

Installed a spell checker and fixed all remaining spelling errors in generation strategy files (gs, generation node, tests, transition criterion)

Differential Revision: D57175357
  • Loading branch information
mgarrard authored and facebook-github-bot committed May 11, 2024
1 parent 0ddfc3e commit 87ac517
Show file tree
Hide file tree
Showing 7 changed files with 60 additions and 58 deletions.
4 changes: 2 additions & 2 deletions ax/modelbridge/generation_node.py
Original file line number Diff line number Diff line change
Expand Up @@ -401,15 +401,15 @@ def node_that_generated_last_gr(self) -> Optional[str]:

@property
def transition_edges(self) -> Dict[str, List[TransitionCriterion]]:
"""Returns a dictionary mapping the next ```GenerationNode``` to the
"""Returns a dictionary mapping the next ``GenerationNode`` to the
TransitionCriteria that define the transition that that node.
Ex: if the transition from the current node to node x is defined by MaxTrials
and MinTrials criterion then the return would be {'x': [MaxTrials, MinTrials]}.
Returns:
Dict[str, List[TransitionCriterion]]: A dictionary mapping the next
```GenerationNode``` to the ```TransitionCriterion``` that are associated
``GenerationNode`` to the ``TransitionCriterion`` that are associated
with it.
"""
if self.transition_criteria is None:
Expand Down
28 changes: 15 additions & 13 deletions ax/modelbridge/generation_strategy.py
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@ class GenerationStrategy(GenerationStrategyInterface):
should use nodes. Notably, either, but not both, of `nodes` and `steps`
must be provided.
steps: A list of `GenerationStep` describing steps of this strategy.
name: An optional name for this generaiton strategy. If not specified,
name: An optional name for this generation strategy. If not specified,
strategy's name will be names of its nodes' models joined with '+'.
"""

Expand Down Expand Up @@ -383,8 +383,8 @@ def gen_with_multiple_nodes(
pending_observations: Optional[Dict[str, List[ObservationFeatures]]] = None,
**kwargs: Any, # TODO: @mgarrard Ensure correct dispatch to nodes
) -> List[GeneratorRun]:
"""Produces a List of GeneratorRuns for a single trial, either ```Trial``` or
```BatchTrial```, and if producing a ```BatchTrial`` allows for multiple
"""Produces a List of GeneratorRuns for a single trial, either ``Trial`` or
``BatchTrial``, and if producing a ``BatchTrial`` allows for multiple
models to be used to generate GeneratorRuns for that trial.
NOTE: This method is in development. Please do not use it yet.
Expand All @@ -404,7 +404,7 @@ def gen_with_multiple_nodes(
resuggesting points that are currently being evaluated.
Returns:
A list of ```GeneratorRuns``` for a single trial.
A list of ``GeneratorRuns`` for a single trial.
"""
# TODO: @mgarrard merge into gen method, just starting here to derisk
grs = []
Expand Down Expand Up @@ -485,7 +485,7 @@ def current_generator_run_limit(
generate any more generator runs at all.
"""
try:
self._maybe_move_to_next_step(raise_data_required_error=False)
self._maybe_transition_to_next_node(raise_data_required_error=False)
except GenerationStrategyCompleted:
return 0, True

Expand Down Expand Up @@ -526,9 +526,9 @@ def _validate_and_set_step_sequence(self, steps: List[GenerationStep]) -> None:
This function validates:
1. That only the last step has num_trials=-1, which indicates unlimited
trial generation is possible.
2. That each step's num_trials attrivute is either positive or -1
2. That each step's num_trials attribute is either positive or -1
3. That each step's max_parallelism attribute is either None or positive
It then sets the corect TransitionCriterion and node_name attributes on the
It then sets the correct TransitionCriterion and node_name attributes on the
underlying GenerationNode objects.
"""
for idx, step in enumerate(steps):
Expand All @@ -537,8 +537,8 @@ def _validate_and_set_step_sequence(self, steps: List[GenerationStep]) -> None:
raise UserInputError(
"Only last step in generation strategy can have "
"`num_trials` set to -1 to indicate that the model in "
"the step shouldbe used to generate new trials "
"indefinitely unless completion critera present."
"the step should be used to generate new trials "
"indefinitely unless completion criteria present."
)
elif step.num_trials < 1 and step.num_trials != -1:
raise UserInputError(
Expand Down Expand Up @@ -691,12 +691,12 @@ def _gen_multiple(
"""Produce multiple generator runs at once, to be made into multiple
trials on the experiment.
NOTE: This is used to ensure that maximum paralellism and number
NOTE: This is used to ensure that maximum parallelism and number
of trials per node are not violated when producing many generator
runs from this generation strategy in a row. Without this function,
if one generates multiple generator runs without first making any
of them into running trials, generation strategy cannot enforce that it only
produces as many generator runs as are allowed by the paralellism
produces as many generator runs as are allowed by the parallelism
limit and the limit on number of trials in current node.
Args:
Expand All @@ -721,7 +721,7 @@ def _gen_multiple(
``ModelSpec.gen``, which will pass them to ``ModelBridge.gen``.
"""
self.experiment = experiment
self._maybe_move_to_next_step()
self._maybe_transition_to_next_node()
self._fit_current_model(data=data)

# Get GeneratorRun limit that respects the node's transition criterion that
Expand Down Expand Up @@ -810,7 +810,9 @@ def _fit_current_model(self, data: Optional[Data]) -> None:
self._curr.fit(experiment=self.experiment, data=data, **model_state_on_lgr)
self._model = self._curr._fitted_model

def _maybe_move_to_next_step(self, raise_data_required_error: bool = True) -> bool:
def _maybe_transition_to_next_node(
self, raise_data_required_error: bool = True
) -> bool:
"""Moves this generation strategy to next node if the current node is completed,
and it is not the last node in this generation strategy. This method is safe to
use both when generating candidates or simply checking how many generator runs
Expand Down
12 changes: 6 additions & 6 deletions ax/modelbridge/tests/test_aepsych_criterion.py
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ def test_single_criterion(self) -> None:

# Has not seen enough of each preference
self.assertFalse(
generation_strategy._maybe_move_to_next_step(
generation_strategy._maybe_transition_to_next_node(
raise_data_required_error=False
)
)
Expand All @@ -71,7 +71,7 @@ def test_single_criterion(self) -> None:
with patch.object(experiment, "fetch_data", return_value=data):
# We have seen three "yes" and three "no"
self.assertTrue(
generation_strategy._maybe_move_to_next_step(
generation_strategy._maybe_transition_to_next_node(
raise_data_required_error=False
)
)
Expand Down Expand Up @@ -105,7 +105,7 @@ def test_many_criteria(self) -> None:

# Has not seen enough of each preference
self.assertFalse(
generation_strategy._maybe_move_to_next_step(
generation_strategy._maybe_transition_to_next_node(
raise_data_required_error=False
)
)
Expand All @@ -125,7 +125,7 @@ def test_many_criteria(self) -> None:
# We have seen three "yes" and three "no", but not enough trials
# are completed
self.assertFalse(
generation_strategy._maybe_move_to_next_step(
generation_strategy._maybe_transition_to_next_node(
raise_data_required_error=False
)
)
Expand All @@ -138,7 +138,7 @@ def test_many_criteria(self) -> None:
# Enough trials are completed but we have not seen three "yes" and three
# "no"
self.assertFalse(
generation_strategy._maybe_move_to_next_step(
generation_strategy._maybe_transition_to_next_node(
raise_data_required_error=False
)
)
Expand All @@ -147,7 +147,7 @@ def test_many_criteria(self) -> None:
# Enough trials are completed but we have not seen three "yes" and three
# "no"
self.assertTrue(
generation_strategy._maybe_move_to_next_step(
generation_strategy._maybe_transition_to_next_node(
raise_data_required_error=False
)
)
Expand Down
6 changes: 3 additions & 3 deletions ax/modelbridge/tests/test_generation_strategy.py
Original file line number Diff line number Diff line change
Expand Up @@ -1323,7 +1323,7 @@ def test_generation_strategy_eq_no_print(self) -> None:
self.assertEqual(gs1, gs2)

def test_gs_with_competing_transition_edges(self) -> None:
"""Test that a ```GenerationStrategy``` with a node with competing transition
"""Test that a ``GenerationStrategy`` with a node with competing transition
edges correctly transitions.
"""
# this gs has a single sobol node which transitions to gpei. If the MaxTrials
Expand All @@ -1340,7 +1340,7 @@ def test_gs_with_competing_transition_edges(self) -> None:
self.assertEqual(gs.current_node_name, "sobol_3")

def test_gs_with_competing_transition_edges_2(self) -> None:
"""Test that a ```GenerationStrategy``` with a node with competing transition
"""Test that a ``GenerationStrategy`` with a node with competing transition
edges correctly transitions.
"""
# this gs has a single sobol node which transitions to gpei. If the MaxTrials
Expand All @@ -1358,7 +1358,7 @@ def test_gs_with_competing_transition_edges_2(self) -> None:
self.assertEqual(gs.current_node_name, "sobol_3")

def test_gs_with_competing_transition_edges_3(self) -> None:
"""Test that a ```GenerationStrategy``` with a node with competing transition
"""Test that a ``GenerationStrategy`` with a node with competing transition
edges correctly transitions.
"""
# this gs has a single sobol node which transitions to gpei. If the MaxTrials
Expand Down
14 changes: 7 additions & 7 deletions ax/modelbridge/tests/test_transition_criterion.py
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ def setUp(self) -> None:
self.branin_experiment = get_branin_experiment()

def test_minimum_preference_criterion(self) -> None:
"""Tests the minimum preference criterion subcalss of TransitionCriterion."""
"""Tests the minimum preference criterion subclass of TransitionCriterion."""
criterion = MinimumPreferenceOccurances(metric_name="m1", threshold=3)
experiment = get_experiment()
generation_strategy = GenerationStrategy(
Expand All @@ -68,7 +68,7 @@ def test_minimum_preference_criterion(self) -> None:

# Has not seen enough of each preference
self.assertFalse(
generation_strategy._maybe_move_to_next_step(
generation_strategy._maybe_transition_to_next_node(
raise_data_required_error=False
)
)
Expand All @@ -87,7 +87,7 @@ def test_minimum_preference_criterion(self) -> None:
with patch.object(experiment, "fetch_data", return_value=data):
# We have seen three "yes" and three "no"
self.assertTrue(
generation_strategy._maybe_move_to_next_step(
generation_strategy._maybe_transition_to_next_node(
raise_data_required_error=False
)
)
Expand Down Expand Up @@ -192,14 +192,14 @@ def test_min_trials_is_met(self) -> None:
for _i in range(4):
experiment.new_trial(gs.gen(experiment=experiment))

# TODO: @mgarrard More comphrensive test of trials_from_node
# TODO: @mgarrard More comprehensive test of trials_from_node
node_0_trials = gs._steps[0].trials_from_node
node_1_trials = gs._steps[1].trials_from_node

self.assertEqual(len(node_0_trials), 4)
self.assertEqual(len(node_1_trials), 0)

# MinTrials is met should not pass yet, becasue no trials
# MinTrials is met should not pass yet, because no trials
# are marked completed
self.assertFalse(
gs._steps[0]
Expand Down Expand Up @@ -450,11 +450,11 @@ def test_repr(self) -> None:
+ "'use_all_trials_in_exp': False, "
+ "'continue_trial_generation': False})",
)
minimum_preference_occurances_criterion = MinimumPreferenceOccurances(
minimum_preference_occurrences_criterion = MinimumPreferenceOccurances(
metric_name="m1", threshold=3
)
self.assertEqual(
str(minimum_preference_occurances_criterion),
str(minimum_preference_occurrences_criterion),
"MinimumPreferenceOccurances({'metric_name': 'm1', 'threshold': 3, "
+ "'transition_to': None, 'block_gen_if_met': False, "
"'block_transition_if_unmet': True})",
Expand Down
50 changes: 25 additions & 25 deletions ax/modelbridge/transition_criterion.py
Original file line number Diff line number Diff line change
Expand Up @@ -41,11 +41,11 @@ class TransitionCriterion(SortableBase, SerializationMixin):
defaults to setting this to False since we can complete and move on from
this node without ever reaching its threshold.
complete_trial_generation: A flag to indicate that all generation for a given
trial is completed. This is necessary because in ```BatchTrial``` there
trial is completed. This is necessary because in ``BatchTrial`` there
are multiple arms per trial, and we enable generation of arms within a
batch from different ```GenerationNodes```. This flag should be set to
True for the last node in a set of ```GenerationNodes``` expected to
create a given ```BatchTrial```.
batch from different ``GenerationNodes``. This flag should be set to
True for the last node in a set of ``GenerationNodes`` expected to
create a given ``BatchTrial``.
"""

_transition_to: Optional[str] = None
Expand Down Expand Up @@ -117,11 +117,11 @@ class AutoTransitionAfterGenCriterion(TransitionCriterion):
setting this to True to ensure we validate a GeneratorRun is generated by
the current GenerationNode.
complete_trial_generation: A flag to indicate that all generation for a given
trial is completed. This is necessary because in ```BatchTrial``` there
trial is completed. This is necessary because in ``BatchTrial`` there
are multiple arms per trial, and we enable generation of arms within a
batch from different ```GenerationNodes```. This flag should be set to
True for the last node in a set of ```GenerationNodes``` expected to
create a given ```BatchTrial```.
batch from different ``GenerationNodes``. This flag should be set to
True for the last node in a set of ``GenerationNodes`` expected to
create a given ``BatchTrial``.
"""

def __init__(
Expand Down Expand Up @@ -167,7 +167,7 @@ class TrialBasedCriterion(TransitionCriterion):
generate at most 3 trials, then the threshold is 3.
block_transition_if_unmet: A flag to prevent the node from completing and
being able to transition to another node. Ex: MaxGenerationParallelism
defaults to setting this to Flase since we can complete and move on from
defaults to setting this to False since we can complete and move on from
this node without ever reaching its threshold.
block_gen_if_met: A flag to prevent continued generation from the
associated GenerationNode if this criterion is met but other criterion
Expand All @@ -184,11 +184,11 @@ class TrialBasedCriterion(TransitionCriterion):
use_all_trials_in_exp: A flag to use all trials in the experiment, instead of
only those generated by the current GenerationNode.
complete_trial_generation: A flag to indicate that all generation for a given
trial is completed. This is necessary because in ```BatchTrial``` there
trial is completed. This is necessary because in ``BatchTrial`` there
are multiple arms per trial, and we enable generation of arms within a
batch from different ```GenerationNodes```. This flag should be set to
True for the last node in a set of ```GenerationNodes``` expected to
create a given ```BatchTrial```.
batch from different ``GenerationNodes``. This flag should be set to
True for the last node in a set of ``GenerationNodes`` expected to
create a given ``BatchTrial``.
"""

def __init__(
Expand Down Expand Up @@ -344,11 +344,11 @@ class MaxGenerationParallelism(TrialBasedCriterion):
use_all_trials_in_exp: A flag to use all trials in the experiment, instead of
only those generated by the current GenerationNode.
complete_trial_generation: A flag to indicate that all generation for a given
trial is completed. This is necessary because in ```BatchTrial``` there
trial is completed. This is necessary because in ``BatchTrial`` there
are multiple arms per trial, and we enable generation of arms within a
batch from different ```GenerationNodes```. This flag should be set to
True for the last node in a set of ```GenerationNodes``` expected to
create a given ```BatchTrial```. Defautls to False for
batch from different ``GenerationNodes``. This flag should be set to
True for the last node in a set of ``GenerationNodes`` expected to
create a given ``BatchTrial``. Defaults to False for
MaxGenerationParallelism since this criterion isn't currently used for
node -> node or trial -> trial transition.
"""
Expand Down Expand Up @@ -428,11 +428,11 @@ class MaxTrials(TrialBasedCriterion):
use_all_trials_in_exp: A flag to use all trials in the experiment, instead of
only those generated by the current GenerationNode.
complete_trial_generation: A flag to indicate that all generation for a given
trial is completed. This is necessary because in ```BatchTrial``` there
trial is completed. This is necessary because in ``BatchTrial`` there
are multiple arms per trial, and we enable generation of arms within a
batch from different ```GenerationNodes```. This flag should be set to
True for the last node in a set of ```GenerationNodes``` expected to
create a given ```BatchTrial```.
batch from different ``GenerationNodes``. This flag should be set to
True for the last node in a set of ``GenerationNodes`` expected to
create a given ``BatchTrial``.
"""

def __init__(
Expand Down Expand Up @@ -504,11 +504,11 @@ class MinTrials(TrialBasedCriterion):
use_all_trials_in_exp: A flag to use all trials in the experiment, instead of
only those generated by the current GenerationNode.
complete_trial_generation: A flag to indicate that all generation for a given
trial is completed. This is necessary because in ```BatchTrial``` there
trial is completed. This is necessary because in ``BatchTrial`` there
are multiple arms per trial, and we enable generation of arms within a
batch from different ```GenerationNodes```. This flag should be set to
True for the last node in a set of ```GenerationNodes``` expected to
create a given ```BatchTrial```.
batch from different ``GenerationNodes``. This flag should be set to
True for the last node in a set of ``GenerationNodes`` expected to
create a given ``BatchTrial``.
"""

def __init__(
Expand Down
Loading

0 comments on commit 87ac517

Please sign in to comment.